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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <wnoawxh7umtbq2b73f4qpbbbe76b5eluz62g3csmq7k4uw2hnm@exggtdzkeyur>
Date: Tue, 19 Aug 2025 21:06:14 -0100
From: Andi Shyti <andi.shyti@...nel.org>
To: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
Cc: Jarkko Nikula <jarkko.nikula@...ux.intel.com>, 
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Mika Westerberg <mika.westerberg@...ux.intel.com>, 
	Jan Dabros <jsd@...ihalf.com>, Kohei Ito <ito.kohei@...ionext.com>, linux-i2c@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] i2c: designware: Fix clock issue when PM is
 disabled

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?

> 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")

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.

Andi