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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
>>> Signed-off-by: Diego Fernando Mancera Gomez <diegomancera.dev@xxxxxxxxx>
>>> ---
>>>  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