[PATCH v4 4/8] Documentation: devicetree: add cpu clock configuration data binding for Exynos4/5

Tomasz Figa
2014-05-16T23:24:15Z

Hi Thomas,

Please see my comments inline.

On 14.05.2014 03:11, Thomas Abraham wrote:
> From; Thomas Abraham <thomas.ab@samsung.com>
>
> The clock blocks within the CMU_CPU clock domain are put together into a
> new composite clock type called the cpu clock. This clock type requires
> configuration data that will be atomically programmed in the multiple
> clock blocks encapsulated within the cpu clock type when the parent clock
> frequency is changed. 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: Tomasz Figa <t.figa@samsung.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> ---
> .../devicetree/bindings/clock/exynos4-clock.txt | 37 ++++++++++++++++++++
> .../devicetree/bindings/clock/exynos5250-clock.txt | 36 +++++++++++++++++++
> 2 files changed, 73 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/exynos4-clock.txt b/Documentation/devicetree/bindings/clock/exynos4-clock.txt
> index f5a5b19..0934e02 100644
> --- a/Documentation/devicetree/bindings/clock/exynos4-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/exynos4-clock.txt
> @@ -15,6 +15,35 @@ Required Properties:
>
> - #clock-cells: should be 1.
>
> +- samsung,armclk-divider-table: when the frequency of the APLL is changed
> + the divider clocks in CMU_CPU clock domain also need to be updated. These
> + divider clocks have SoC specific divider clock output requirements for a
> + specific APLL clock speeds. When APLL clock rate is changed, these divider
> + clocks are reprogrammed with pre-determined values in order to maintain the
> + SoC specific divider clock outputs. This property lists the divider values
> + for divider clocks in the CMU_CPU block for supported APLL clock speeds.
> + The format of each entry included in the arm-frequency-table should be
> + as defined below

As far as I understand, the relation is not between the APLL frequency
and particular clocks in CPU domain, but rather between the latter and
input clock to CPU domain, which is _after_ the two dividers (called
DIV_CORE and DIV_CORE2 or ARM_DIV1 and ARM_DIV2), which is also exactly
the output frequency of ARMCLK.

> +
> + - for Exynos4210 and Exynos4212 based platforms:
> + cell #1: arm clock parent frequency

Considering my comment above, this should be rather ARMCLK frequency.

> + cell #2 ~ cell 9#: value of clock divider in the following order
> + corem0_ratio, corem1_ratio, periph_ratio, atb_ratio,
> + pclk_dbg_ratio, apll_ratio, copy_ratio, hpm_ratio.
> +
> + - for Exynos4412 based platforms:
> + cell #1: expected arm clock parent frequency

Ditto.

> + cell #2 ~ cell #10: value of clock divider in the following order
> + corem0_ratio, corem1_ratio, periph_ratio, atb_ratio,
> + pclk_dbg_ratio, apll_ratio, copy_ratio, hpm_ratio, cores_ratio
> +
> +- samsung,armclk-cells: defines the number of cells in
> + samsung,armclk-divider-table property. The value of this property depends on
> + the SoC type.

To follow conventions used by all other bindings with variable number of
cells, the property should be called "#samsung,armclk-cells". AFAIK the
"#" should be interpreted as "number of" and so accents the meaning of
the property.

> +
> + - for Exynos4210 and Exynos4212: the value should be 9.
> + - for Exynos4412: the value should be 10.
> +
> Each clock is assigned an identifier and client nodes can use this identifier
> to specify the clock which they consume.
>
> @@ -28,6 +57,14 @@ Example 1: An example of a clock controller node is listed below.
> compatible = "samsung,exynos4210-clock";
> reg = <0x10030000 0x20000>;
> #clock-cells = <1>;
> +
> + samsung,armclk-cells = <9>;
> + samsung,armclk-divider-table = <1200000 3 7 3 4 1 7 5 0>,
> + <1000000 3 7 3 4 1 7 4 0>,
> + < 800000 3 7 3 3 1 7 3 0>,
> + < 500000 3 7 3 3 1 7 3 0>,
> + < 400000 3 7 3 3 1 7 3 0>,
> + < 200000 1 3 1 1 1 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 536eacd..3d63d09 100644
> --- a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt
> @@ -13,6 +13,24 @@ Required Properties:

Same comments apply to this file as well.

Also, shouldn't you also extend exynos5420-clock.txt in the same way?

Best regards,
Tomasz