[PATCH v2 3/7] devicetree: bindings: add cpu clock configuration data binding for Exynos4/5

Fri Jan 24 10:24:04 EST 2014

Hi Rob,

On Tue, Jan 21, 2014 at 1:01 PM, Thomas Abraham <ta.omasab at gmail.com> wrote:
> Hi Rob,
>
>
> On Sat, Jan 18, 2014 at 8:52 PM, Rob Herring <robherring2 at gmail.com> wrote:
>> On Sat, Jan 18, 2014 at 6:10 AM, Thomas Abraham <ta.omasab at gmail.com> wrote:
>>> From: Thomas Abraham <thomas.ab at samsung.com>
>>>
>>> The clk ops of the new Samsung cpu clock provider type requires configuration
>>> data that will be programmed in the multiple clock blocks encapsulated within
>>> the cpu clock provider type. This configuration data is held in the clock
>>> controller node. Update clock binding documentation about this configuration
>>> data format for Samsung Exynos4 and Exynos5 platforms.
>>>
>>> Cc: Rob Herring <robh+dt at kernel.org>
>>
>> Please copy all maintainers.
>
> Okay.
>
>>
>>> Cc: Tomasz Figa <t.figa at samsung.com>
>>> Cc: <devicetree at vger.kernel.org>
>>> Signed-off-by: Thomas Abraham <thomas.ab at samsung.com>
>>> ---
>>>  .../devicetree/bindings/clock/exynos4-clock.txt    |   30 ++++++++++++++++++++
>>>  .../devicetree/bindings/clock/exynos5250-clock.txt |   21 ++++++++++++++
>>>  2 files changed, 51 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/exynos4-clock.txt b/Documentation/devicetree/bindings/clock/exynos4-clock.txt
>>> index a2ac2d9..c28aabd 100644
>>> --- a/Documentation/devicetree/bindings/clock/exynos4-clock.txt
>>> +++ b/Documentation/devicetree/bindings/clock/exynos4-clock.txt
>>> @@ -15,6 +15,29 @@ Required Properties:
>>>
>>>  - #clock-cells: should be 1.
>>>
>>> +- arm-frequency-table: defines the list of arm clock speeds supported and
>>
>> Seems like a Samsung specific property and nothing to do with ARM, so
>> it should be named accordingly.
>
> Okay.
>
>>
>>> +  the associated configuration values required to setup the clock controller
>>> +  for generating those speeds. The format of each entry included in the
>>> +  arm-frequency-table should be as defined below (#cells per entry = 13)
>>> +
>>> +  - for Exynos4210 and Exynos4212 based platforms:
>>> +      cell #1: arm clock frequency
>>> +      cell #2: expected arm clock parent frequency
>>> +      cell #3 ~ cell 12#: value of clock divider in the following order
>>> +              core_ratio, corem0_ratio, corem1_ratio, periph_ratio,
>>> +              atb_ratio, pclk_dbg_ratio, apll_ratio, core2_ratio,
>>> +              copy_ratio, hpm_ratio.
>>> +      cell #13: reserved (should be zero).
>>> +
>>> +  - for Exynos4412 based platforms:
>>> +      cell #1: arm clock frequency
>>> +      cell #2: expected arm clock parent frequency
>>> +      cell #3 ~ cell #13: value of clock divider in the following order
>>> +              core_ratio, corem0_ratio, corem1_ratio, periph_ratio,
>>> +              atb_ratio, pclk_dbg_ratio, apll_ratio, core2_ratio,
>>> +              copy_ratio, hpm_ratio, cores_ratio
>>
>> You don't need voltages? Are the h/w limitations really ratios or each
>> clock has a max frequency that must be maintained? I would expect the
>> latter and think it would be better to specify maximum frequencies of
>> each clock. Then you can calculate the dividers to keep frequencies in
>> range.
>
> Voltage is not needed here since the cpu clock speed is managed by the
> cpufreq driver which will take care of setting appropriate voltage.
> Any other users apart from cpufreq will have to setup the voltage
> appropriately.
>
> About the ratios, yes the user manual lists the optimal clock
> frequency for these clocks. As suggested, I will remove the ratio list
> for these clocks.

I tried the approach of listing the optimal frequencies of these
clocks in dt and then calculating a divider value. But it turns out
that the divider values for these clocks is dependent on the frequency
of the PLL from which they are derived (or the voltage applied to the
CPU clock domain). During tests, the system is unstable if divider
values are calculated using the optimal clock speed listed in the SoC
manual.

So the list of divider values as used in this series (and documented
above) is required. Can I continue to use the binding as documented
above?

Thanks,
Thomas.

>
>>
>> How will this scale to multi-cluster chips with different frequency ranges?
>
> Usually, each cluster would have its own independent clock input. So
> there will be two or more cpu clocks in multi-cluster chips. So the
> clocks can be independently programmed as required by the cluster.
>
>>
>> Rob
>>
>
> Thanks Rob for your review.
>
> Thomas.
>
>>> +
>>> +
>>>  The following is the list of clocks generated by the controller. Each clock is
>>>  assigned an identifier and client nodes use this identifier to specify the
>>>  clock which they consume. Some of the clocks are available only on a particular
>>> @@ -275,6 +298,13 @@ Example 1: An example of a clock controller node is listed below.
>>>                 compatible = "samsung,exynos4210-clock";
>>>                 reg = <0x10030000 0x20000>;
>>>                 #clock-cells = <1>;
>>> +
>>> +               arm-frequency-table = <1200000 1200000 0 3 7 3 4 1 7 0 5 0>,
>>> +                                     <1000000 1000000 0 3 7 3 4 1 7 0 4 0>,
>>> +                                     < 800000  800000 0 3 7 3 3 1 7 0 3 0>,
>>> +                                     < 500000  500000 0 3 7 3 3 1 7 0 3 0>,
>>> +                                     < 400000  400000 0 3 7 3 3 1 7 0 3 0>,
>>> +                                     < 200000  200000 0 1 3 1 1 1 0 0 3 0>;
>>>         };
>>>
>>>  Example 2: UART controller node that consumes the clock generated by the clock
>>> diff --git a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt
>>> index 72ce617..99eae9c 100644
>>> --- a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt
>>> +++ b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt
>>> @@ -13,6 +13,20 @@ Required Properties:
>>>
>>>  - #clock-cells: should be 1.
>>>
>>> +- arm-frequency-table: defines the list of arm clock speeds supported and
>>> +  the associated configuration values required to setup the clock controller
>>> +  for generating those speeds. The format of each entry included in the
>>> +  arm-frequency-table should be as defined below (#cells per entry = 13)
>>> +
>>> +      cell #1: arm clock frequency
>>> +      cell #2: expected arm clock parent frequency
>>> +      cell #3 ~ cell #12: value of clock divider in the following order
>>> +              arm_ratio, cpud_ratio, acp_ratio, periph_ratio,
>>> +              atb_ratio, pclk_dbg_ratio, apll_ratio, arm2_ratio,
>>> +              copy_ratio, hpm_ratio
>>> +      cell #13: reserved (should be zero)
>>> +
>>> +
>>>  The following is the list of clocks generated by the controller. Each clock is
>>>  assigned an identifier and client nodes use this identifier to specify the
>>>  clock which they consume.
>>> @@ -177,6 +191,13 @@ Example 1: An example of a clock controller node is listed below.
>>>                 compatible = "samsung,exynos5250-clock";
>>>                 reg = <0x10010000 0x30000>;
>>>                 #clock-cells = <1>;
>>> +
>>> +               arm-frequency-table = <1700000 1700000 0 3 7 7 7 3 5 0 0 2>,
>>> +                                     <1600000 1600000 0 3 7 7 7 1 4 0 0 2>,
>>> +                                     <1500000 1500000 0 2 7 7 7 1 4 0 0 2>,
>>> +                                     <1400000 1400000 0 2 7 7 6 1 4 0 0 2>,
>>> +                                     <1300000 1300000 0 2 7 7 6 1 3 0 0 2>,
>>> +                                     <1200000 1200000 0 2 7 7 5 1 3 0 0 2>;
>>>         };
>>>
>>>  Example 2: UART controller node that consumes the clock generated by the clock
>>> --
>>> 1.6.6.rc2
>>>