Re: [PATCH 0/2 v3] Add pl031 RTC support for Hi6220

[Michael Turquette]

Quoting Arnd Bergmann (2016-07-07 01:22:30)
> On Wednesday, July 6, 2016 5:58:14 PM CEST John Stultz wrote:
> > On Wed, Jul 6, 2016 at 12:38 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
> > > On Wednesday, July 6, 2016 12:20:15 AM CEST John Stultz wrote:
> > >> On Wed, Jul 6, 2016 at 12:04 AM, Olof Johansson <olof@xxxxxxxxx> wrote:
> > >> > On Tue, Jul 5, 2016 at 11:55 PM, John Stultz <john.stultz@xxxxxxxxxx> wrote:
> > 
> > Though this seemingly goes against the otherwise widely recommended
> > approach of breaking up patches into small obvious chunks.
> > 
> > And personally, and I don't mean to criticize, but the suggestions
> > here (use numerical values, then later rename to macros; add
> > everything in one go, then make dts changes a release later) all seem
> > like non-optimal workarounds for the fact that adding almost any
> > functionality requires cross subsystem-maintainer negotiations (or two
> > release steps to get one bit of functionality merged).
> > 
> > It seems like it might even just be clearer to make the
> > two-release-steps method the widely broadcast rule (ie: no
> > dependencies on in-flight patches for dts changes), so this doesn't
> > confuse/dismay new developers.
> > 
> > Anyway... In this case, I don't have the clk documentation, so I'll
> > ping Zhangfei to check if there is any other clock values that should
> > be added in the future, but at least for HiKey, while there are still
> > a few clk patches remaining in the tree, I don't have any more
> > additions to the clk list.
> 
> I think the main underlying problem is hardware that is so badly
> structured that there is no way to describe it other than to enumerate
> each output in a header file and have a separate handler in the driver
> for it.
> 
> We typically have it easier for other subsystems like irqchip or gpio
> where nobody would consider writing a driver that can only handle
> the I/O lines that are used on their board with a minimal set of
> drivers, but for some reason it seems acceptable to do it for clock
> controllers just because they are harder to describe.

gpio and irqchip are interesting analogues. It makes pretty good sense
to expose all of those lines via DT, since those are resources that
consumer drivers may be interested in. But is the same true for clock
signals?

Clearly drivers will care about their input clocks, which are often leaf
gates. But the mess and tangle of "root" and "branch" clocks above that?
Why expose it to DT if we don't need to? These are resources that are
often internal to the clock controller IP block. In an ideal world we
would never need to provide a way for clock consumer drivers to get at
these root and branch clocks, just the peripheral leaf clocks.

As an example of this, ccf has tried to be smart about propagating rate
requests up the chain of parents since it was originally merged, and
that directly has led to lots of consolidation around the cpufreq-dt.c
driver, where a single leaf clock can ultimately affect a PLL or
post-divider without the cpufreq driver needing to know the details of
the clock hierarchy internal to the clock controller IP block.

(in reality we do need to expose root and branch clocks for drivers some
times, but I disagree that we should expose every single clock signal
just because it is there)

> 
> For the common case where the driver developer actually has a
> description of the clock controller hardware in a manual, I see
> no reason not to implement the complete driver right away.

That case is less common than you might think.

Regards,
Mike

> 
>         Arnd