Annaliese McDermond <nh6z@xxxxxxxx> writes: >> On May 6, 2019, at 11:14 AM, Eric Anholt <eric@xxxxxxxxxx> wrote: >> >> Annaliese McDermond <nh6z@xxxxxxxx> writes: >> >>> 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. >>> >>> Modeling the divider in the CCF natively removes this dependency >>> and the divider value is only set upon changing the bus clock >>> frequency or changes in the parent clock that cascade down to this >>> divisor. This obviates the need to set the divider with every >>> transfer and avoids the deadlock described above. It also should >>> provide better clock debugging and save a few cycles on each >>> transfer due to not having to recalcuate the divider value. >> >> Any chance we could reuse clk_register_divider() instead of having our >> own set/round/recalc rate implementations? > > Eric -- > > I’d love to, but the set_rate implementation includes setting the > BCM2835_I2C_FEDL_SHIFT and BCM2835_I2C_REDL_SHIFT registers for the > rising and falling edge delay on the I2C bus based on what the divider > value is. Hmm. I ran into that in clk-bcm2835.c as well, and the solution was that bcm2835_register_pll_divider() sets up the divider structure and then reuses clk_divider_ops.round_rate() and .recalc_rate()
Attachment:
signature.asc
Description: PGP signature
