> On May 6, 2019, at 9:32 PM, Eric Anholt <eric@xxxxxxxxxx> wrote: > > Annaliese McDermond <nh6z@xxxxxxxx> writes: > >>> On May 6, 2019, at 11:14 AM, Eric Anholt <eric@xxxxxxxxxx> wrote: >>> >>> 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() I’m not sure this makes a lot of sense in this particular case. I’d still have to keep the bcm2835_i2c_register_div() function, and really I’d only be saving having to implement round_rate() and recalc_rate(). The tradeoff is that the common round_rate and recalc rate are much more complex and require a more complex private structure (clk_divider) which also precludes my use of the common bcm2835_i2c_writel() used in the rest of the driver because I no longer have a pointer to the bcm2835_i2c_dev structure that I need to call it in set_rate(). I get the desire to reuse code and to use common structures whenever possible. It just seems to me that going down this path leads to more overall code that’s less straight forward. Maybe I’m wrong. -- Annaliese McDermond nh6z@xxxxxxxx
