Ensure that exclusive regulators are properly disabled when their reference
count drops to one before they are released. This prevents possible issues
where exclusive regulators may remain enabled unintentionally after being
put.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/regulator/devres.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 2cf03042fddf..48da9823ce2f 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -16,7 +16,13 @@
 
 static void devm_regulator_release(struct device *dev, void *res)
 {
-	regulator_put(*(struct regulator **)res);
+	struct regulator *regulator = *(struct regulator **)res;
+	struct regulator_dev *rdev = regulator->rdev;
+
+	if (rdev->exclusive && regulator->enable_count == 1)
+		regulator_disable(regulator);
+
+	regulator_put(regulator);
 }
 
 static struct regulator *_devm_regulator_get(struct device *dev, const char *id,
-- 
2.43.0

On Wed, Oct 01, 2025 at 11:26:51PM +0200, Tommaso Merciai wrote:

You've not copied me on the rest of the series so I don't know what's
going on with dependencies.  When sending a patch series it is important
to ensure that all the various maintainers understand what the
relationship between the patches as the expecation is that there will be
interdependencies.  Either copy everyone on the whole series or at least
copy them on the cover letter and explain what's going on.  If there are
no strong interdependencies then it's generally simplest to just send
the patches separately to avoid any possible confusion.

> Ensure that exclusive regulators are properly disabled when their reference
> count drops to one before they are released. This prevents possible issues
> where exclusive regulators may remain enabled unintentionally after being
> put.

The reason we don't normally drop references that devices hold is that
we're allowing the driver to control if the suppy should be disabled on
exit, powering off something that's critical for the system just because
we're not managing it in software won't go well.  Consider reloading a
module during development for example.

>  static void devm_regulator_release(struct device *dev, void *res)
>  {
> -	regulator_put(*(struct regulator **)res);
> +	struct regulator *regulator = *(struct regulator **)res;
> +	struct regulator_dev *rdev = regulator->rdev;
> +
> +	if (rdev->exclusive && regulator->enable_count == 1)
> +		regulator_disable(regulator);
> +
> +	regulator_put(regulator);
>  }

There's no reason that exclusive consumers don't use the refcounting
support...

Hi Mark,
Thank you for your comments,

On Thu, Oct 02, 2025 at 05:29:19PM +0100, Mark Brown wrote:
> On Wed, Oct 01, 2025 at 11:26:51PM +0200, Tommaso Merciai wrote:
> 
> You've not copied me on the rest of the series so I don't know what's
> going on with dependencies.  When sending a patch series it is important
> to ensure that all the various maintainers understand what the
> relationship between the patches as the expecation is that there will be
> interdependencies.  Either copy everyone on the whole series or at least
> copy them on the cover letter and explain what's going on.  If there are
> no strong interdependencies then it's generally simplest to just send
> the patches separately to avoid any possible confusion.

Thanks for the explanation.
I made a mistake when I sent the series.
I only ran ./scripts/get_maintainer.pl on some patches, not all.
My fault, sorry for that.

> 
> > Ensure that exclusive regulators are properly disabled when their reference
> > count drops to one before they are released. This prevents possible issues
> > where exclusive regulators may remain enabled unintentionally after being
> > put.
> 
> The reason we don't normally drop references that devices hold is that
> we're allowing the driver to control if the suppy should be disabled on
> exit, powering off something that's critical for the system just because
> we're not managing it in software won't go well.  Consider reloading a
> module during development for example.
> 
> >  static void devm_regulator_release(struct device *dev, void *res)
> >  {
> > -	regulator_put(*(struct regulator **)res);
> > +	struct regulator *regulator = *(struct regulator **)res;
> > +	struct regulator_dev *rdev = regulator->rdev;
> > +
> > +	if (rdev->exclusive && regulator->enable_count == 1)
> > +		regulator_disable(regulator);
> > +
> > +	regulator_put(regulator);
> >  }
> 
> There's no reason that exclusive consumers don't use the refcounting
> support...

I will need to move the refcounting handlingfor the exclusive regulator
at USB driver lvl.
The drivers/phy/renesas/phy-rcar-gen3-usb2.c is using
regulator_hardware_enable() for some USB otg channel. I think this is
the reason why I need this patch to handle multiple unbind/bind.
Without this I'm getting some WARN_ON(regulator->enable_count) doing
multiple unbind/bind.

I'm going to investigate on that and I need to find a solution at usb driver lvl.
Thanks again for your feedback!

Regards,
Tommaso

On Fri, Oct 03, 2025 at 07:33:25PM +0200, Tommaso Merciai wrote:
> On Thu, Oct 02, 2025 at 05:29:19PM +0100, Mark Brown wrote:

> I will need to move the refcounting handlingfor the exclusive regulator
> at USB driver lvl.
> The drivers/phy/renesas/phy-rcar-gen3-usb2.c is using
> regulator_hardware_enable() for some USB otg channel. I think this is
> the reason why I need this patch to handle multiple unbind/bind.
> Without this I'm getting some WARN_ON(regulator->enable_count) doing
> multiple unbind/bind.

Are you sure it's not just that the driver doesn't always disable the
regulator before unbinding?  It only disables in the power off callback
so might leave a dangling reference behind.

Hi Mark,
Thanks for your comment.

On Mon, Oct 06, 2025 at 12:52:29PM +0100, Mark Brown wrote:
> On Fri, Oct 03, 2025 at 07:33:25PM +0200, Tommaso Merciai wrote:
> > On Thu, Oct 02, 2025 at 05:29:19PM +0100, Mark Brown wrote:
> 
> > I will need to move the refcounting handlingfor the exclusive regulator
> > at USB driver lvl.
> > The drivers/phy/renesas/phy-rcar-gen3-usb2.c is using
> > regulator_hardware_enable() for some USB otg channel. I think this is
> > the reason why I need this patch to handle multiple unbind/bind.
> > Without this I'm getting some WARN_ON(regulator->enable_count) doing
> > multiple unbind/bind.
> 
> Are you sure it's not just that the driver doesn't always disable the
> regulator before unbinding?  It only disables in the power off callback
> so might leave a dangling reference behind.

Yep you are right.
I will fix that in v2 dropping this unnecessary patch.

Thanks & Regards,
Tommaso