From: Tomasz Figa <t.figa@samsung.com> To: Rahul Sharma <rahul.sharma@samsung.com>, linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: mturquette@linaro.org, kgene.kim@samsung.com, tomasz.figa@gmail.com, joshi@samsung.com, r.sh.open@gmail.com, pankaj.dubey@samsung.com Subject: Re: [PATCH v5 4/5] clk/exynos5260: add macros and documentation for exynos5260 Date: Thu, 13 Mar 2014 13:36:28 +0100 [thread overview] Message-ID: <5321A64C.3070902@samsung.com> (raw) In-Reply-To: <1394636208-3125-5-git-send-email-rahul.sharma@samsung.com> Hi Rahul, On 12.03.2014 15:56, Rahul Sharma wrote: > Add macros which are used as Clock IDs in DT and clock file. > It also adds the documentation for the exynos5260 clocks. > > Signed-off-by: Rahul Sharma <Rahul.Sharma@samsung.com> > --- > .../devicetree/bindings/clock/exynos5260-clock.txt | 55 +++++ > include/dt-bindings/clock/exynos5260-clk.h | 233 ++++++++++++++++++++ > 2 files changed, 288 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/exynos5260-clock.txt > create mode 100644 include/dt-bindings/clock/exynos5260-clk.h > > diff --git a/Documentation/devicetree/bindings/clock/exynos5260-clock.txt b/Documentation/devicetree/bindings/clock/exynos5260-clock.txt > new file mode 100644 > index 0000000..4128892 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/exynos5260-clock.txt > @@ -0,0 +1,55 @@ > +* Samsung Exynos5260 Clock Controller > + > +The Exynos5260 clock controller encapsulate all CMUs which are s/encapsulate/encapsulates/ Anyway, I don't think there is any global clock controller entity that should be described here. The binding is related to several separate entities found on Exynos5260, i.e. particular CMUs. > +instantiaited independently from the device-tree. As a whole, these s/instantiaited/instantiated/ s/the device-tree/device tree/ > +CMUs generates and supplies clocks to various controllers within s/generates/generate/ s/supplies/supply/ > +the Exynos5260 SoC. > + > +Required Properties: > + > +- compatible: should be one of the following. > + "exynos5260-clock-top" > + "exynos5260-clock-peri" > + "exynos5260-clock-egl" > + "exynos5260-clock-kfc" > + "exynos5260-clock-g2d" > + "exynos5260-clock-mif" > + "exynos5260-clock-mfc" > + "exynos5260-clock-g3d" > + "exynos5260-clock-fsys" > + "exynos5260-clock-aud" > + "exynos5260-clock-isp" > + "exynos5260-clock-gscl" > + "exynos5260-clock-disp" Compatible strings should be prefixed with Samsung's vendor prefix, i.e. "samsung,". > + > +- reg: physical base address of the controller and length of memory mapped > + region. > + > +- #clock-cells: should be 1. As implemented in patch [1] the clock core will be able to sort out dependencies between particular clock providers based on clock consumer bindings. This means that it would be a good idea to add clock properties to this binding and strictly specify which clocks outside particular CMU block are fed into the block. So for example, if CMU TOP uses fin_pll clock, its node would need to have "clock-names" property with "fin_pll" entry and "clocks" property with respective entry pointing to the fixed rate clock node. In general, all external, IO and PHY clocks fed into CMU blocks should be listed this way to have dependencies between clocks properly defined in the system. [1] http://thread.gmane.org/gmane.linux.kernel/1654635 > + > +The following is the list of clocks generated by each controller. There is no list below. IMHO this sentence should be removed. > Each > +clock is assigned with a MACRO constant. These constants are defined in > +"dt-bindings/clk/exynos5260-clk.h". DT client nodes use this MACRO to > +specify the clock, which they want to consume. IMHO the whole paragraph above can be rewritten taking samsung,s3c64xx-clock.txt as an example: Each clock is assigned an identifier and client nodes can use this identifier to specify the clock which they consume. All available clocks are defined as preprocessor macros in dt-bindings/clock/exynos5260-clk.h header and can be used in device tree sources. In addition to the above, a list of external clocks that need or may be present should be added. Again, see samsung,s3c64xx-clock.txt for an example. > + > +Example 1: An example of a clock controller node is listed below. > + > + cmu_disp: clock-controller@0x14550000 { > + compatible = "exynos5260-clock-disp"; Incorrect compatible string. > + reg = <0x14550000 0x10000>; > + #clock-cells = <1>; > + }; > + > +Example 2: UART controller node that consumes the clock generated by the > + peri clock controller. Refer to the standard clock bindings for > + information about 'clocks' and 'clock-names' property. > + > + serial@12C00000 { > + compatible = "samsung,exynos4210-uart"; > + reg = <0x12C00000 0x100>; > + interrupts = <0 146 0>; > + clocks = <&clock_peri PERI_PCLK_UART0>, <&clock_peri PERI_SCLK_UART0>; > + clock-names = "uart", "clk_uart_baud0"; > + status = "disabled"; > + }; > + > diff --git a/include/dt-bindings/clock/exynos5260-clk.h b/include/dt-bindings/clock/exynos5260-clk.h > new file mode 100644 > index 0000000..d6f4391 > --- /dev/null > +++ b/include/dt-bindings/clock/exynos5260-clk.h > @@ -0,0 +1,233 @@ > +/* > + * Copyright (c) 2014 Samsung Electronics Co., Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Provides Constants for Exynos5260 clocks. > +*/ > + > +#ifndef _DT_BINDINGS_CLK_EXYNOS5260_H > +#define _DT_BINDINGS_CLK_EXYNOS5260_H > + > +/* > + * Clock names: XXXXX_YYYYYY_ZZZZ > + * |---| |----| |--| > + * cmu type IP > + */ > + > +/* List Of Clocks For CMU_TOP */ > +#define FIN_PLL 1 > +#define TOP_FOUT_DISP_PLL 2 > +#define TOP_FOUT_AUD_PLL 3 > +#define TOP_SCLK_MMC0 4 [snip] > +#define DISP_CLK_DSIM1 9 > +#define DISP_CLK_DPPHY 10 > +#define DISP_CLK_DP 11 > +#define DISP_SCLK_PIXEL 12 > +#define DISP_MOUT_HDMI_PHY_PIXEL_USER 13 > +#define DISP_MOUT_HDMI_PHY_PIXEL 14 nit: Incorrect alignment. Please indent further all the numbers to match the longest indentation level. Best regards, Tomasz
WARNING: multiple messages have this Message-ID (diff)
From: t.figa@samsung.com (Tomasz Figa) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v5 4/5] clk/exynos5260: add macros and documentation for exynos5260 Date: Thu, 13 Mar 2014 13:36:28 +0100 [thread overview] Message-ID: <5321A64C.3070902@samsung.com> (raw) In-Reply-To: <1394636208-3125-5-git-send-email-rahul.sharma@samsung.com> Hi Rahul, On 12.03.2014 15:56, Rahul Sharma wrote: > Add macros which are used as Clock IDs in DT and clock file. > It also adds the documentation for the exynos5260 clocks. > > Signed-off-by: Rahul Sharma <Rahul.Sharma@samsung.com> > --- > .../devicetree/bindings/clock/exynos5260-clock.txt | 55 +++++ > include/dt-bindings/clock/exynos5260-clk.h | 233 ++++++++++++++++++++ > 2 files changed, 288 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/exynos5260-clock.txt > create mode 100644 include/dt-bindings/clock/exynos5260-clk.h > > diff --git a/Documentation/devicetree/bindings/clock/exynos5260-clock.txt b/Documentation/devicetree/bindings/clock/exynos5260-clock.txt > new file mode 100644 > index 0000000..4128892 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/exynos5260-clock.txt > @@ -0,0 +1,55 @@ > +* Samsung Exynos5260 Clock Controller > + > +The Exynos5260 clock controller encapsulate all CMUs which are s/encapsulate/encapsulates/ Anyway, I don't think there is any global clock controller entity that should be described here. The binding is related to several separate entities found on Exynos5260, i.e. particular CMUs. > +instantiaited independently from the device-tree. As a whole, these s/instantiaited/instantiated/ s/the device-tree/device tree/ > +CMUs generates and supplies clocks to various controllers within s/generates/generate/ s/supplies/supply/ > +the Exynos5260 SoC. > + > +Required Properties: > + > +- compatible: should be one of the following. > + "exynos5260-clock-top" > + "exynos5260-clock-peri" > + "exynos5260-clock-egl" > + "exynos5260-clock-kfc" > + "exynos5260-clock-g2d" > + "exynos5260-clock-mif" > + "exynos5260-clock-mfc" > + "exynos5260-clock-g3d" > + "exynos5260-clock-fsys" > + "exynos5260-clock-aud" > + "exynos5260-clock-isp" > + "exynos5260-clock-gscl" > + "exynos5260-clock-disp" Compatible strings should be prefixed with Samsung's vendor prefix, i.e. "samsung,". > + > +- reg: physical base address of the controller and length of memory mapped > + region. > + > +- #clock-cells: should be 1. As implemented in patch [1] the clock core will be able to sort out dependencies between particular clock providers based on clock consumer bindings. This means that it would be a good idea to add clock properties to this binding and strictly specify which clocks outside particular CMU block are fed into the block. So for example, if CMU TOP uses fin_pll clock, its node would need to have "clock-names" property with "fin_pll" entry and "clocks" property with respective entry pointing to the fixed rate clock node. In general, all external, IO and PHY clocks fed into CMU blocks should be listed this way to have dependencies between clocks properly defined in the system. [1] http://thread.gmane.org/gmane.linux.kernel/1654635 > + > +The following is the list of clocks generated by each controller. There is no list below. IMHO this sentence should be removed. > Each > +clock is assigned with a MACRO constant. These constants are defined in > +"dt-bindings/clk/exynos5260-clk.h". DT client nodes use this MACRO to > +specify the clock, which they want to consume. IMHO the whole paragraph above can be rewritten taking samsung,s3c64xx-clock.txt as an example: Each clock is assigned an identifier and client nodes can use this identifier to specify the clock which they consume. All available clocks are defined as preprocessor macros in dt-bindings/clock/exynos5260-clk.h header and can be used in device tree sources. In addition to the above, a list of external clocks that need or may be present should be added. Again, see samsung,s3c64xx-clock.txt for an example. > + > +Example 1: An example of a clock controller node is listed below. > + > + cmu_disp: clock-controller at 0x14550000 { > + compatible = "exynos5260-clock-disp"; Incorrect compatible string. > + reg = <0x14550000 0x10000>; > + #clock-cells = <1>; > + }; > + > +Example 2: UART controller node that consumes the clock generated by the > + peri clock controller. Refer to the standard clock bindings for > + information about 'clocks' and 'clock-names' property. > + > + serial at 12C00000 { > + compatible = "samsung,exynos4210-uart"; > + reg = <0x12C00000 0x100>; > + interrupts = <0 146 0>; > + clocks = <&clock_peri PERI_PCLK_UART0>, <&clock_peri PERI_SCLK_UART0>; > + clock-names = "uart", "clk_uart_baud0"; > + status = "disabled"; > + }; > + > diff --git a/include/dt-bindings/clock/exynos5260-clk.h b/include/dt-bindings/clock/exynos5260-clk.h > new file mode 100644 > index 0000000..d6f4391 > --- /dev/null > +++ b/include/dt-bindings/clock/exynos5260-clk.h > @@ -0,0 +1,233 @@ > +/* > + * Copyright (c) 2014 Samsung Electronics Co., Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Provides Constants for Exynos5260 clocks. > +*/ > + > +#ifndef _DT_BINDINGS_CLK_EXYNOS5260_H > +#define _DT_BINDINGS_CLK_EXYNOS5260_H > + > +/* > + * Clock names: XXXXX_YYYYYY_ZZZZ > + * |---| |----| |--| > + * cmu type IP > + */ > + > +/* List Of Clocks For CMU_TOP */ > +#define FIN_PLL 1 > +#define TOP_FOUT_DISP_PLL 2 > +#define TOP_FOUT_AUD_PLL 3 > +#define TOP_SCLK_MMC0 4 [snip] > +#define DISP_CLK_DSIM1 9 > +#define DISP_CLK_DPPHY 10 > +#define DISP_CLK_DP 11 > +#define DISP_SCLK_PIXEL 12 > +#define DISP_MOUT_HDMI_PHY_PIXEL_USER 13 > +#define DISP_MOUT_HDMI_PHY_PIXEL 14 nit: Incorrect alignment. Please indent further all the numbers to match the longest indentation level. Best regards, Tomasz
next prev parent reply other threads:[~2014-03-13 12:36 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-03-12 14:56 [PATCH v5 0/5] clk: exynos: add support for exynos5260 SoC Rahul Sharma 2014-03-12 14:56 ` Rahul Sharma 2014-03-12 14:56 ` [PATCH v5 1/5] clk/samsung: add support for multiple clock providers Rahul Sharma 2014-03-12 14:56 ` Rahul Sharma 2014-03-13 14:44 ` Tomasz Figa 2014-03-13 14:44 ` Tomasz Figa 2014-03-12 14:56 ` [PATCH v5 2/5] clk/samsung: add support for pll2550xx Rahul Sharma 2014-03-12 14:56 ` Rahul Sharma 2014-03-13 14:49 ` Tomasz Figa 2014-03-13 14:49 ` Tomasz Figa 2014-03-12 14:56 ` [PATCH v5 3/5] clk/samsung: add support for pll2650xx Rahul Sharma 2014-03-12 14:56 ` Rahul Sharma 2014-03-13 1:28 ` Pankaj Dubey 2014-03-13 1:28 ` Pankaj Dubey 2014-03-13 10:29 ` Pankaj Dubey 2014-03-13 10:29 ` Pankaj Dubey 2014-03-13 14:50 ` Tomasz Figa 2014-03-13 14:50 ` Tomasz Figa 2014-03-12 14:56 ` [PATCH v5 4/5] clk/exynos5260: add macros and documentation for exynos5260 Rahul Sharma 2014-03-12 14:56 ` Rahul Sharma 2014-03-13 1:23 ` Pankaj Dubey 2014-03-13 1:23 ` Pankaj Dubey 2014-03-13 5:14 ` Rahul Sharma 2014-03-13 5:14 ` Rahul Sharma 2014-03-13 12:36 ` Tomasz Figa [this message] 2014-03-13 12:36 ` Tomasz Figa 2014-03-12 14:56 ` [PATCH v5 5/5] clk/exynos5260: add clock file " Rahul Sharma 2014-03-12 14:56 ` Rahul Sharma 2014-03-13 13:10 ` Tomasz Figa 2014-03-13 13:10 ` Tomasz Figa 2014-03-23 20:24 ` Rahul Sharma 2014-03-23 20:24 ` Rahul Sharma
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5321A64C.3070902@samsung.com \ --to=t.figa@samsung.com \ --cc=devicetree@vger.kernel.org \ --cc=joshi@samsung.com \ --cc=kgene.kim@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=mturquette@linaro.org \ --cc=pankaj.dubey@samsung.com \ --cc=r.sh.open@gmail.com \ --cc=rahul.sharma@samsung.com \ --cc=tomasz.figa@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.