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

[Masahiro Yamada]

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.

-- 
Best Regards
Masahiro Yamada