Re: [PATCH] i2c: bcm2835: Model Divider in CCF

From: Annaliese McDermond <nh6z@nh6z.net>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Stefan Wahren <stefan.wahren@i2se.com>,
	wsa@the-dreams.de, team@nwdigitalradio.com, eric@anholt.net,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org
Subject: Re: [PATCH] i2c: bcm2835: Model Divider in CCF
Date: Sun, 5 May 2019 10:21:49 -0700	[thread overview]
Message-ID: <362948E6-B012-4F3D-89B7-396141C7E596@nh6z.net> (raw)
In-Reply-To: <c43eb848-23cd-4b0c-638d-3bb0df58ed31@gmail.com>

> On May 5, 2019, at 10:13 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
> 
> 
> On 5/5/2019 3:36 AM, Stefan Wahren wrote:
>> Hi Annaliese,
>> 
>> Am 05.05.19 um 05:43 schrieb Annaliese McDermond:
>>> Model the I2C bus clock divider as a part of the Core Clock Framework.
>>> Primarily this removes the clk_get_rate() call from each transfer.
>>> This call causes problems for slave drivers that themselves have
>>> internal clock components that are controlled by an I2C interface.
>>> When the slave's internal clock component is prepared, the prepare
>>> lock is obtained, and it makes calls to the I2C subsystem to
>>> command the hardware to activate the clock.  In order to perform
>>> the I2C transfer, this driver sets the divider, which requires
>>> it to get the parent clock rate, which it does with clk_get_rate().
>>> Unfortunately, this function will try to take the clock prepare
>>> lock, which is already held by the slave's internal clock calls
>>> creating a deadlock.
>> 
>> i think i understand the problem, but could you please explain the
>> specific use case where this happend?
>> 
>> I suspect bcm2835 is not the only platform which is affected, so it
>> would be better to fix this in general.
> 
> Agreed, if you could identify i2c bus drivers that support changing the
> bus clock and move the registration of the bus 'struct clk' into the i2c
> core that would scale a lot better.

This isn’t necessarily because of changing the bus clock.  This is
because one of the parents of the bus clock could change.  This necessitates
resetting the divider feeding the bus clock of the i2c device so that
it still produces the desired frequency.

I would imagine that not every device has an internal divider that helps
create the bus frequency.  I could very well be wrong.

> -- 
> Florian

--
Annaliese McDermond
nh6z@nh6z.net
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel