[PATCH v2 0/2] i2c: designware: Fixes clock issues with platform driver

[PATCH v2 0/2] i2c: designware: Fixes clock issues with platform driver

[Kunihiko Hayashi]

This series fixes two clock issues with i2c-designware-platdrv driver.

The first is that the clocks aren't disabled when the driver exits if
CONFIG_PM=n, the other is that the clocks aren't disabled when the probe
function fails.

Changes since v1:
- Fix the description about pm_runtime_put_noidle()

Kunihiko Hayashi (2):
  i2c: designware: Fix clock issue when PM is disabled
  i2c: designware: Add disabling clocks when probe fails

 drivers/i2c/busses/i2c-designware-platdrv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.34.1

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

[Kunihiko Hayashi]

When removing the driver, enable the clocks once by calling
pm_runtime_get_sync(), and call pm_runtime_put_sync() to disable
the clocks.

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")
Co-developed-by: Kohei Ito <ito.kohei@socionext.com>
Signed-off-by: Kohei Ito <ito.kohei@socionext.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d6e1ee935399..edaebfb165f9 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -325,9 +325,11 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
 	i2c_dw_disable(dev);
 
 	pm_runtime_dont_use_autosuspend(device);
-	pm_runtime_put_sync(device);
+	pm_runtime_put_noidle(device);
 	dw_i2c_plat_pm_cleanup(dev);
 
+	i2c_dw_prepare_clk(dev, false);
+
 	i2c_dw_remove_lock_support(dev);
 
 	reset_control_assert(dev->rst);
-- 
2.34.1

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

[Jarkko Nikula]

Hi

On 7/24/25 7:22 AM, 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.
> 
> 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")
> Co-developed-by: Kohei Ito <ito.kohei@socionext.com>
> Signed-off-by: Kohei Ito <ito.kohei@socionext.com>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>   drivers/i2c/busses/i2c-designware-platdrv.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
Good catch!

Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

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

[Andi Shyti]

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

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

[PATCH v2 2/2] i2c: designware: Add disabling clocks when probe fails

[Kunihiko Hayashi]

After an error occurs during probing state, dw_i2c_plat_pm_cleanup() is
called. However, this function doesn't disable clocks and the clock-enable
count keeps increasing. Should disable these clocks explicitly.

Co-developed-by: Kohei Ito <ito.kohei@socionext.com>
Signed-off-by: Kohei Ito <ito.kohei@socionext.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index edaebfb165f9..f6424dcfdff6 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -308,6 +308,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 
 exit_probe:
 	dw_i2c_plat_pm_cleanup(dev);
+	i2c_dw_prepare_clk(dev, false);
 exit_reset:
 	reset_control_assert(dev->rst);
 	return ret;
-- 
2.34.1

Re: [PATCH v2 2/2] i2c: designware: Add disabling clocks when probe fails

[Jarkko Nikula]

Hi

On 7/24/25 7:22 AM, Kunihiko Hayashi wrote:
> After an error occurs during probing state, dw_i2c_plat_pm_cleanup() is
> called. However, this function doesn't disable clocks and the clock-enable
> count keeps increasing. Should disable these clocks explicitly.
> 
> Co-developed-by: Kohei Ito <ito.kohei@socionext.com>
> Signed-off-by: Kohei Ito <ito.kohei@socionext.com>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>   drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index edaebfb165f9..f6424dcfdff6 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -308,6 +308,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>   
>   exit_probe:
>   	dw_i2c_plat_pm_cleanup(dev);
> +	i2c_dw_prepare_clk(dev, false);
>   exit_reset:
>   	reset_control_assert(dev->rst);
>   	return ret;

Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>