regulator: fixed: Rely on the core freeing the enable GPIO

[PATCH] regulator: fixed: Rely on the core freeing the enable GPIO

Posted by Mark Brown 2 weeks ago

In order to simplify ownership rules for enable GPIOs supplied by drivers
regulator_register() always takes ownership of them, even if it ends up
failing for some other reason. We therefore should	not free the GPIO if
registration fails but just let the core worry about things.

Fixes: 636f4618b1cd (regulator: fixed: fix GPIO descriptor leak on register failure)
Reported-by: Diederik de Haas <diederik@cknow-tech.com>
Closes: https://lore.kernel.org/r/DEPEYUF5BRGY.UKFBWRRE8HNP@cknow-tech.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/fixed.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index a2d16e9abfb5..254c0a8a4555 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -330,13 +330,10 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 
 	drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
 					       &cfg);
-	if (IS_ERR(drvdata->dev)) {
-		ret = dev_err_probe(&pdev->dev, PTR_ERR(drvdata->dev),
-				    "Failed to register regulator: %ld\n",
-				    PTR_ERR(drvdata->dev));
-		gpiod_put(cfg.ena_gpiod);
-		return ret;
-	}
+	if (IS_ERR(drvdata->dev))
+		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->dev),
+				     "Failed to register regulator: %ld\n",
+				     PTR_ERR(drvdata->dev));
 
 	platform_set_drvdata(pdev, drvdata);
 

---
base-commit: 81d431130ae1af4e64030f6a956ee9137e6fc1b0
change-id: 20251204-regulator-fixed-fix-gpiod-leak-b1f50fb8c388

Best regards,
--  
Mark Brown <broonie@kernel.org>

Re: [PATCH] regulator: fixed: Rely on the core freeing the enable GPIO

Posted by Mark Brown 1 week, 6 days ago

On Thu, 04 Dec 2025 19:39:34 +0000, Mark Brown wrote:
> In order to simplify ownership rules for enable GPIOs supplied by drivers
> regulator_register() always takes ownership of them, even if it ends up
> failing for some other reason. We therefore should	not free the GPIO if
> registration fails but just let the core worry about things.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: fixed: Rely on the core freeing the enable GPIO
      commit: 79a45ddcdbba330f5139c7c7ff7042d69cf147b2

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH] regulator: fixed: Rely on the core freeing the enable GPIO

Posted by Diederik de Haas 2 weeks ago

On Thu Dec 4, 2025 at 8:39 PM CET, Mark Brown wrote:

s/enable GPIO/enabled GPIOs/ ? (in Subject)

> In order to simplify ownership rules for enable GPIOs supplied by drivers
> regulator_register() always takes ownership of them, even if it ends up
> failing for some other reason. We therefore should	not free the GPIO if
> registration fails but just let the core worry about things.

I built a 6.18 kernel with this patch added and that fixed the problem.
Thanks :-) Feel free to add:

Tested-by: Diederik de Haas <diederik@cknow-tech.com>

Cheers,
  Diederik

> Fixes: 636f4618b1cd (regulator: fixed: fix GPIO descriptor leak on register failure)
> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> Closes: https://lore.kernel.org/r/DEPEYUF5BRGY.UKFBWRRE8HNP@cknow-tech.com
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  drivers/regulator/fixed.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
> index a2d16e9abfb5..254c0a8a4555 100644
> --- a/drivers/regulator/fixed.c
> +++ b/drivers/regulator/fixed.c
> @@ -330,13 +330,10 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
>  
>  	drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
>  					       &cfg);
> -	if (IS_ERR(drvdata->dev)) {
> -		ret = dev_err_probe(&pdev->dev, PTR_ERR(drvdata->dev),
> -				    "Failed to register regulator: %ld\n",
> -				    PTR_ERR(drvdata->dev));
> -		gpiod_put(cfg.ena_gpiod);
> -		return ret;
> -	}
> +	if (IS_ERR(drvdata->dev))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->dev),
> +				     "Failed to register regulator: %ld\n",
> +				     PTR_ERR(drvdata->dev));
>  
>  	platform_set_drvdata(pdev, drvdata);
>  
>
> ---
> base-commit: 81d431130ae1af4e64030f6a956ee9137e6fc1b0
> change-id: 20251204-regulator-fixed-fix-gpiod-leak-b1f50fb8c388
>
> Best regards,
> --  
> Mark Brown <broonie@kernel.org>