Re: [PATCH] i2c: jz4780: Cache clock rate at probe to prevent CCF prepare_lock deadlock

[H. Nikolaus Schaller]

> Am 19.07.2026 um 17:07 schrieb Paul Cercueil <paul@xxxxxxxxxxxxxxx>:
> 
> Hi Nikolaus,
> 
> Le vendredi 10 juillet 2026 à 08:58 +0200, H. Nikolaus Schaller a
> écrit :
>> Fix a severe AB/BA deadlock between the Common Clock Framework (CCF)
>> and the I2C adapter lock, which triggers when an I2C-controlled clock
>> generator (like the Si5351) is registered or modified under the CCF.
>> 
>> During a clock frequency change, the CCF acquires its global
>> 'prepare_lock'
>> mutex and calls i2c_transfer() to update the chip registers, stalling
>> for the adapter's I2C bus lock.
>> 
>> Concurrently, a parallel transfer on the same bus (e.g., a GPIO
>> expander
>> handling LEDs) can hold the I2C adapter lock. Inside the transfer
>> path,
>> jz4780_i2c_set_speed() calls clk_get_rate() to dynamically calculate
>> timings. This call attempts to acquire the blocked CCF
>> 'prepare_lock',
>> creating a circular dependency that freezes the system or at least
>> the involved processes and workers.
>> 
>> Eliminate the synchronous clk_get_rate() call from the active
>> transfer
>> path by caching the peripheral clock rate once - inside the private
>> jz4780_i2c structure during jz4780_i2c_probe(). Update
>> jz4780_i2c_set_speed() to use this cached value, decoupling active
>> I2C
>> transactions from the CCF internal locks.
>> 
>> Assisted-by web based Google AI.
>> 
>> Fixes: ba92222ed63a12 ("i2c: jz4780: Add i2c bus controller driver
>> for Ingenic JZ4780")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
>> ---
>>  drivers/i2c/busses/i2c-jz4780.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/i2c/busses/i2c-jz4780.c
>> b/drivers/i2c/busses/i2c-jz4780.c
>> index 664a5471d9335..d729cec9cdf2c 100644
>> --- a/drivers/i2c/busses/i2c-jz4780.c
>> +++ b/drivers/i2c/busses/i2c-jz4780.c
>> @@ -141,6 +141,7 @@ struct jz4780_i2c {
>>   void __iomem *iomem;
>>   int irq;
>>   struct clk *clk;
>> + unsigned long clk_rate;
> 
> That's a weird spacing. Should probably be "unsigned long" on the left
> side.

indeed a typo - should be

TAB unsigned long TAB clk_rate;

Interestingly checkpatch.pl did not complain...

> 
>>   struct i2c_adapter adap;
>>   const struct ingenic_i2c_config *cdata;
>>  
>> @@ -246,7 +247,7 @@ static int jz4780_i2c_set_target(struct
>> jz4780_i2c *i2c, unsigned char address)
>>  
>>  static int jz4780_i2c_set_speed(struct jz4780_i2c *i2c)
>>  {
>> - int dev_clk_khz = clk_get_rate(i2c->clk) / 1000;
>> + int dev_clk_khz = i2c->clk_rate / 1000;
> 
> You cache `i2c->clk_rate` but only use it divided by 1000, so maybe
> cache `clk_rate_khz` instead?

Yes, that is more precise.

I can update and send a V2 with a revised commit message.

BR and thanks,
Nikolaus