Re: [PATCH] i2c: stm32f7: Do not prepare/unprepare clock during runtime suspend/resume

[Marek Vasut <marex@denx.de>]

On 9/30/24 8:38 PM, Alain Volmat wrote:
> Hi Marek,

Hello Alain,

> thanks a lot for the patch.
> 
> On Sat, Sep 28, 2024 at 03:43:46AM +0200, Marek Vasut wrote:
>> In case there is any sort of clock controller attached to this I2C bus
>> controller, for example Versaclock or even an AIC32x4 I2C codec, then
>> an I2C transfer triggered from the clock controller clk_ops .prepare
>> callback may trigger a deadlock on drivers/clk/clk.c prepare_lock mutex.
>>
>> This is because the clock controller first grabs the prepare_lock mutex
>> and then performs the prepare operation, including its I2C access. The
>> I2C access resumes this I2C bus controller via .runtime_resume callback,
>> which calls clk_prepare_enable(), which attempts to grab the prepare_lock
>> mutex again and deadlocks.
>>
>> Since the clock are already prepared since probe() and unprepared in
>> remove(), use simple clk_enable()/clk_disable() calls to enable and
>> disable the clock on runtime suspend and resume, to avoid hitting the
>> prepare_lock mutex.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Alain Volmat <alain.volmat@foss.st.com>
>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
>> Cc: Andi Shyti <andi.shyti@kernel.org>
>> Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> Cc: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>
>> Cc: kernel@dh-electronics.com
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-i2c@vger.kernel.org
>> Cc: linux-stm32@st-md-mailman.stormreply.com
>> ---
>>   drivers/i2c/busses/i2c-stm32f7.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
>> index cfee2d9c09de3..65c035728a4fa 100644
>> --- a/drivers/i2c/busses/i2c-stm32f7.c
>> +++ b/drivers/i2c/busses/i2c-stm32f7.c
>> @@ -2395,7 +2395,7 @@ static int __maybe_unused stm32f7_i2c_runtime_suspend(struct device *dev)
>>   	struct stm32f7_i2c_dev *i2c_dev = dev_get_drvdata(dev);
>>   
>>   	if (!stm32f7_i2c_is_slave_registered(i2c_dev))
>> -		clk_disable_unprepare(i2c_dev->clk);
>> +		clk_disable(i2c_dev->clk);
>>   
>>   	return 0;
>>   }
>> @@ -2406,7 +2406,7 @@ static int __maybe_unused stm32f7_i2c_runtime_resume(struct device *dev)
>>   	int ret;
>>   
>>   	if (!stm32f7_i2c_is_slave_registered(i2c_dev)) {
>> -		ret = clk_prepare_enable(i2c_dev->clk);
>> +		ret = clk_enable(i2c_dev->clk);
>>   		if (ret) {
>>   			dev_err(dev, "failed to prepare_enable clock\n");
> 
> The call now being clk_enable, could you also change the error message
> from prepare_enable to enable ?
> 
> With that done,
> Acked-by: Alain Volmat <alain.volmat@foss.st.com>
Fixed in V2, thanks !