[PATCH] gpio: omap: handle clk_prepare failure in probe

[PATCH] gpio: omap: handle clk_prepare failure in probe

[Jiawen Liu]

omap_gpio_probe() ignores the return value of clk_prepare(bank->dbck).
If clk_prepare fails, the clock is not prepared, but bank->dbck_flag
remains true. Later, omap_gpio_remove() or the probe error path calls
clk_unprepare(bank->dbck) unconditionally when dbck_flag is true,
leading to an unbalanced clock operation.

Check the return value of clk_prepare in omap_gpio_probe. On failure,
clear dbck_flag and return the error, preventing unbalanced
clk_unprepare in remove or error paths.

Signed-off-by: jiawen <1298662399@qq.com>
---
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1462,7 +1462,12 @@
 				"Could not get gpio dbck. Disable debounce\n");
 			bank->dbck_flag = false;
 		} else {
-			clk_prepare(bank->dbck);
+			ret = clk_prepare(bank->dbck);
+			if (ret) {
+				dev_err(dev, "Could not prepare gpio dbck\n");
+				bank->dbck_flag = false;
+				return ret;
+			}
 		}
 	}
 

Re: [PATCH] gpio: omap: handle clk_prepare failure in probe

[Markus Elfring]

…
> Check the return value of clk_prepare in omap_gpio_probe. On failure,
> clear dbck_flag and return the error, preventing unbalanced
> clk_unprepare in remove or error paths.

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2#n34
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n145
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n792

Regards,
Markus

Re: [PATCH] gpio: omap: handle clk_prepare failure in probe

[Andreas Kemnade]

On Thu, 20 Aug 2026 11:19:00 +0200
Markus Elfring <Markus.Elfring@web.de> wrote:

> …
> > Check the return value of clk_prepare in omap_gpio_probe. On failure,
> > clear dbck_flag and return the error, preventing unbalanced
> > clk_unprepare in remove or error paths.  
> 
> How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
> 
About adding this to stable:
does it bother anyone?
Is this actually being used with clocks having prepare() ops?
I do not see anything there. SoC-internal clocks are typically not
so easy being rewired.
To be clear, this should be fixed in -next to be prepared if something
more fundamental in clock handling changes.

Regards,
Andreas

> See also once more:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2#n34
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n145
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n792
> 
> Regards,
> Markus
> 
> 

Re: gpio: omap: handle clk_prepare failure in probe

[Markus Elfring]

> About adding this to stable:
> does it bother anyone?

I imagine that it can be safer to avoid return value ignorance a bit more.
https://cwe.mitre.org/data/definitions/252.html

Regards,
Markus

Re: gpio: omap: handle clk_prepare failure in probe

[Andreas Kemnade]

On Thu, 20 Aug 2026 12:47:05 +0200
Markus Elfring <Markus.Elfring@web.de> wrote:

> > About adding this to stable:
> > does it bother anyone?  
> 
> I imagine that it can be safer to avoid return value ignorance a bit more.
> https://cwe.mitre.org/data/definitions/252.html
> 
Well, if that would depend on user input, esp. over the network, it would be
clear.
But here you need to patch the devicetree to add a clock there
that has a prepare() in the ancestry. shows the desired behavior to exploit
something.

And then the error check is done by clk_enable() anyways.
BTW: here the return value of that is not checked. That is the more
interesting issue here.

Quoting stable kernel rules:
"
 - No "This could be a problem..." type of things like a "theoretical race
    condition", unless an explanation of how the bug can be exploited is also
    provided.
"
From taking that verbatim, I would say no CC stable.
But pragmatically, also to avoid noise in any security scanner, I would agree
to a CC stable here.

Regards,
Andreas

Re: [PATCH] gpio: omap: handle clk_prepare failure in probe

[Jiawen Liu]

Hi Andreas, Markus,

Thanks for looking into this. I don't have evidence that the debounce clock
in an existing configuration has a prepare operation that can fail, so my
original description should not imply an observed failure.

I'll check the clock providers and the devm_clk_get_prepared() option before
deciding whether an update is warranted, including whether a stable tag
would be appropriate.

Thanks,
Jiawen

Re: [PATCH] gpio: omap: handle clk_prepare failure in probe

[Dan Carpenter]

Andreas is the expert in this context, not Markus.  His suggestion to
use devm_clk_get_prepared() was a good idea.  Add a Fixes tag but a
CC to stable is not required.

regards,
dan carpenter

Re: [PATCH] gpio: omap: handle clk_prepare failure in probe

[Bartosz Golaszewski]

On Mon, Nov 09, 2026 at 08:47:17AM +0300, Dan Carpenter wrote:
> Andreas is the expert in this context, not Markus.  His suggestion to
> use devm_clk_get_prepared() was a good idea.  Add a Fixes tag but a
> CC to stable is not required.
>

Yes. Please don't spend time discussing with Markus, he's well known for
nonsensensical comments and you're wasting your time.

Add the Fixes: tag as it is - after all - a fix.

Thanks,
Bartosz

Re: gpio: omap: handle clk_prepare failure in probe

[Markus Elfring]

> Yes. Please don't spend time discussing with Markus, he's well known for
> nonsensensical comments and you're wasting your time.

Under which circumstances can the patch review communication become more constructive again?

Regards,
Markus

Re: [PATCH] gpio: omap: handle clk_prepare failure in probe

[Andreas Kemnade]

On Tue, 18 Aug 2026 17:08:54 +0400
Jiawen Liu <1298662399@qq.com> wrote:

> omap_gpio_probe() ignores the return value of clk_prepare(bank->dbck).
> If clk_prepare fails, the clock is not prepared, but bank->dbck_flag
> remains true. Later, omap_gpio_remove() or the probe error path calls
> clk_unprepare(bank->dbck) unconditionally when dbck_flag is true,
> leading to an unbalanced clock operation.
> 
> Check the return value of clk_prepare in omap_gpio_probe. On failure,
> clear dbck_flag and return the error, preventing unbalanced
> clk_unprepare in remove or error paths.
> 
> Signed-off-by: jiawen <1298662399@qq.com>
> ---
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1462,7 +1462,12 @@
>  				"Could not get gpio dbck. Disable debounce\n");
>  			bank->dbck_flag = false;
>  		} else {
> -			clk_prepare(bank->dbck);
> +			ret = clk_prepare(bank->dbck);
> +			if (ret) {
> +				dev_err(dev, "Could not prepare gpio dbck\n");
> +				bank->dbck_flag = false;
> +				return ret;
> +			}
>
What about simply using devm_clk_get_prepared() here? That would simplify
things a lot, given that AFAIK,  prepare is a no-op here anyways.

Regards,
Andreas

[PATCH v2] gpio: omap: handle debounce clock preparation failure

[Jiawen Liu]

omap_gpio_probe() ignores the return value of clk_prepare() for the
debounce clock. If preparation fails, the driver still treats debounce as
available and may use or unprepare a clock that was not prepared.

Use devm_clk_get_prepared() and disable debounce if the clock cannot be
obtained or prepared. This keeps the existing behavior for clock lookup
failures. Remove both manual clk_unprepare() calls because the managed
clock is released on probe failure and device removal.

An earlier managed-clock conversion was dropped after a reported boot
failure. A follow-up removing the two manual unprepare calls was reported
to restore boot. This change also removes those calls.
Built gpio-omap.o with ARM multi_v7_defconfig and Clang 19; the current
patch has not been tested on OMAP hardware.

The original report was generated using BMC-Agent; source review and
maintainer feedback informed this revision.

Fixes: 5d9452e7c52a ("gpio: omap: fix clk_prepare/unprepare usage")
Suggested-by: Andreas Kemnade <andreas@kemnade.info>
Link: https://lore.kernel.org/r/tencent_B4F504456790E8BA3C4EAB80F4BFB6954E08@qq.com
Link: https://lkml.rescloud.iu.edu/hypermail/linux/kernel/2412.2/00414.html
Signed-off-by: Jiawen Liu <1298662399@qq.com>
---
Changes in v2:
- Use devm_clk_get_prepared() and remove both manual unprepare calls.
- Disable debounce on prepare failure, retaining clock-get failure behavior.
- Add the Fixes tag and describe the earlier managed-clock attempt.

Built with ARCH=arm, LLVM=-19, multi_v7_defconfig, W=1 and
CONFIG_GPIO_OMAP=y. No OMAP hardware test or current reproducer is available.

 drivers/gpio/gpio-omap.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index a9734b2a1..314bbb48e 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1446,13 +1446,11 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	}
 
 	if (bank->dbck_flag) {
-		bank->dbck = devm_clk_get(dev, "dbclk");
+		bank->dbck = devm_clk_get_prepared(dev, "dbclk");
 		if (IS_ERR(bank->dbck)) {
 			dev_err(dev,
-				"Could not get gpio dbck. Disable debounce\n");
+				"Could not get or prepare gpio dbck. Disable debounce\n");
 			bank->dbck_flag = false;
-		} else {
-			clk_prepare(bank->dbck);
 		}
 	}
 
@@ -1470,8 +1468,6 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	if (ret) {
 		pm_runtime_put_sync(dev);
 		pm_runtime_disable(dev);
-		if (bank->dbck_flag)
-			clk_unprepare(bank->dbck);
 		return ret;
 	}
 
@@ -1492,8 +1488,6 @@ static void omap_gpio_remove(struct platform_device *pdev)
 	cpu_pm_unregister_notifier(&bank->nb);
 	gpiochip_remove(&bank->chip);
 	pm_runtime_disable(&pdev->dev);
-	if (bank->dbck_flag)
-		clk_unprepare(bank->dbck);
 }
 
 static int omap_gpio_runtime_suspend(struct device *dev)
-- 
2.34.1

Re: [PATCH v2] gpio: omap: handle debounce clock preparation failure

[Sverdlin, Alexander]

Hi Jiawen,

thank you for the patch!

On Mon, 2026-11-09 at 17:42 +0400, Jiawen Liu wrote:
> omap_gpio_probe() ignores the return value of clk_prepare() for the
> debounce clock. If preparation fails, the driver still treats debounce as
> available and may use or unprepare a clock that was not prepared.
> 
> Use devm_clk_get_prepared() and disable debounce if the clock cannot be
> obtained or prepared. This keeps the existing behavior for clock lookup
> failures. Remove both manual clk_unprepare() calls because the managed
> clock is released on probe failure and device removal.

The development history below could probably go below "---" so that
it doesn't appear in git history?

> An earlier managed-clock conversion was dropped after a reported boot
> failure. A follow-up removing the two manual unprepare calls was reported
> to restore boot. This change also removes those calls.
> Built gpio-omap.o with ARM multi_v7_defconfig and Clang 19; the current
> patch has not been tested on OMAP hardware.
> 
> The original report was generated using BMC-Agent; source review and
> maintainer feedback informed this revision.
> 
> Fixes: 5d9452e7c52a ("gpio: omap: fix clk_prepare/unprepare usage")
> Suggested-by: Andreas Kemnade <andreas@kemnade.info>
> Link: https://lore.kernel.org/r/tencent_B4F504456790E8BA3C4EAB80F4BFB6954E08@qq.com
> Link: https://lkml.rescloud.iu.edu/hypermail/linux/kernel/2412.2/00414.html
> Signed-off-by: Jiawen Liu <1298662399@qq.com>

other than above,
Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

The patch causes no regression on our AM335x HW deploying gpio-omap driver,
Tested-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
> Changes in v2:
> - Use devm_clk_get_prepared() and remove both manual unprepare calls.
> - Disable debounce on prepare failure, retaining clock-get failure behavior.
> - Add the Fixes tag and describe the earlier managed-clock attempt.
> 
> Built with ARCH=arm, LLVM=-19, multi_v7_defconfig, W=1 and
> CONFIG_GPIO_OMAP=y. No OMAP hardware test or current reproducer is available.
> 
>  drivers/gpio/gpio-omap.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index a9734b2a1..314bbb48e 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com