From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH v4 4/8] Documentation: devicetree: add cpu clock configuration data binding for Exynos4/5 Date: Sat, 17 May 2014 01:24:15 +0200 Message-ID: <53769E1F.1090706@gmail.com> References: <1400029876-5830-1-git-send-email-thomas.ab@samsung.com> <1400029876-5830-5-git-send-email-thomas.ab@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1400029876-5830-5-git-send-email-thomas.ab@samsung.com> Sender: linux-samsung-soc-owner@vger.kernel.org To: Thomas Abraham , cpufreq@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: mturquette@linaro.org, shawn.guo@linaro.org, devicetree@vger.kernel.org, rjw@rjwysocki.net, linux-samsung-soc@vger.kernel.org, kgene.kim@samsung.com, t.figa@samsung.com, l.majewski@samsung.com, viresh.kumar@linaro.org, heiko@sntech.de, thomas.ab@samsung.com, Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala List-Id: devicetree@vger.kernel.org Hi Thomas, Please see my comments inline. On 14.05.2014 03:11, Thomas Abraham wrote: > From; Thomas Abraham > > 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 > Cc: Rob Herring > Cc: Pawel Moll > Cc: Mark Rutland > Cc: Ian Campbell > Cc: Kumar Gala > Cc: > Signed-off-by: Thomas Abraham > --- > .../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