[U-Boot] [PATCH v2 07/22] clock-uclass: allow disabling a peripheral clock

[Benjamin Tietz <uboot@dresden.micronet24.de>]

Hello Stephen,

On Fri, Jul 29, 2016 at 10:04:56AM -0600, Stephen Warren wrote:
> On 07/28/2016 01:28 PM, Benjamin Tietz wrote:
> >Hello Vikas, hello Simon,
> >
> >the new clk-API leaves me with a problem. Previously there was a
> >seperate way to access the clock-device itself (using clk_[gs]et_rate) and
> >the peripherals connected (clk_[gs]et_periph_rate). The former case now isn't
> >available no more. But the system clock in STM32 doesn't have a separate ID.
> >According to the device-tree the kernel doesn't care about that - or
> >uses special logic.
> 
> I don't understand the issue here. The device tree model is that every clock
> has some provider (a node/phandle) and some ID (a sequence of integer
> values). There's no such thing as "the clock-device itself".

For the current STM32 DT exactly that is the problem. The phandle is
available and the set of IDs is defined. There is - at least at the
moment - no ID defined for the system clock itself, but only for the
derived clocks for the individual peripherals.

The existing peripherals IDs even start to count at 0, so the "intuitive"
solution to use that ID, doesn't work either.

> 
> The kernel is consistent with that model; each client executes clk_get(),
> which for a DT-based system parses the phandle+clock_specifier and returns a
> clock handle, and then the client just uses that handle. That's exactly how
> U-Boot works too.

I must admit, that I haven't had an in-depth look on the STM32 clocking
kernel sources yet. From other architectures I've seen, the system clock
is either accessed by a certain ID, based on the underlying hardware -
which isn't available on STM32 - or assumed to be "just there". On a
first glance, the kernel STM32 driver seems to fall into the second category.

> 
> Perhaps you can show the portion of DT that's causing the issue?

It is the not existing potion of the DT ;)

> 
> Is the issue that there are clocks that your code needs to use that haven't
> yet been assigned a clock ID (clock specifier value) for use in DT (i.e. you
> have SoC-specific code that's calling clk_get() and the mapping isn't via
> DT)? If so, you simply need to assign one and everything will work fine.
> High numbers work fine for this.
> 
> >Possible solutions:
> >
> >a) Using a magic ID. Unfortunately, the peripheral used in the current
> >device-tree are 0-based (and 0 is already in use), so this number isn't
> >available. Moving the IDs around would break compatibility to the linux
> >kernel.
> >
> >Negative numbers look like errors.
> >
> >Using a special high number looks unintuitive. And often result in
> >additional work-arounds in the future.
> 
> What specific issues are you thinking of? I haven't experienced any when
> assigning IDs on Tegra, and I haven't observed anyone having issues doing
> this on any platform within the Linux kernel, where the exact same thing
> would be required.

I'm no friend of magic numbers. Experience had shown, that - especially
those introduced as a work-around - generate complicate problems at a
later point of time.
The first thing I can think of is to run into a hardware, having more
peripheral clocks than expected. Nevertheless, in a follow-up mail I
made a suggestion interpreting part of the ID as a bit-field - with
enough room for bigger hardware.

regards
Benjamin