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

[Annaliese McDermond <nh6z@nh6z.net>]

Wolfram --

Thank you very much for your response.

> On May 16, 2019, at 12:58 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> 
> Hi Annaliese,
> 
> On Wed, May 15, 2019 at 10:37:03PM -0700, Annaliese McDermond wrote:
>> I’m just following up on this since I haven’t heard anything since I submitted the
>> v2 patch a week ago.  Wolfram, does this look like a sane approach?  Stefan,
>> were my changes in v2 acceptable?
> 
> There is a bit of overhead involved, but conceptually it looks like an
> elegant solution to me. However, I am not an expert of CCF. Grepping
> through kernel sources, I don't see many clocks defined outside
> drivers/clk. So, I'd appreciate if we could get some ack/feedback from
> one of the CCF experts/maintainers.

I was also similarly nervous about a clock provider being outside of 
drivers/clk, especially since one of the instances of that I wrote.

When writing this, there was a certain logic to putting this inside of
clk-bcm2835.c instead.  Eric may like this approach better because there
will probably be more code reuse of some of the dividers he uses in that
driver.  

My concern was surrounding the cross pollenation of the i2c and
clock drivers to a certain extent.  You have to be able to set the
FEDL_SHIFT and REDL_SHIFT registers when you change the clock rate.
You can’t do it in the transfer function like was done before because
it requires a call to clk_get_rate() which can deadlock in the
conditions I encountered.  You’d have to move those calls to the
clk-bcm2835 driver instead.

My nervousness surrounding moving the code into the clock driver
rather than the i2c driver is that I don’t know how dependent they
are on one another.  Do we anticipate that there may be a use for
the i2c-bcm2835 driver without the clk-bcm2835 driver?  Asking the
question very well may be premature optimization for a condition
that may never exist.  I’m thinking maybe Eric has some more
insight into this?

The other minor issue is that it’s going to require a change to the
bcm2835 DT to change the clock pointer to the divider.

If it fits better in the clk-bcm2835 driver, I’m happy to submit a
patch that puts it there.  Just let me know and I can work on it.

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