Re: [PATCH v2 1/2] i2c: designware: Fix clock issue when PM is disabled

[Kunihiko Hayashi]

Hi Andi,

Thank you for your comment.

On 2025/08/20 7:06, Andi Shyti wrote:
> Hi Kunihiko,
>
> On Thu, Jul 24, 2025 at 01:22:10PM +0900, Kunihiko Hayashi wrote:
> > When removing the driver, enable the clocks once by calling
> > pm_runtime_get_sync(), and call pm_runtime_put_sync() to disable
> > the clocks.
>
> can we rephrase this to something like:
>
>    When the driver is removed, the clocks are first enabled by
>    calling pm_runtime_get_sync(), and then disabled with
>    pm_runtime_put_sync().
>
> Does it work?

Yes, your description is better.

> > If CONFIG_PM=y, clocks for this controller are disabled when it's in
> > the idle state. So the clocks are properly disabled when the driver
> > exits.
> >
> > Othewise, the clocks are always enabled and the PM functions have
> > no effect. Therefore, the driver exits without disabling the clocks.
> >
> >      # cat /sys/kernel/debug/clk/clk-pclk/clk_enable_count
> >      18
> >      # echo 1214a000.i2c > /sys/bus/platform/drivers/i2c_designware/bind
> >      # cat /sys/kernel/debug/clk/clk-pclk/clk_enable_count
> >      20
> >      # echo 1214a000.i2c >
> /sys/bus/platform/drivers/i2c_designware/unbind
> >      # cat /sys/kernel/debug/clk/clk-pclk/clk_enable_count
> >      20
> >
> > To ensure that the clocks can be disabled correctly even without
> > CONFIG_PM=y, should add the following fixes:
> >
> > - Replace with pm_runtime_put_noidle(), which only decrements the
> runtime
> >    PM usage count.
> > - Call i2c_dw_prepare_clk(false) to explicitly disable the clocks.
> >
> > Fixes: 7272194ed391f ("i2c-designware: add minimal support for runtime
> PM")
>
> This commit doesn't look quite right to me, although it's quite
> difficult to find the culprit, as the clk api's have changed a
> lot over time.
>
> Do you think this is better:
>
> Fixes: b33af11de236 ("i2c: designware: Do not require clock when SSCN and
> FFCN are provided")

Surely, the code that handles PM and clocks has changed some time,
so it's difficult to identify the commit with "Fixes" tag.
I think the first commit that called pm_runtime_put_sync() was the cause.

> It doesn't matter much as it won't apply in any of the two
> versions> 
> The Fixes tag should be added in both the patches, but there is
> no need to resend, I will apply them once we agree on the commit
> message.

Okay, I agree with you.

Thank you,

---
Best Regards
Kunihiko Hayashi