[U-Boot] [RFC PATCH 6/6] clk: add fixed rate clock driver

[Simon Glass]

Hi Masahiro,

On 18 January 2016 at 22:15, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> 2015-12-28 23:20 GMT+09:00 Simon Glass <sjg at chromium.org>:
>> Hi Masahiro,
>>
>> On 18 December 2015 at 04:15, Masahiro Yamada
>> <yamada.masahiro at socionext.com> wrote:
>>> This commit intends to implement "fixed-clock" as in Linux.
>>> (drivers/clk/clk-fixed-rate.c in Linux)
>>>
>>> If you need a very simple clock to just provide fixed clock rate
>>> like a crystal oscillator, you do not have to write a new driver.
>>> This driver can support it.
>>>
>>> Note:
>>> As you see in dts/ directories, fixed clocks are often collected in
>>> one container node like this:
>>>
>>>   clocks {
>>>           refclk_a: refclk_a {
>>>                   #clock-cells = <0>;
>>>                   compatible = "fixed-clock";
>>>                   clock-frequency = <10000000>;
>>>           };
>>>
>>>           refclk_b: refclk_b {
>>>                   #clock-cells = <0>;
>>>                   compatible = "fixed-clock";
>>>                   clock-frequency = <20000000>;
>>>           };
>>>   };
>>>
>>> This does not work in the current DM of U-Boot, unfortunately.
>>> The "clocks" node must have 'compatible = "simple-bus";' or something
>>> to bind children.
>>
>> I suppose we could explicitly probe the children of the 'clocks' node
>> somewhere. What do you suggest?
>>
>>>
>>> Most of developers would be unhappy about adding such a compatible
>>> string only in U-Boot because we generally want to use the same set
>>> of device trees beyond projects.
>>
>> I'm not sure we need to change it, but if we did, we could try to
>> upstream the change.
>>
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com>
>>> ---
>>>
>>> I do not understand why we need both .get_rate and .get_periph_rate.
>>>
>>> I set both in this driver, but I am not sure if I am doing right.
>>
>> This is to avoid needing a new clock device for every single clock
>> divider in the SoC. For example, it is common to have a PLL be used by
>> 20-30 devices. In U-Boot we can encode the device number as a
>> peripheral ID, Then we can adjust those dividers by settings the
>> clock's rate on a per-peripheral basis. Thus we need only one clock
>> device instead of 20-30.
>>
>> In the case of your clock I think you could return -ENOSYS for
>> get_periph_rate().
>
> I've just posted v2.
>
> I am still keeping both .get_rate() and .get_periph_rate().
>
> If I follow your suggestion, each clock consumer must know the
> detail of its clock providers to choose the correct one,
> either .get_rate() or .get_periph_rate().
>
> Or do you want drivers to do like this?
>
>
>
>     clock_cells = clk_get_nr_cells(...);
>
>     if (clock_cells == 0)
>           rate = clk_get_rate(...);
>     else
>           rate = clk_get_periph_rate(...);
>
>
>
> For the proper use of these two, the details of clocks must be
> hard-coded in drivers.
> They, of course should be describe in device tree and clock providers.

In general drivers don't use PLLs directly. So I doubt this case will
arise. Do you have an example?

Regards,
Simon