[PATCH] memory: stm32_omm: fix child clock leak on set_amcr() error path

[PATCH] memory: stm32_omm: fix child clock leak on set_amcr() error path

[Diego Fernando Mancera Gomez]

When the mux is enabled, stm32_omm_configure() enables the two OSPI
child clocks via stm32_omm_toggle_child_clock(dev, true); they are meant
to stay enabled for the device's lifetime and are disabled again in
stm32_omm_remove() and on the devm_of_platform_populate() error path.

However, if the subsequent stm32_omm_set_amcr() call fails, the function
jumps to the "error:" label which only calls pm_runtime_put_sync_suspend()
before returning, leaving the two child clocks enabled. As this happens
during probe, the error is propagated and .remove() is never called, so
the clock enable references are leaked. devm_clk_bulk_get() only releases
the clock handles on unwind, it does not undo clk_prepare_enable().

Disable the child clocks on the set_amcr() error path, mirroring the
cleanup already done on the devm_of_platform_populate() failure path.

Fixes: 8181d061dcff ("memory: Add STM32 Octo Memory Manager driver")
Cc: stable@vger.kernel.org
Signed-off-by: Diego Fernando Mancera Gomez <diegomancera.dev@gmail.com>
---
 drivers/memory/stm32_omm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/memory/stm32_omm.c b/drivers/memory/stm32_omm.c
index 84eb23c824f5..3599683ab04a 100644
--- a/drivers/memory/stm32_omm.c
+++ b/drivers/memory/stm32_omm.c
@@ -274,6 +274,8 @@ static int stm32_omm_configure(struct device *dev)
 	writel_relaxed(omm->cr, omm->io_base + OMM_CR);
 
 	ret = stm32_omm_set_amcr(dev, true);
+	if (ret && (mux & CR_MUXEN))
+		stm32_omm_toggle_child_clock(dev, false);
 
 error:
 	pm_runtime_put_sync_suspend(dev);
-- 
2.55.0

Re: [PATCH] memory: stm32_omm: fix child clock leak on set_amcr() error path

[Krzysztof Kozlowski]

On 18/09/2026 01:59, Diego Fernando Mancera Gomez wrote:
> When the mux is enabled, stm32_omm_configure() enables the two OSPI
> child clocks via stm32_omm_toggle_child_clock(dev, true); they are meant
> to stay enabled for the device's lifetime and are disabled again in
> stm32_omm_remove() and on the devm_of_platform_populate() error path.
> 
> However, if the subsequent stm32_omm_set_amcr() call fails, the function
> jumps to the "error:" label which only calls pm_runtime_put_sync_suspend()
> before returning, leaving the two child clocks enabled. As this happens
> during probe, the error is propagated and .remove() is never called, so
> the clock enable references are leaked. devm_clk_bulk_get() only releases
> the clock handles on unwind, it does not undo clk_prepare_enable().
> 
> Disable the child clocks on the set_amcr() error path, mirroring the
> cleanup already done on the devm_of_platform_populate() failure path.
> 
> Fixes: 8181d061dcff ("memory: Add STM32 Octo Memory Manager driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Diego Fernando Mancera Gomez <diegomancera.dev@gmail.com>
> ---
>  drivers/memory/stm32_omm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/memory/stm32_omm.c b/drivers/memory/stm32_omm.c
> index 84eb23c824f5..3599683ab04a 100644
> --- a/drivers/memory/stm32_omm.c
> +++ b/drivers/memory/stm32_omm.c
> @@ -274,6 +274,8 @@ static int stm32_omm_configure(struct device *dev)
>  	writel_relaxed(omm->cr, omm->io_base + OMM_CR);
>  
>  	ret = stm32_omm_set_amcr(dev, true);
> +	if (ret && (mux & CR_MUXEN))
> +		stm32_omm_toggle_child_clock(dev, false);

How does it work without st,omm-mux?

>  
>  error:
>  	pm_runtime_put_sync_suspend(dev);


Best regards,
Krzysztof

Re: [PATCH] memory: stm32_omm: fix child clock leak on set_amcr() error path

[Diego Fernando Mancera Gomez]

Without st,omm-mux, of_property_read_u32() fails and leaves mux at its
initial value of 0, so the "if (mux & CR_MUXEN)" block is skipped and
the child clocks are never enabled in stm32_omm_configure(). The new
check is then false and nothing is disabled on the set_amcr() error
path, so there is nothing to leak and no unbalanced
clk_disable_unprepare().

The cleanup uses exactly the same condition that gated
stm32_omm_toggle_child_clock(dev, true), so this also covers the case
where st,omm-mux is present but CR_MUXEN is not set. The earlier
"goto error" paths are all taken before the clocks are enabled, or
from toggle_child_clock() itself, which already unwinds on failure.

If it helps, I can send a v2 that states this in the commit message.

Best regards,
Diego


On Wed, Sep 23, 2026 at 8:54 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 18/09/2026 01:59, Diego Fernando Mancera Gomez wrote:
> > When the mux is enabled, stm32_omm_configure() enables the two OSPI
> > child clocks via stm32_omm_toggle_child_clock(dev, true); they are meant
> > to stay enabled for the device's lifetime and are disabled again in
> > stm32_omm_remove() and on the devm_of_platform_populate() error path.
> >
> > However, if the subsequent stm32_omm_set_amcr() call fails, the function
> > jumps to the "error:" label which only calls pm_runtime_put_sync_suspend()
> > before returning, leaving the two child clocks enabled. As this happens
> > during probe, the error is propagated and .remove() is never called, so
> > the clock enable references are leaked. devm_clk_bulk_get() only releases
> > the clock handles on unwind, it does not undo clk_prepare_enable().
> >
> > Disable the child clocks on the set_amcr() error path, mirroring the
> > cleanup already done on the devm_of_platform_populate() failure path.
> >
> > Fixes: 8181d061dcff ("memory: Add STM32 Octo Memory Manager driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Diego Fernando Mancera Gomez <diegomancera.dev@gmail.com>
> > ---
> >  drivers/memory/stm32_omm.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/memory/stm32_omm.c b/drivers/memory/stm32_omm.c
> > index 84eb23c824f5..3599683ab04a 100644
> > --- a/drivers/memory/stm32_omm.c
> > +++ b/drivers/memory/stm32_omm.c
> > @@ -274,6 +274,8 @@ static int stm32_omm_configure(struct device *dev)
> >       writel_relaxed(omm->cr, omm->io_base + OMM_CR);
> >
> >       ret = stm32_omm_set_amcr(dev, true);
> > +     if (ret && (mux & CR_MUXEN))
> > +             stm32_omm_toggle_child_clock(dev, false);
>
> How does it work without st,omm-mux?
>
> >
> >  error:
> >       pm_runtime_put_sync_suspend(dev);
>
>
> Best regards,
> Krzysztof

Re: [PATCH] memory: stm32_omm: fix child clock leak on set_amcr() error path

[Krzysztof Kozlowski]

On 23/09/2026 18:35, Diego Fernando Mancera Gomez wrote:
> Without st,omm-mux, of_property_read_u32() fails and leaves mux at its
> initial value of 0, so the "if (mux & CR_MUXEN)" block is skipped and
> the child clocks are never enabled in stm32_omm_configure(). The new
> check is then false and nothing is disabled on the set_amcr() error
> path, so there is nothing to leak and no unbalanced
> clk_disable_unprepare().
> 
> The cleanup uses exactly the same condition that gated
> stm32_omm_toggle_child_clock(dev, true), so this also covers the case
> where st,omm-mux is present but CR_MUXEN is not set. The earlier
> "goto error" paths are all taken before the clocks are enabled, or
> from toggle_child_clock() itself, which already unwinds on failure.
> 
> If it helps, I can send a v2 that states this in the commit message.

Oohh, I see... I also see simple code paths were made here originally
complicated. To figure out if the cleanup has to happen or not, one has
to follow three places/variables. ret, mux & CR_MUXEN and where the mux
is even initialized.

Side note, feels like you used LLM to reply to me. This reply is exactly
what LLMs are sending to us. 100%. If that happened, then be careful not
to ever repeat that. I don't want to talk with someone's LLM through
that person.

> 
> Best regards,
> Diego
> 
> 
> On Wed, Sep 23, 2026 at 8:54 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 18/09/2026 01:59, Diego Fernando Mancera Gomez wrote:
>>> When the mux is enabled, stm32_omm_configure() enables the two OSPI
>>> child clocks via stm32_omm_toggle_child_clock(dev, true); they are meant
>>> to stay enabled for the device's lifetime and are disabled again in
>>> stm32_omm_remove() and on the devm_of_platform_populate() error path.
>>>
>>> However, if the subsequent stm32_omm_set_amcr() call fails, the function
>>> jumps to the "error:" label which only calls pm_runtime_put_sync_suspend()
>>> before returning, leaving the two child clocks enabled. As this happens
>>> during probe, the error is propagated and .remove() is never called, so
>>> the clock enable references are leaked. devm_clk_bulk_get() only releases
>>> the clock handles on unwind, it does not undo clk_prepare_enable().
>>>
>>> Disable the child clocks on the set_amcr() error path, mirroring the
>>> cleanup already done on the devm_of_platform_populate() failure path.
>>>
>>> Fixes: 8181d061dcff ("memory: Add STM32 Octo Memory Manager driver")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Diego Fernando Mancera Gomez <diegomancera.dev@gmail.com>
>>> ---
>>>  drivers/memory/stm32_omm.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/memory/stm32_omm.c b/drivers/memory/stm32_omm.c
>>> index 84eb23c824f5..3599683ab04a 100644
>>> --- a/drivers/memory/stm32_omm.c
>>> +++ b/drivers/memory/stm32_omm.c
>>> @@ -274,6 +274,8 @@ static int stm32_omm_configure(struct device *dev)
>>>       writel_relaxed(omm->cr, omm->io_base + OMM_CR);
>>>
>>>       ret = stm32_omm_set_amcr(dev, true);
>>> +     if (ret && (mux & CR_MUXEN))
>>> +             stm32_omm_toggle_child_clock(dev, false);
>>
>> How does it work without st,omm-mux?
>>
>>>
>>>  error:
>>>       pm_runtime_put_sync_suspend(dev);
>>
>>
>> Best regards,
>> Krzysztof


Best regards,
Krzysztof

Re: [PATCH] memory: stm32_omm: fix child clock leak on set_amcr() error path

[Krzysztof Kozlowski]

On Thu, 17 Sep 2026 17:59:25 -0600, Diego Fernando Mancera Gomez wrote:
> When the mux is enabled, stm32_omm_configure() enables the two OSPI
> child clocks via stm32_omm_toggle_child_clock(dev, true); they are meant
> to stay enabled for the device's lifetime and are disabled again in
> stm32_omm_remove() and on the devm_of_platform_populate() error path.
> 
> However, if the subsequent stm32_omm_set_amcr() call fails, the function
> jumps to the "error:" label which only calls pm_runtime_put_sync_suspend()
> before returning, leaving the two child clocks enabled. As this happens
> during probe, the error is propagated and .remove() is never called, so
> the clock enable references are leaked. devm_clk_bulk_get() only releases
> the clock handles on unwind, it does not undo clk_prepare_enable().
> 
> [...]

Applied, thanks!

[1/1] memory: stm32_omm: fix child clock leak on set_amcr() error path
      https://git.kernel.org/krzk/linux-mem-ctrl/c/a22355280361d2a376a2020059a2bae11e6cea10

Best regards,
-- 
Krzysztof Kozlowski <krzk@kernel.org>