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

Paul Cercueil <[email protected]>
Newsgroups org.kernel.vger.linux-i2c,org.kernel.vger.linux-kernel,org.kernel.vger.linux-mips,org.kernel.vger.stable
Message-ID <[email protected]>
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: [email protected]
> Signed-off-by: H. Nikolaus Schaller <[email protected]>
> ---
>  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.

>  	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?

Cheers,
-Paul

>  	int cnt_high = 0;	/* HIGH period count of the SCL
> clock */
>  	int cnt_low = 0;	/* LOW period count of the SCL clock
> */
>  	int cnt_period = 0;	/* period count of the SCL clock */
> @@ -796,6 +797,8 @@ static int jz4780_i2c_probe(struct
> platform_device *pdev)
>  	if (IS_ERR(i2c->clk))
>  		return PTR_ERR(i2c->clk);
>  
> +	i2c->clk_rate = clk_get_rate(i2c->clk);
> +
>  	ret = of_property_read_u32(pdev->dev.of_node, "clock-
> frequency",
>  				   &clk_freq);
>  	if (ret) {
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.