Wolfram -- Thank you very much for your response. > On May 16, 2019, at 12:58 AM, Wolfram Sang <wsa@xxxxxxxxxxxxx> 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@xxxxxxxx
