Re: [PATCH v3] clk: Add Si5341/Si5340 driver

[Stephen Boyd <sboyd@kernel.org>]

Quoting Mike Looijmans (2019-06-27 23:42:03)
> On 27-06-19 23:06, Stephen Boyd wrote:
> > Quoting Mike Looijmans (2019-05-17 06:23:52)
> >> Adds a driver for the Si5341 and Si5340 chips. The driver does not fully
> >> support all features of these chips, but allows the chip to be used
> >> without any support from the "clockbuilder pro" software.
> >>
> >> If the chip is preprogrammed, that is, you bought one with some defaults
> >> burned in, or you programmed the NVM in some way, the driver will just
> >> take over the current settings and only change them on demand. Otherwise
> >> the input must be a fixed XTAL in its most basic configuration (no
> >> predividers, no feedback, etc.).
> >>
> >> The driver supports dynamic changes of multisynth, output dividers and
> >> enabling or powering down outputs and multisynths.
> >>
> >> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> >> ---
> > 
> > Applied to clk-next + some fixes. I'm not super thrilled about the kHz
> > thing but we don't have a solution for it right now so might as well
> > come back to it later.
> 
> Thanks for the fixes. And I'm not exactly proud of that kHz part either.
> 
> While thinking about a solution, I've also had a use case for less than 1Hz 
> frequency adjustment (a video clock to "follow" another one). These clock 
> generators allow for ridiculous ranges and accuracy, you can request it to 
> generate a 200000000.0005 Hz clock.
> 

Right. We need to make a plan to replace unsigned long with u64 in the
clk framework and then figure out how to support whatever use-cases we
can with the extra 32-bits we get on the 32-bit unsigned long platforms.
I had a patch lying around that started to plumb u64 through the core
clock framework code, but I didn't pursue it because it didn't seem
necessary. I've seen some code for display PLLs that need to support
10GHz frequencies for display port too, so you're not alone here. 

Some questions to get the discussion going:

 1. Do we need to use the clk framework to set these frequencies or can
 it be done via other means in whatever subsystem wants to program these
 frequencies, like a broadcast TV tuner subsystem or the IIO subsystem?

 2. If clk framework must handle these frequencies, does it need to be
 set through the clk consumer APIs or can we manage to set the rates on
 these clks via child dividers, muxes, etc. that have
 CLK_SET_RATE_PARENT flag? This might avoid changing the consumer API
 and be simpler to implement.

 3. What's the maximum frequency and the highest resolution we need to
 support? Maybe we just need to support GHz and not THz (10^12) and have
 a resolution of uHz (micro-Hertz)?

 4. Not really a question, but a goal. We should try to avoid a
 performance hit due to an increase in 64-bit math. If possible we can
 do things differently on different CPU architectures to achieve this or
 we can have the clk providers use different clk ops/flags to indicate
 the max range and precision they require.

Anyway, I'm not going to be working on this topic anytime soon but these
are my rough thoughts. I'm sure others on the list have thought about
this topic too so if you want to work on this then it would be good to
float an RFC that answers these questions.