[PATCH 1/5] arm/dts: babbage: add gpt and uart related clock nodes

Grant Likely grant.likely at secretlab.ca
Tue Mar 8 04:48:10 EST 2011


On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo <shawn.guo at linaro.org> wrote:
> The patch is to add all gpt, uart related dt clock nodes for babbage.
> It sticks to the clock name used in clock-mx51-mx53.c, so that
> everything gets consistent to Reference Manual.  For example, the
> numbering in clock name usually starts from 1, while 'reg' property
> numbering starts from 0 to easy clock binding.
>
> Besides the generally used clock bindings, the following properties
> are proposed in this patch.
>
> * clock-alias
> Like clock-outputs to reflect cl->dev_id, property clock-alias is
> defined to reflect cl->con_id.

This feels like leakage of Linux kernel implementation details getting
encoded into the binding.  There shouldn't be any need for a clock
alias property.  It should all just work to have multiple devices
explicitly refer to the same clock node because the dt clock support
patch gets first crack at resolving a struct clk pointer before clkdev
does its lookup.

>
> * clock-depend
> The mxc 'struct clk' has the member 'secondary' to refer to the clock
> that the 'clk' has dependency on.  This 'secondary' clock needs to be
> on whenever the 'clk' is set to on.  This clock-depend property is
> defined to reflect this 'secondary' clock.

This is fine, but it is a Freescale specific binding.  Each of the
imx51 clock nodes should have compatible set to something like
"fsl,imx51-clock" so that the OS can know that it should be using this
binding.

>
> Signed-off-by: Shawn Guo <shawn.guo at linaro.org>
> ---
>  arch/arm/boot/dts/babbage.dts |  162 +++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 156 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/boot/dts/babbage.dts b/arch/arm/boot/dts/babbage.dts
> index 46a3071..1774cec 100644
> --- a/arch/arm/boot/dts/babbage.dts
> +++ b/arch/arm/boot/dts/babbage.dts
> @@ -35,19 +35,169 @@
>                #address-cells = <1>;
>                #size-cells = <0>;
>
> -               uart0_clk: uart at 0 {
> +               ckil_clk: clkil {
> +                       compatible = "fixed-clock";
> +                       #frequency-cells = <1>;
> +                       clock-outputs = "clil";
> +                       clock-frequency = <32768>;
> +               };
> +
> +               ckih_clk: ckih {
> +                       compatible = "fixed-clock";
> +                       #frequency-cells = <1>;
> +                       clock-outputs = "ckih";
> +                       clock-frequency = <22579200>;
> +               };
> +
> +               osc_clk: soc {
> +                       compatible = "fixed-clock";
> +                       #frequency-cells = <1>;
> +                       clock-outputs = "osc";
> +                       clock-frequency = <24000000>;
> +               };
> +
> +               pll1_main_clk: pll1_main {
> +                       compatible = "clock";

As hinted on above, "clock" doesn't look like a good compatible
property.  It should specify the specific implementation of a clock
device.  ie. "fsl,imx51-clock".  Even that example may be too generic
if there are multiple types of clock controllers in the imx51 SoC.

> +                       reg = <0>;
> +                       clock-outputs = "pll1_main";
> +                       clock-source = <&osc_clk>;
> +               };
> +
> +               pll1_sw_clk: pll_switch at 0 {
> +                       compatible = "clock";
> +                       reg = <0>;
> +                       clock-outputs = "pll1_sw";
> +                       clock-source = <&pll1_main_clk>;
> +               };
> +
> +               pll2_sw_clk: pll_switch at 1 {
> +                       compatible = "clock";
> +                       reg = <1>;
> +                       clock-outputs = "pll2_sw";
> +                       clock-source = <&osc_clk>;
> +               };
> +
> +               pll3_sw_clk: pll_switch at 2 {
> +                       compatible = "clock";
> +                       reg = <2>;
> +                       clock-outputs = "pll3_sw";
> +                       clock-source = <&osc_clk>;
> +               };
> +
> +               lp_apm_clk: lp_apm {
> +                       compatible = "clock";
> +                       clock-outputs = "lp_apm";
> +                       clock-source = <&osc_clk>;
> +               };
> +
> +               main_bus_clk: main_bus {
> +                       compatible = "clock";
> +                       clock-outputs = "main_bus";
> +                       clock-source = <&pll2_sw_clk>;
> +               };
> +
> +               ahb_clk: ahb {
> +                       compatible = "clock";
> +                       clock-outputs = "ahb";
> +                       clock-source = <&main_bus_clk>;
> +               };
> +
> +               ipg_clk: ipg {
> +                       compatible = "clock";
> +                       clock-outputs = "ipg";
> +                       clock-source = <&ahb_clk>;
> +               };
> +
> +               spba_clk: spba {
> +                       compatible = "clock";
> +                       clock-outputs = "spba";
> +                       clock-source = <&ipg_clk>;
> +               };
> +
> +               ahb_max_clk: ahb_max {
> +                       compatible = "clock";
> +                       clock-outputs = "ahb_max";
> +                       clock-source = <&ahb_clk>;
> +               };
> +
> +               aips_tz1_clk: aips_tz at 0 {
> +                       compatible = "clock";
> +                       reg = <0>;
> +                       clock-outputs = "aips_tz1";
> +                       clock-source = <&ahb_clk>;
> +                       clock-depend = <&ahb_max_clk>;
> +               };
> +
> +               aips_tz2_clk: aips_tz at 1 {
> +                       compatible = "clock";
> +                       reg = <1>;
> +                       clock-outputs = "aips_tz2";
> +                       clock-source = <&ahb_clk>;
> +                       clock-depend = <&ahb_max_clk>;
> +               };
> +
> +               gpt_ipg_clk: gpt_ipg {
> +                       compatible = "clock";
> +                       clock-outputs = "gpt_ipg";
> +                       clock-source = <&ipg_clk>;
> +               };
> +
> +               gpt_clk: gpt {
> +                       compatible = "clock";
> +                       clock-outputs = "gpt";
> +                       clock-source = <&ipg_clk>;
> +                       clock-depend = <&gpt_ipg_clk>;
> +               };
> +
> +               uart1_ipg_clk: uart_ipg at 0 {
>                        compatible = "clock";
> +                       reg = <0>;
> +                       clock-outputs = "uart1_ipg";
> +                       clock-source = <&ipg_clk>;
> +                       clock-depend = <&aips_tz1_clk>;
> +               };
> +
> +               uart2_ipg_clk: uart_ipg at 1 {
> +                       compatible = "clock";
> +                       reg = <1>;
> +                       clock-outputs = "uart2_ipg";
> +                       clock-source = <&ipg_clk>;
> +                       clock-depend = <&aips_tz1_clk>;
> +               };
> +
> +               uart3_ipg_clk: uart_ipg at 2 {
> +                       compatible = "clock";
> +                       reg = <2>;
> +                       clock-outputs = "uart3_ipg";
> +                       clock-source = <&ipg_clk>;
> +                       clock-depend = <&spba_clk>;
> +               };
> +
> +               uart_root_clk: uart_root {
> +                       compatible = "clock";
> +                       clock-outputs = "uart_root";
> +                       clock-source = <&pll2_sw_clk>;
> +               };
> +
> +               uart1_clk: uart at 0 {
> +                       compatible = "clock";
> +                       reg = <0>;
>                        clock-outputs = "imx-uart.0";
> +                       clock-source = <&uart_root_clk>;
>                };
>
> -               uart1_clk: uart at 1 {
> +               uart2_clk: uart at 1 {
>                        compatible = "clock";
> +                       reg = <1>;
>                        clock-outputs = "imx-uart.1";
> +                       clock-source = <&uart_root_clk>;
>                };
>
> -               uart2_clk: uart at 2 {
> +               uart3_clk: uart at 2 {
>                        compatible = "clock";
> +                       reg = <2>;
>                        clock-outputs = "imx-uart.2";
> +                       clock-source = <&uart_root_clk>;
>                };
>
>                fec_clk: fec at 0 {
> @@ -67,7 +217,7 @@
>                        reg = <0xc000 0x1000>;
>                        interrupts = <0x21>;
>                        rts-cts;
> -                       uart-clock = <&uart2_clk>, "uart";
> +                       uart-clock = <&uart3_clk>, "uart";
>                };
>        };
>
> @@ -82,7 +232,7 @@
>                        reg = <0xbc000 0x1000>;
>                        interrupts = <0x1f>;
>                        rts-cts;
> -                       uart-clock = <&uart0_clk>, "uart";
> +                       uart-clock = <&uart1_clk>, "uart";
>                };
>
>                imx-uart at c0000 {
> @@ -90,7 +240,7 @@
>                        reg = <0xc0000 0x1000>;
>                        interrupts = <0x20>;
>                        rts-cts;
> -                       uart-clock = <&uart1_clk>, "uart";
> +                       uart-clock = <&uart2_clk>, "uart";
>                };
>        };
>
> --
> 1.7.1
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.


More information about the devicetree-discuss mailing list