Before translating all mx5 clocks from static adding way to dynamic way, I would send this out for review and comment, so that I can turn around in case that I'm on a wrong direction.
It's based on Jason's mx51-basic-dt-support patch set, and only adds gpt and uart related clocks, but it's enough to get the system at where Jason's patch can get.
[PATCH 1/5] arm/dts: babbage: add gpt and uart related clock nodes [PATCH 2/5] arm/mxc: add clk members to ease dt clock support [PATCH 3/5] arm/dt: mx51: dynamically add gpt and uart related clocks per dt nodes [PATCH 4/5] arm/dt: mx5: change timer init function to dt clock way [PATCH 5/5] of/clock: eliminate function __of_clk_get_from_provider
arch/arm/boot/dts/babbage.dts | 162 ++++++++++++- arch/arm/mach-mx5/board-dt.c | 9 +- arch/arm/mach-mx5/clock-mx51-mx53.c | 436 +++++++++++++++++++++++++++++++- arch/arm/plat-mxc/include/mach/clock.h | 4 + drivers/of/clock.c | 23 +-- drivers/tty/serial/imx.c | 79 +++++- 6 files changed, 661 insertions(+), 52 deletions(-)
Regards, Shawn
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.
* 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.
Signed-off-by: Shawn Guo shawn.guo@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@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"; + reg = <0>; + clock-outputs = "pll1_main"; + clock-source = <&osc_clk>; + }; + + pll1_sw_clk: pll_switch@0 { + compatible = "clock"; + reg = <0>; + clock-outputs = "pll1_sw"; + clock-source = <&pll1_main_clk>; + }; + + pll2_sw_clk: pll_switch@1 { + compatible = "clock"; + reg = <1>; + clock-outputs = "pll2_sw"; + clock-source = <&osc_clk>; + }; + + pll3_sw_clk: pll_switch@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@0 { + compatible = "clock"; + reg = <0>; + clock-outputs = "aips_tz1"; + clock-source = <&ahb_clk>; + clock-depend = <&ahb_max_clk>; + }; + + aips_tz2_clk: aips_tz@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@0 { compatible = "clock"; + reg = <0>; + clock-outputs = "uart1_ipg"; + clock-source = <&ipg_clk>; + clock-depend = <&aips_tz1_clk>; + }; + + uart2_ipg_clk: uart_ipg@1 { + compatible = "clock"; + reg = <1>; + clock-outputs = "uart2_ipg"; + clock-source = <&ipg_clk>; + clock-depend = <&aips_tz1_clk>; + }; + + uart3_ipg_clk: uart_ipg@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@0 { + compatible = "clock"; + reg = <0>; clock-outputs = "imx-uart.0"; + clock-source = <&uart_root_clk>; };
- uart1_clk: uart@1 { + uart2_clk: uart@1 { compatible = "clock"; + reg = <1>; clock-outputs = "imx-uart.1"; + clock-source = <&uart_root_clk>; };
- uart2_clk: uart@2 { + uart3_clk: uart@2 { compatible = "clock"; + reg = <2>; clock-outputs = "imx-uart.2"; + clock-source = <&uart_root_clk>; };
fec_clk: fec@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@c0000 { @@ -90,7 +240,7 @@ reg = <0xc0000 0x1000>; interrupts = <0x20>; rts-cts; - uart-clock = <&uart1_clk>, "uart"; + uart-clock = <&uart2_clk>, "uart"; }; };
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@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@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@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@0 {
- compatible = "clock";
- reg = <0>;
- clock-outputs = "pll1_sw";
- clock-source = <&pll1_main_clk>;
- };
- pll2_sw_clk: pll_switch@1 {
- compatible = "clock";
- reg = <1>;
- clock-outputs = "pll2_sw";
- clock-source = <&osc_clk>;
- };
- pll3_sw_clk: pll_switch@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@0 {
- compatible = "clock";
- reg = <0>;
- clock-outputs = "aips_tz1";
- clock-source = <&ahb_clk>;
- clock-depend = <&ahb_max_clk>;
- };
- aips_tz2_clk: aips_tz@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@0 {
compatible = "clock";
- reg = <0>;
- clock-outputs = "uart1_ipg";
- clock-source = <&ipg_clk>;
- clock-depend = <&aips_tz1_clk>;
- };
- uart2_ipg_clk: uart_ipg@1 {
- compatible = "clock";
- reg = <1>;
- clock-outputs = "uart2_ipg";
- clock-source = <&ipg_clk>;
- clock-depend = <&aips_tz1_clk>;
- };
- uart3_ipg_clk: uart_ipg@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@0 {
- compatible = "clock";
- reg = <0>;
clock-outputs = "imx-uart.0";
- clock-source = <&uart_root_clk>;
};
- uart1_clk: uart@1 {
- uart2_clk: uart@1 {
compatible = "clock";
- reg = <1>;
clock-outputs = "imx-uart.1";
- clock-source = <&uart_root_clk>;
};
- uart2_clk: uart@2 {
- uart3_clk: uart@2 {
compatible = "clock";
- reg = <2>;
clock-outputs = "imx-uart.2";
- clock-source = <&uart_root_clk>;
};
fec_clk: fec@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@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@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
On Mon, Mar 07, 2011 at 10:48:10AM -0700, Grant Likely wrote:
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@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.
This is to make clk_get() work. Let's take a look at this example. i.MX28 integrates a amba-pl011 uart controller, and there are two places calling clk_get() with the same dev_id to get the different 'clk'.
static struct clk_lookup lookups[] = { /* for amba bus driver */ _REGISTER_CLOCK("duart", "apb_pclk", xbus_clk) /* for amba-pl011 driver */ _REGISTER_CLOCK("duart", NULL, uart_clk) ... };
* drivers/amba/bus.c - to get xbus_clk static int amba_get_enable_pclk(struct amba_device *pcdev) { struct clk *pclk = clk_get(&pcdev->dev, "apb_pclk"); int ret;
pcdev->pclk = pclk;
if (IS_ERR(pclk)) return PTR_ERR(pclk);
ret = clk_enable(pclk); if (ret) clk_put(pclk);
return ret; }
* drivers/tty/serial/amba-pl011.c - to get uart_clk static int pl011_probe(struct amba_device *dev, struct amba_id *id) { ...
uap->clk = clk_get(&dev->dev, NULL); if (IS_ERR(uap->clk)) { ret = PTR_ERR(uap->clk); goto unmap; }
... }
Will this be broken if we do not have an alias in dt clock to reflect con_id?
- 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.
I doubt this is Freescale clock only use case. But I will do what you suggest here anyway.
Oh, I forgot another new property, clock-source, which is to reflect the parent clock. This should be very common one, but not sure if the naming is proper. The naming 'clock-provider' should not be the one, I guess.
Signed-off-by: Shawn Guo shawn.guo@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@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.
We are implementing clock-mx51-mx53.c. Would it be better to use 'fsl,mx5-clock'? Or, we will have to scan 'fsl,imx51-clock' and 'fsl,imx53-clock'. Oh, i.MX50 is also coming.
On Tue, Mar 08, 2011 at 11:44:48AM +0800, Shawn Guo wrote:
On Mon, Mar 07, 2011 at 10:48:10AM -0700, Grant Likely wrote:
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@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.
This is to make clk_get() work. Let's take a look at this example. i.MX28 integrates a amba-pl011 uart controller, and there are two places calling clk_get() with the same dev_id to get the different 'clk'.
static struct clk_lookup lookups[] = { /* for amba bus driver */ _REGISTER_CLOCK("duart", "apb_pclk", xbus_clk) /* for amba-pl011 driver */ _REGISTER_CLOCK("duart", NULL, uart_clk) ... };
- drivers/amba/bus.c - to get xbus_clk
static int amba_get_enable_pclk(struct amba_device *pcdev) { struct clk *pclk = clk_get(&pcdev->dev, "apb_pclk"); int ret;
pcdev->pclk = pclk; if (IS_ERR(pclk)) return PTR_ERR(pclk); ret = clk_enable(pclk); if (ret) clk_put(pclk); return ret;}
- drivers/tty/serial/amba-pl011.c - to get uart_clk
static int pl011_probe(struct amba_device *dev, struct amba_id *id) { ...
uap->clk = clk_get(&dev->dev, NULL); if (IS_ERR(uap->clk)) { ret = PTR_ERR(uap->clk); goto unmap; }... }
Will this be broken if we do not have an alias in dt clock to reflect con_id?
- 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.
I doubt this is Freescale clock only use case. But I will do what you suggest here anyway.
[...]
- 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.
We are implementing clock-mx51-mx53.c. Would it be better to use 'fsl,mx5-clock'? Or, we will have to scan 'fsl,imx51-clock' and 'fsl,imx53-clock'. Oh, i.MX50 is also coming.
I'm going to use 'fsl,mxc-clock', as the 'clk' is currently defined under plat-mxc. Let me know if anyone is uncomfortable with it.
On Tue, Mar 08, 2011 at 11:44:48AM +0800, Shawn Guo wrote:
On Mon, Mar 07, 2011 at 10:48:10AM -0700, Grant Likely wrote:
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@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.
This is to make clk_get() work. Let's take a look at this example. i.MX28 integrates a amba-pl011 uart controller, and there are two places calling clk_get() with the same dev_id to get the different 'clk'.
static struct clk_lookup lookups[] = { /* for amba bus driver */ _REGISTER_CLOCK("duart", "apb_pclk", xbus_clk) /* for amba-pl011 driver */ _REGISTER_CLOCK("duart", NULL, uart_clk) ... };
- drivers/amba/bus.c - to get xbus_clk
static int amba_get_enable_pclk(struct amba_device *pcdev) { struct clk *pclk = clk_get(&pcdev->dev, "apb_pclk"); int ret;
pcdev->pclk = pclk; if (IS_ERR(pclk)) return PTR_ERR(pclk); ret = clk_enable(pclk); if (ret) clk_put(pclk); return ret;}
- drivers/tty/serial/amba-pl011.c - to get uart_clk
static int pl011_probe(struct amba_device *dev, struct amba_id *id) { ...
uap->clk = clk_get(&dev->dev, NULL); if (IS_ERR(uap->clk)) { ret = PTR_ERR(uap->clk); goto unmap; }... }
Will this be broken if we do not have an alias in dt clock to reflect con_id?
Sorry. The argument above is invalid, as neither dev_id nor con_id will be used to find the 'clk' when DT clock code applies. So, yes, property clock-alias is not needed.
Hi, Shawn,
On Tue, Mar 8, 2011 at 12:22 AM, Shawn Guo shawn.guo@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.
- 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.
Signed-off-by: Shawn Guo shawn.guo@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@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";
- reg = <0>;
- clock-outputs = "pll1_main";
- clock-source = <&osc_clk>;
- };
- pll1_sw_clk: pll_switch@0 {
- compatible = "clock";
- reg = <0>;
- clock-outputs = "pll1_sw";
- clock-source = <&pll1_main_clk>;
- };
- pll2_sw_clk: pll_switch@1 {
- compatible = "clock";
- reg = <1>;
- clock-outputs = "pll2_sw";
- clock-source = <&osc_clk>;
- };
It seems that you mis-used the reg property, it need fixed globally.
BR, Jason
-- 1.7.1
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
On Tue, Mar 08, 2011 at 02:56:52PM +0800, Jason Hui wrote:
Hi, Shawn,
On Tue, Mar 8, 2011 at 12:22 AM, Shawn Guo shawn.guo@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.
- 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.
Signed-off-by: Shawn Guo shawn.guo@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@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";
- reg = <0>;
- clock-outputs = "pll1_main";
- clock-source = <&osc_clk>;
- };
- pll1_sw_clk: pll_switch@0 {
- compatible = "clock";
- reg = <0>;
- clock-outputs = "pll1_sw";
- clock-source = <&pll1_main_clk>;
- };
- pll2_sw_clk: pll_switch@1 {
- compatible = "clock";
- reg = <1>;
- clock-outputs = "pll2_sw";
- clock-source = <&osc_clk>;
- };
It seems that you mis-used the reg property, it need fixed globally.
I guessed it out from Grant's comment on your babbage.dts as below.
--- quote begin ---
- uart_clk0: uart@0 {
@0 should only be specified if the node has a 'reg = <0>' property. In this case it doesn't so either 'reg' should be added, or '@0' should be removed. --- quote end ---
Hi, Shawn,
On Tue, Mar 8, 2011 at 3:07 PM, Shawn Guo shawn.guo@freescale.com wrote:
On Tue, Mar 08, 2011 at 02:56:52PM +0800, Jason Hui wrote:
Hi, Shawn,
On Tue, Mar 8, 2011 at 12:22 AM, Shawn Guo shawn.guo@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.
- 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.
Signed-off-by: Shawn Guo shawn.guo@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@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";
- reg = <0>;
- clock-outputs = "pll1_main";
- clock-source = <&osc_clk>;
- };
- pll1_sw_clk: pll_switch@0 {
- compatible = "clock";
- reg = <0>;
- clock-outputs = "pll1_sw";
- clock-source = <&pll1_main_clk>;
- };
- pll2_sw_clk: pll_switch@1 {
- compatible = "clock";
- reg = <1>;
- clock-outputs = "pll2_sw";
- clock-source = <&osc_clk>;
- };
It seems that you mis-used the reg property, it need fixed globally.
I guessed it out from Grant's comment on your babbage.dts as below.
I don't understand clearly. Can we have the this usage, grant? reg=<1> in this case, it will be decoded as clk->id finally.
pll2_sw_clk: pll_switch@1 {
- compatible = "clock";
- reg = <1>;
- clock-outputs = "pll2_sw";
- clock-source = <&osc_clk>;
- };
I just want to raise the problems. According to ePAPR, 2.3.6 reg Property: reg Value type: <prop-encoded-array> encoded as arbitrary number of (address,length) pairs. Description: The reg property describes the address and length of a device’s memory mapped register space within its parent’s address space. The value is a <prop-encoded-array>, composed of an arbitrary number of pairs of address and length, <address, length>. The number of <u32> cells required to specify the address and length are bus-specific and are specified by the #address-cells and #size-cells properties in the parent of the device node. If the parent node specifies a value of 0 for #size-cells, the length field in the value of reg shall be omitted. Example: Suppose a device within a system-on-a-chip had two blocks of registers—a 32-byte block at offset 0x3000 in the SOC and a 256-byte block at offset 0xFE00. The reg property would be encoded as follows (assuming #address-cells and #size-cells values of 1): reg = <0x3000 0x20 0xFE00 0x100>;
--- quote end ---
-- Regards, Shawn
The 'rate' is added for fixed-clock support, while 'pll_base' is for pll clock. These two particular type of clocks are supposed to be gracefully supported by the common clk api when it gets ready.
Signed-off-by: Shawn Guo shawn.guo@linaro.org --- arch/arm/plat-mxc/include/mach/clock.h | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-mxc/include/mach/clock.h b/arch/arm/plat-mxc/include/mach/clock.h index 753a598..a29dc45 100644 --- a/arch/arm/plat-mxc/include/mach/clock.h +++ b/arch/arm/plat-mxc/include/mach/clock.h @@ -38,6 +38,10 @@ struct clk { /* Register address for clock's enable/disable control. */ void __iomem *enable_reg; u32 flags; + /* clock rate used by fixed-clock */ + unsigned long rate; + /* base address of pll */ + void __iomem *pll_base; /* get the current clock rate (always a fresh value) */ unsigned long (*get_rate) (struct clk *); /* Function ptr to set the clock to a new rate. The rate must match a
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@linaro.org wrote:
The 'rate' is added for fixed-clock support, while 'pll_base' is for pll clock. These two particular type of clocks are supposed to be gracefully supported by the common clk api when it gets ready.
How does the current imx clock code handle fixed and pll clocks? Using the dt shouldn't require any special treatment in this regard.
g.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
arch/arm/plat-mxc/include/mach/clock.h | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-mxc/include/mach/clock.h b/arch/arm/plat-mxc/include/mach/clock.h index 753a598..a29dc45 100644 --- a/arch/arm/plat-mxc/include/mach/clock.h +++ b/arch/arm/plat-mxc/include/mach/clock.h @@ -38,6 +38,10 @@ struct clk { /* Register address for clock's enable/disable control. */ void __iomem *enable_reg; u32 flags;
- /* clock rate used by fixed-clock */
- unsigned long rate;
- /* base address of pll */
- void __iomem *pll_base;
/* get the current clock rate (always a fresh value) */ unsigned long (*get_rate) (struct clk *); /* Function ptr to set the clock to a new rate. The rate must match a -- 1.7.1
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@linaro.org wrote:
The 'rate' is added for fixed-clock support, while 'pll_base' is for pll clock. These two particular type of clocks are supposed to be gracefully supported by the common clk api when it gets ready.
How does the current imx clock code handle fixed and pll clocks?
For fixed-clock, the current code gets several variables holding the rate and then return the rate from several get_rate functions.
static unsigned long external_high_reference, external_low_reference; static unsigned long oscillator_reference, ckih2_reference;
static unsigned long get_high_reference_clock_rate(struct clk *clk) { return external_high_reference; }
static unsigned long get_low_reference_clock_rate(struct clk *clk) { return external_low_reference; }
static unsigned long get_oscillator_reference_clock_rate(struct clk *clk) { return oscillator_reference; }
static unsigned long get_ckih2_reference_clock_rate(struct clk *clk) { return ckih2_reference; }
With this new rate member added, all these can be consolidated into one.
For base address of pll, the current code uses the reference to clocks statically defined to know which pll is the one.
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { #ifdef CONFIG_OF return pll->pll_base; #else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) return MX51_DPLL2_BASE; else if (pll == &pll3_sw_clk) return MX51_DPLL3_BASE; else BUG();
return NULL; #endif }
static inline void __iomem *_get_pll_base(struct clk *pll) { if (cpu_is_mx51()) return _mx51_get_pll_base(pll); else return _mx53_get_pll_base(pll); }
Using the dt shouldn't require any special treatment in this regard.
I would say these two members were added to make dt clock code simple and good.
On Tue, Mar 08, 2011 at 11:56:34AM +0800, Shawn Guo wrote:
On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@linaro.org wrote:
The 'rate' is added for fixed-clock support, while 'pll_base' is for pll clock. These two particular type of clocks are supposed to be gracefully supported by the common clk api when it gets ready.
How does the current imx clock code handle fixed and pll clocks?
For fixed-clock, the current code gets several variables holding the rate and then return the rate from several get_rate functions.
static unsigned long external_high_reference, external_low_reference; static unsigned long oscillator_reference, ckih2_reference;
static unsigned long get_high_reference_clock_rate(struct clk *clk) { return external_high_reference; }
static unsigned long get_low_reference_clock_rate(struct clk *clk) { return external_low_reference; }
static unsigned long get_oscillator_reference_clock_rate(struct clk *clk) { return oscillator_reference; }
static unsigned long get_ckih2_reference_clock_rate(struct clk *clk) { return ckih2_reference; }
With this new rate member added, all these can be consolidated into one.
For base address of pll, the current code uses the reference to clocks statically defined to know which pll is the one.
I just noticed that the references to clocks statically created are being used in the current clock code widely, and I need to work around it 'globally' anyway, so I will not add the new member 'pll_base'.
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { #ifdef CONFIG_OF return pll->pll_base; #else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) return MX51_DPLL2_BASE; else if (pll == &pll3_sw_clk) return MX51_DPLL3_BASE; else BUG();
return NULL;#endif }
static inline void __iomem *_get_pll_base(struct clk *pll) { if (cpu_is_mx51()) return _mx51_get_pll_base(pll); else return _mx53_get_pll_base(pll); }
Using the dt shouldn't require any special treatment in this regard.
I would say these two members were added to make dt clock code simple and good.
On Tue, Mar 08, 2011 at 11:56:34AM +0800, Shawn Guo wrote:
On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@linaro.org wrote:
The 'rate' is added for fixed-clock support, while 'pll_base' is for pll clock. These two particular type of clocks are supposed to be gracefully supported by the common clk api when it gets ready.
How does the current imx clock code handle fixed and pll clocks?
For fixed-clock, the current code gets several variables holding the rate and then return the rate from several get_rate functions.
static unsigned long external_high_reference, external_low_reference; static unsigned long oscillator_reference, ckih2_reference;
static unsigned long get_high_reference_clock_rate(struct clk *clk) { return external_high_reference; }
static unsigned long get_low_reference_clock_rate(struct clk *clk) { return external_low_reference; }
static unsigned long get_oscillator_reference_clock_rate(struct clk *clk) { return oscillator_reference; }
static unsigned long get_ckih2_reference_clock_rate(struct clk *clk) { return ckih2_reference; }
With this new rate member added, all these can be consolidated into one.
For base address of pll, the current code uses the reference to clocks statically defined to know which pll is the one.
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { #ifdef CONFIG_OF return pll->pll_base; #else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) return MX51_DPLL2_BASE; else if (pll == &pll3_sw_clk) return MX51_DPLL3_BASE; else BUG();
return NULL;#endif
Be careful about stuff like this. Remember that enabling CONFIG_OF must *not break* board support that does not use the device tree. The above #ifdef block will break existing users.
g.
On Tue, Mar 15, 2011 at 01:41:01AM -0600, Grant Likely wrote:
On Tue, Mar 08, 2011 at 11:56:34AM +0800, Shawn Guo wrote:
On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@linaro.org wrote:
The 'rate' is added for fixed-clock support, while 'pll_base' is for pll clock. These two particular type of clocks are supposed to be gracefully supported by the common clk api when it gets ready.
How does the current imx clock code handle fixed and pll clocks?
For fixed-clock, the current code gets several variables holding the rate and then return the rate from several get_rate functions.
static unsigned long external_high_reference, external_low_reference; static unsigned long oscillator_reference, ckih2_reference;
static unsigned long get_high_reference_clock_rate(struct clk *clk) { return external_high_reference; }
static unsigned long get_low_reference_clock_rate(struct clk *clk) { return external_low_reference; }
static unsigned long get_oscillator_reference_clock_rate(struct clk *clk) { return oscillator_reference; }
static unsigned long get_ckih2_reference_clock_rate(struct clk *clk) { return ckih2_reference; }
With this new rate member added, all these can be consolidated into one.
For base address of pll, the current code uses the reference to clocks statically defined to know which pll is the one.
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { #ifdef CONFIG_OF return pll->pll_base; #else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) return MX51_DPLL2_BASE; else if (pll == &pll3_sw_clk) return MX51_DPLL3_BASE; else BUG();
return NULL;#endif
Be careful about stuff like this. Remember that enabling CONFIG_OF must *not break* board support that does not use the device tree. The above #ifdef block will break existing users.
Though the code has been killed in the latest version I just sent yesterday I sent last night, I do not understand how it will break the existing users. The existing code is:
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) return MX51_DPLL2_BASE; else if (pll == &pll3_sw_clk) return MX51_DPLL3_BASE; else BUG();
return NULL; }
On Tue, Mar 15, 2011 at 03:50:29PM +0800, Shawn Guo wrote:
On Tue, Mar 15, 2011 at 01:41:01AM -0600, Grant Likely wrote:
On Tue, Mar 08, 2011 at 11:56:34AM +0800, Shawn Guo wrote:
On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@linaro.org wrote:
The 'rate' is added for fixed-clock support, while 'pll_base' is for pll clock. These two particular type of clocks are supposed to be gracefully supported by the common clk api when it gets ready.
How does the current imx clock code handle fixed and pll clocks?
For fixed-clock, the current code gets several variables holding the rate and then return the rate from several get_rate functions.
static unsigned long external_high_reference, external_low_reference; static unsigned long oscillator_reference, ckih2_reference;
static unsigned long get_high_reference_clock_rate(struct clk *clk) { return external_high_reference; }
static unsigned long get_low_reference_clock_rate(struct clk *clk) { return external_low_reference; }
static unsigned long get_oscillator_reference_clock_rate(struct clk *clk) { return oscillator_reference; }
static unsigned long get_ckih2_reference_clock_rate(struct clk *clk) { return ckih2_reference; }
With this new rate member added, all these can be consolidated into one.
For base address of pll, the current code uses the reference to clocks statically defined to know which pll is the one.
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { #ifdef CONFIG_OF return pll->pll_base; #else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) return MX51_DPLL2_BASE; else if (pll == &pll3_sw_clk) return MX51_DPLL3_BASE; else BUG();
return NULL;#endif
Be careful about stuff like this. Remember that enabling CONFIG_OF must *not break* board support that does not use the device tree. The above #ifdef block will break existing users.
Though the code has been killed in the latest version I just sent yesterday I sent last night, I do not understand how it will break the existing users. The existing code is:
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) return MX51_DPLL2_BASE; else if (pll == &pll3_sw_clk) return MX51_DPLL3_BASE; else BUG();
return NULL;}
What you wrote wrapped the current implementation with #ifdef CONFIG_OF ... #else [existing code] #endif. That says to me that when CONFIG_OF is enabled, the old code gets compiled out, which means the function no longer works on non-dt platforms.
The goal is to support both dt and non-dt machines with a single kernel image.
g.
-- Regards, Shawn
On Tue, Mar 15, 2011 at 02:02:56AM -0600, Grant Likely wrote:
On Tue, Mar 15, 2011 at 03:50:29PM +0800, Shawn Guo wrote:
On Tue, Mar 15, 2011 at 01:41:01AM -0600, Grant Likely wrote:
On Tue, Mar 08, 2011 at 11:56:34AM +0800, Shawn Guo wrote:
On Mon, Mar 07, 2011 at 10:53:37AM -0700, Grant Likely wrote:
On Mon, Mar 7, 2011 at 9:22 AM, Shawn Guo shawn.guo@linaro.org wrote:
The 'rate' is added for fixed-clock support, while 'pll_base' is for pll clock. These two particular type of clocks are supposed to be gracefully supported by the common clk api when it gets ready.
How does the current imx clock code handle fixed and pll clocks?
For fixed-clock, the current code gets several variables holding the rate and then return the rate from several get_rate functions.
static unsigned long external_high_reference, external_low_reference; static unsigned long oscillator_reference, ckih2_reference;
static unsigned long get_high_reference_clock_rate(struct clk *clk) { return external_high_reference; }
static unsigned long get_low_reference_clock_rate(struct clk *clk) { return external_low_reference; }
static unsigned long get_oscillator_reference_clock_rate(struct clk *clk) { return oscillator_reference; }
static unsigned long get_ckih2_reference_clock_rate(struct clk *clk) { return ckih2_reference; }
With this new rate member added, all these can be consolidated into one.
For base address of pll, the current code uses the reference to clocks statically defined to know which pll is the one.
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { #ifdef CONFIG_OF return pll->pll_base; #else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) return MX51_DPLL2_BASE; else if (pll == &pll3_sw_clk) return MX51_DPLL3_BASE; else BUG();
return NULL;#endif
Be careful about stuff like this. Remember that enabling CONFIG_OF must *not break* board support that does not use the device tree. The above #ifdef block will break existing users.
Though the code has been killed in the latest version I just sent yesterday I sent last night, I do not understand how it will break the existing users. The existing code is:
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) return MX51_DPLL2_BASE; else if (pll == &pll3_sw_clk) return MX51_DPLL3_BASE; else BUG();
return NULL;}
What you wrote wrapped the current implementation with #ifdef CONFIG_OF ... #else [existing code] #endif. That says to me that when CONFIG_OF is enabled, the old code gets compiled out, which means the function no longer works on non-dt platforms.
The goal is to support both dt and non-dt machines with a single kernel image.
Ah, I missed this point. Thanks.
This patch is to change the static clock creating and registering to the dynamic way, which scans dt clock nodes, associate clk with device_node, and then add them to clkdev accordingly.
Signed-off-by: Shawn Guo shawn.guo@linaro.org --- arch/arm/mach-mx5/clock-mx51-mx53.c | 436 +++++++++++++++++++++++++++++++++-- 1 files changed, 422 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c index dedb7f9..1940171 100644 --- a/arch/arm/mach-mx5/clock-mx51-mx53.c +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c @@ -135,6 +135,9 @@ static inline u32 _get_mux(struct clk *parent, struct clk *m0,
static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { +#ifdef CONFIG_OF + return pll->pll_base; +#else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) @@ -145,6 +148,7 @@ static inline void __iomem *_mx51_get_pll_base(struct clk *pll) BUG();
return NULL; +#endif }
static inline void __iomem *_mx53_get_pll_base(struct clk *pll) @@ -1439,33 +1443,437 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc, return 0; }
+/* + * Dynamically create and register clks per dt nodes + */ #ifdef CONFIG_OF -static struct clk *mx5_dt_clk_get(struct device_node *np, - const char *output_id, void *data) + +#define ALLOC_CLK_LOOKUP() \ + struct clk_lookup *cl; \ + struct clk *clk; \ + int ret; \ + \ + do { \ + cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL); \ + if (!cl) \ + return -ENOMEM; \ + clk = (struct clk *) (cl + 1); \ + \ + clk->parent = mx5_get_source_clk(node); \ + clk->secondary = mx5_get_source_clk(node); \ + } while (0) + +#define ADD_CLK_LOOKUP() \ + do { \ + node->data = clk; \ + cl->dev_id = of_get_property(node, \ + "clock-outputs", NULL); \ + cl->con_id = of_get_property(node, \ + "clock-alias", NULL); \ + if (!cl->dev_id && !cl->con_id) { \ + ret = -EINVAL; \ + goto out_kfree; \ + } \ + cl->clk = clk; \ + clkdev_add(cl); \ + \ + return 0; \ + \ + out_kfree: \ + kfree(cl); \ + return ret; \ + } while (0) + +static unsigned long get_fixed_clk_rate(struct clk *clk) { - return data; + return clk->rate; }
-static __init void mx5_dt_scan_clks(void) +static __init int mx5_scan_fixed_clks(void) { struct device_node *node; + struct clk_lookup *cl; struct clk *clk; - const char *id; - int rc; + const __be32 *rate; + int ret = 0;
- for_each_compatible_node(node, NULL, "clock") { - id = of_get_property(node, "clock-outputs", NULL); - if (!id) + for_each_compatible_node(node, NULL, "fixed-clock") { + cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL); + if (!cl) { + ret = -ENOMEM; + break; + } + clk = (struct clk *) (cl + 1); + + rate = of_get_property(node, "clock-frequency", NULL); + if (!rate) { + kfree(cl); continue; + } + clk->rate = be32_to_cpu(*rate); + clk->get_rate = get_fixed_clk_rate; + + node->data = clk;
- clk = clk_get_sys(id, NULL); - if (IS_ERR(clk)) + cl->dev_id = of_get_property(node, "clock-outputs", NULL); + cl->con_id = of_get_property(node, "clock-alias", NULL); + if (!cl->dev_id && !cl->con_id) { + kfree(cl); continue; + } + cl->clk = clk; + clkdev_add(cl); + } + + return ret; +} + +static struct clk *mx5_prop_name_to_clk(struct device_node *node, + const char *prop_name) +{ + struct device_node *provnode; + struct clk *clk; + const void *prop; + u32 provhandle; + + prop = of_get_property(node, prop_name, NULL); + if (!prop) + return NULL; + provhandle = be32_to_cpup(prop); + + provnode = of_find_node_by_phandle(provhandle); + if (!provnode) + return NULL; + + clk = provnode->data; + + of_node_put(provnode); + + return clk; +} + +static inline struct clk *mx5_get_source_clk(struct device_node *node) +{ + return mx5_prop_name_to_clk(node, "clock-source"); +} + +static inline struct clk *mx5_get_depend_clk(struct device_node *node) +{ + return mx5_prop_name_to_clk(node, "clock-depend"); +}
- rc = of_clk_add_provider(node, mx5_dt_clk_get, clk); - if (rc) - pr_err("error adding fixed clk %s\n", node->name); +static __init int mx5_add_uart_clk(struct device_node *node) +{ + const __be32 *reg; + int id; + + ALLOC_CLK_LOOKUP(); + + reg = of_get_property(node, "reg", NULL); + if (!reg) { + ret = -ENOENT; + goto out_kfree; + } + + id = be32_to_cpu(*reg); + if (id < 0 || id > 2) { + ret = -EINVAL; + goto out_kfree; + } + + clk->id = id; + clk->parent = mx5_get_source_clk(node); + clk->secondary = mx5_get_depend_clk(node); + clk->enable = _clk_ccgr_enable; + clk->disable = _clk_ccgr_disable; + clk->enable_reg = MXC_CCM_CCGR1; + + switch (id) { + case 0: + clk->enable_shift = MXC_CCM_CCGRx_CG4_OFFSET; + break; + case 1: + clk->enable_shift = MXC_CCM_CCGRx_CG6_OFFSET; + break; + case 2: + clk->enable_shift = MXC_CCM_CCGRx_CG8_OFFSET; + } + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_uart_root_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->get_rate = clk_uart_get_rate; + clk->set_parent = clk_uart_set_parent; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_uart_ipg_clk(struct device_node *node) +{ + const __be32 *reg; + int id; + + ALLOC_CLK_LOOKUP(); + + reg = of_get_property(node, "reg", NULL); + if (!reg) { + ret = -ENOENT; + goto out_kfree; } + + id = be32_to_cpu(*reg); + if (id < 0 || id > 2) { + ret = -EINVAL; + goto out_kfree; + } + + clk->id = id; + clk->enable_reg = MXC_CCM_CCGR1; + clk->enable = _clk_ccgr_enable; + clk->disable = _clk_ccgr_disable; + + switch (id) { + case 0: + clk->enable_shift = MXC_CCM_CCGRx_CG3_OFFSET; + break; + case 1: + clk->enable_shift = MXC_CCM_CCGRx_CG5_OFFSET; + break; + case 2: + clk->enable_shift = MXC_CCM_CCGRx_CG7_OFFSET; + } + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_gpt_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->enable_reg = MXC_CCM_CCGR2; + clk->enable_shift = MXC_CCM_CCGRx_CG9_OFFSET; + clk->enable = _clk_ccgr_enable; + clk->disable = _clk_ccgr_disable; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_gpt_ipg_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->enable_reg = MXC_CCM_CCGR2; + clk->enable_shift = MXC_CCM_CCGRx_CG10_OFFSET; + clk->enable = _clk_ccgr_enable; + clk->disable = _clk_ccgr_disable; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_aips_tz_clk(struct device_node *node) +{ + const __be32 *reg; + int id; + + ALLOC_CLK_LOOKUP(); + + reg = of_get_property(node, "reg", NULL); + if (!reg) { + ret = -ENOENT; + goto out_kfree; + } + + id = be32_to_cpu(*reg); + if (id < 0 || id > 1) { + ret = -EINVAL; + goto out_kfree; + } + + clk->id = id; + clk->enable_reg = MXC_CCM_CCGR0; + clk->enable_shift = id ? MXC_CCM_CCGRx_CG12_OFFSET : + MXC_CCM_CCGRx_CG13_OFFSET; + clk->enable = _clk_ccgr_enable; + clk->disable = _clk_ccgr_disable_inwait; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_ahb_max_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->enable_reg = MXC_CCM_CCGR0; + clk->enable_shift = MXC_CCM_CCGRx_CG14_OFFSET; + clk->enable = _clk_max_enable; + clk->disable = _clk_max_disable; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_spba_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->enable_reg = MXC_CCM_CCGR5; + clk->enable_shift = MXC_CCM_CCGRx_CG0_OFFSET; + clk->enable = _clk_ccgr_enable; + clk->disable = _clk_ccgr_disable; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_ipg_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->get_rate = clk_ipg_get_rate; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_ahb_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->get_rate = clk_ahb_get_rate; + clk->set_rate = _clk_ahb_set_rate; + clk->round_rate = _clk_ahb_round_rate; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_main_bus_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->set_parent = _clk_main_bus_set_parent; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_lp_apm_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->set_parent = _clk_lp_apm_set_parent; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_pll_switch_clk(struct device_node *node) +{ + const __be32 *reg; + int id; + + ALLOC_CLK_LOOKUP(); + + reg = of_get_property(node, "reg", NULL); + if (!reg) { + ret = -ENOENT; + goto out_kfree; + } + + id = be32_to_cpu(*reg); + if (id < 0 || id > 2) { + ret = -EINVAL; + goto out_kfree; + } + + clk->id = id; + + switch (id) { + case 0: + clk->get_rate = clk_pll1_sw_get_rate; + clk->set_parent = _clk_pll1_sw_set_parent; + break; + case 1: + clk->get_rate = clk_pll_get_rate; + clk->set_rate = _clk_pll_set_rate; + clk->enable = _clk_pll_enable; + clk->disable = _clk_pll_disable; + clk->set_parent = _clk_pll2_sw_set_parent; + clk->pll_base = MX51_DPLL2_BASE; + break; + case 2: + clk->get_rate = clk_pll_get_rate; + clk->set_rate = _clk_pll_set_rate; + clk->enable = _clk_pll_enable; + clk->disable = _clk_pll_disable; + clk->pll_base = MX51_DPLL3_BASE; + } + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_add_pll1_main_clk(struct device_node *node) +{ + ALLOC_CLK_LOOKUP(); + + clk->get_rate = clk_pll_get_rate; + clk->enable = _clk_pll_enable; + clk->disable = _clk_pll_disable; + clk->pll_base = MX51_DPLL1_BASE; + + ADD_CLK_LOOKUP(); +} + +static __init int mx5_dt_scan_clks(void) +{ + struct device_node *node; + int ret; + + ret = mx5_scan_fixed_clks(); + if (ret) { + pr_err("%s: fixed-clock failed %d\n", __func__, ret); + return ret; + } + + for_each_compatible_node(node, NULL, "clock") { + if (!strcmp(node->name, "pll1_main")) + ret = mx5_add_pll1_main_clk(node); + else if (!strcmp(node->name, "pll_switch")) + ret = mx5_add_pll_switch_clk(node); + else if (!strcmp(node->name, "lp_apm")) + ret = mx5_add_lp_apm_clk(node); + else if (!strcmp(node->name, "main_bus")) + ret = mx5_add_main_bus_clk(node); + else if (!strcmp(node->name, "ahb")) + ret = mx5_add_ahb_clk(node); + else if (!strcmp(node->name, "ipg")) + ret = mx5_add_ipg_clk(node); + else if (!strcmp(node->name, "spba")) + ret = mx5_add_spba_clk(node); + else if (!strcmp(node->name, "ahb_max")) + ret = mx5_add_ahb_max_clk(node); + else if (!strcmp(node->name, "aips_tz")) + ret = mx5_add_aips_tz_clk(node); + else if (!strcmp(node->name, "gpt_ipg")) + ret = mx5_add_gpt_ipg_clk(node); + else if (!strcmp(node->name, "gpt")) + ret = mx5_add_gpt_clk(node); + else if (!strcmp(node->name, "uart_ipg")) + ret = mx5_add_uart_ipg_clk(node); + else if (!strcmp(node->name, "uart_root")) + ret = mx5_add_uart_root_clk(node); + else if (!strcmp(node->name, "uart")) + ret = mx5_add_uart_clk(node); + else + pr_warn("%s: unknown clock node %s\n", + __func__, node->name); + + if (ret) { + pr_err("%s: clock %s failed %d\n", + __func__, node->name, ret); + break; + } + } + + return ret; }
void __init mx5_clk_dt_init(void)
On Tue, Mar 08, 2011 at 12:22:10AM +0800, Shawn Guo wrote:
This patch is to change the static clock creating and registering to the dynamic way, which scans dt clock nodes, associate clk with device_node, and then add them to clkdev accordingly.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
arch/arm/mach-mx5/clock-mx51-mx53.c | 436 +++++++++++++++++++++++++++++++++-- 1 files changed, 422 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c index dedb7f9..1940171 100644 --- a/arch/arm/mach-mx5/clock-mx51-mx53.c +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c @@ -135,6 +135,9 @@ static inline u32 _get_mux(struct clk *parent, struct clk *m0, static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { +#ifdef CONFIG_OF
- return pll->pll_base;
+#else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) @@ -145,6 +148,7 @@ static inline void __iomem *_mx51_get_pll_base(struct clk *pll) BUG(); return NULL; +#endif } static inline void __iomem *_mx53_get_pll_base(struct clk *pll) @@ -1439,33 +1443,437 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc, return 0; } +/*
- Dynamically create and register clks per dt nodes
- */
#ifdef CONFIG_OF -static struct clk *mx5_dt_clk_get(struct device_node *np,
const char *output_id, void *data)
+#define ALLOC_CLK_LOOKUP() \
- struct clk_lookup *cl; \
- struct clk *clk; \
- int ret; \
\- do { \
cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL); \if (!cl) \return -ENOMEM; \clk = (struct clk *) (cl + 1); \\clk->parent = mx5_get_source_clk(node); \clk->secondary = mx5_get_source_clk(node); \- } while (0)
+#define ADD_CLK_LOOKUP() \
- do { \
node->data = clk; \cl->dev_id = of_get_property(node, \"clock-outputs", NULL); \cl->con_id = of_get_property(node, \"clock-alias", NULL); \if (!cl->dev_id && !cl->con_id) { \ret = -EINVAL; \goto out_kfree; \} \cl->clk = clk; \clkdev_add(cl); \\return 0; \\- out_kfree: \
kfree(cl); \return ret; \- } while (0)
Yikes! Doing this as a macro will be a nightmare for debugging. This needs refactoring into functions.
+static unsigned long get_fixed_clk_rate(struct clk *clk) {
- return data;
- return clk->rate;
} -static __init void mx5_dt_scan_clks(void) +static __init int mx5_scan_fixed_clks(void) { struct device_node *node;
- struct clk_lookup *cl; struct clk *clk;
- const char *id;
- int rc;
- const __be32 *rate;
- int ret = 0;
- for_each_compatible_node(node, NULL, "clock") {
id = of_get_property(node, "clock-outputs", NULL);if (!id)
- for_each_compatible_node(node, NULL, "fixed-clock") {
cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL);if (!cl) {ret = -ENOMEM;break;}clk = (struct clk *) (cl + 1);rate = of_get_property(node, "clock-frequency", NULL);if (!rate) {kfree(cl); continue;}clk->rate = be32_to_cpu(*rate);clk->get_rate = get_fixed_clk_rate;node->data = clk;
clk = clk_get_sys(id, NULL);if (IS_ERR(clk))
cl->dev_id = of_get_property(node, "clock-outputs", NULL);cl->con_id = of_get_property(node, "clock-alias", NULL);
As discussed briefly earlier, clock-alias looks like it is encoding Linux-specific implementation details into the device tree, and it shouldn't be necessary when explicit links to clock providers are supplied in the device tree.
if (!cl->dev_id && !cl->con_id) {kfree(cl); continue;}cl->clk = clk;clkdev_add(cl);- }
- return ret;
+}
+static struct clk *mx5_prop_name_to_clk(struct device_node *node,
const char *prop_name)+{
- struct device_node *provnode;
- struct clk *clk;
- const void *prop;
- u32 provhandle;
- prop = of_get_property(node, prop_name, NULL);
- if (!prop)
return NULL;- provhandle = be32_to_cpup(prop);
- provnode = of_find_node_by_phandle(provhandle);
- if (!provnode)
return NULL;- clk = provnode->data;
- of_node_put(provnode);
- return clk;
+}
+static inline struct clk *mx5_get_source_clk(struct device_node *node) +{
- return mx5_prop_name_to_clk(node, "clock-source");
+}
+static inline struct clk *mx5_get_depend_clk(struct device_node *node) +{
- return mx5_prop_name_to_clk(node, "clock-depend");
+}
Ditto here. 'clock-depend' seems to be Linux specifc. I need to look at the usage model for these properties.
rc = of_clk_add_provider(node, mx5_dt_clk_get, clk);if (rc)pr_err("error adding fixed clk %s\n", node->name);+static __init int mx5_add_uart_clk(struct device_node *node) +{
- const __be32 *reg;
- int id;
- ALLOC_CLK_LOOKUP();
- reg = of_get_property(node, "reg", NULL);
- if (!reg) {
ret = -ENOENT;goto out_kfree;- }
- id = be32_to_cpu(*reg);
- if (id < 0 || id > 2) {
ret = -EINVAL;goto out_kfree;- }
- clk->id = id;
- clk->parent = mx5_get_source_clk(node);
- clk->secondary = mx5_get_depend_clk(node);
- clk->enable = _clk_ccgr_enable;
- clk->disable = _clk_ccgr_disable;
- clk->enable_reg = MXC_CCM_CCGR1;
- switch (id) {
- case 0:
clk->enable_shift = MXC_CCM_CCGRx_CG4_OFFSET;break;- case 1:
clk->enable_shift = MXC_CCM_CCGRx_CG6_OFFSET;break;- case 2:
clk->enable_shift = MXC_CCM_CCGRx_CG8_OFFSET;- }
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_uart_root_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->get_rate = clk_uart_get_rate;
- clk->set_parent = clk_uart_set_parent;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_uart_ipg_clk(struct device_node *node) +{
- const __be32 *reg;
- int id;
- ALLOC_CLK_LOOKUP();
- reg = of_get_property(node, "reg", NULL);
- if (!reg) {
ret = -ENOENT;goto out_kfree;- id = be32_to_cpu(*reg);
- if (id < 0 || id > 2) {
ret = -EINVAL;goto out_kfree;- }
- clk->id = id;
- clk->enable_reg = MXC_CCM_CCGR1;
- clk->enable = _clk_ccgr_enable;
- clk->disable = _clk_ccgr_disable;
- switch (id) {
- case 0:
clk->enable_shift = MXC_CCM_CCGRx_CG3_OFFSET;break;- case 1:
clk->enable_shift = MXC_CCM_CCGRx_CG5_OFFSET;break;- case 2:
clk->enable_shift = MXC_CCM_CCGRx_CG7_OFFSET;- }
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_gpt_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->enable_reg = MXC_CCM_CCGR2;
- clk->enable_shift = MXC_CCM_CCGRx_CG9_OFFSET;
- clk->enable = _clk_ccgr_enable;
- clk->disable = _clk_ccgr_disable;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_gpt_ipg_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->enable_reg = MXC_CCM_CCGR2;
- clk->enable_shift = MXC_CCM_CCGRx_CG10_OFFSET;
- clk->enable = _clk_ccgr_enable;
- clk->disable = _clk_ccgr_disable;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_aips_tz_clk(struct device_node *node) +{
- const __be32 *reg;
- int id;
- ALLOC_CLK_LOOKUP();
- reg = of_get_property(node, "reg", NULL);
- if (!reg) {
ret = -ENOENT;goto out_kfree;- }
- id = be32_to_cpu(*reg);
- if (id < 0 || id > 1) {
ret = -EINVAL;goto out_kfree;- }
- clk->id = id;
- clk->enable_reg = MXC_CCM_CCGR0;
- clk->enable_shift = id ? MXC_CCM_CCGRx_CG12_OFFSET :
MXC_CCM_CCGRx_CG13_OFFSET;- clk->enable = _clk_ccgr_enable;
- clk->disable = _clk_ccgr_disable_inwait;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_ahb_max_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->enable_reg = MXC_CCM_CCGR0;
- clk->enable_shift = MXC_CCM_CCGRx_CG14_OFFSET;
- clk->enable = _clk_max_enable;
- clk->disable = _clk_max_disable;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_spba_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->enable_reg = MXC_CCM_CCGR5;
- clk->enable_shift = MXC_CCM_CCGRx_CG0_OFFSET;
- clk->enable = _clk_ccgr_enable;
- clk->disable = _clk_ccgr_disable;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_ipg_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->get_rate = clk_ipg_get_rate;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_ahb_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->get_rate = clk_ahb_get_rate;
- clk->set_rate = _clk_ahb_set_rate;
- clk->round_rate = _clk_ahb_round_rate;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_main_bus_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->set_parent = _clk_main_bus_set_parent;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_lp_apm_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->set_parent = _clk_lp_apm_set_parent;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_pll_switch_clk(struct device_node *node) +{
- const __be32 *reg;
- int id;
- ALLOC_CLK_LOOKUP();
- reg = of_get_property(node, "reg", NULL);
- if (!reg) {
ret = -ENOENT;goto out_kfree;- }
- id = be32_to_cpu(*reg);
- if (id < 0 || id > 2) {
ret = -EINVAL;goto out_kfree;- }
- clk->id = id;
- switch (id) {
- case 0:
clk->get_rate = clk_pll1_sw_get_rate;clk->set_parent = _clk_pll1_sw_set_parent;break;- case 1:
clk->get_rate = clk_pll_get_rate;clk->set_rate = _clk_pll_set_rate;clk->enable = _clk_pll_enable;clk->disable = _clk_pll_disable;clk->set_parent = _clk_pll2_sw_set_parent;clk->pll_base = MX51_DPLL2_BASE;break;- case 2:
clk->get_rate = clk_pll_get_rate;clk->set_rate = _clk_pll_set_rate;clk->enable = _clk_pll_enable;clk->disable = _clk_pll_disable;clk->pll_base = MX51_DPLL3_BASE;- }
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_add_pll1_main_clk(struct device_node *node) +{
- ALLOC_CLK_LOOKUP();
- clk->get_rate = clk_pll_get_rate;
- clk->enable = _clk_pll_enable;
- clk->disable = _clk_pll_disable;
- clk->pll_base = MX51_DPLL1_BASE;
- ADD_CLK_LOOKUP();
+}
+static __init int mx5_dt_scan_clks(void) +{
- struct device_node *node;
- int ret;
- ret = mx5_scan_fixed_clks();
- if (ret) {
pr_err("%s: fixed-clock failed %d\n", __func__, ret);return ret;- }
- for_each_compatible_node(node, NULL, "clock") {
if (!strcmp(node->name, "pll1_main"))ret = mx5_add_pll1_main_clk(node);else if (!strcmp(node->name, "pll_switch"))ret = mx5_add_pll_switch_clk(node);else if (!strcmp(node->name, "lp_apm"))ret = mx5_add_lp_apm_clk(node);else if (!strcmp(node->name, "main_bus"))ret = mx5_add_main_bus_clk(node);else if (!strcmp(node->name, "ahb"))ret = mx5_add_ahb_clk(node);else if (!strcmp(node->name, "ipg"))ret = mx5_add_ipg_clk(node);else if (!strcmp(node->name, "spba"))ret = mx5_add_spba_clk(node);else if (!strcmp(node->name, "ahb_max"))ret = mx5_add_ahb_max_clk(node);else if (!strcmp(node->name, "aips_tz"))ret = mx5_add_aips_tz_clk(node);else if (!strcmp(node->name, "gpt_ipg"))ret = mx5_add_gpt_ipg_clk(node);else if (!strcmp(node->name, "gpt"))ret = mx5_add_gpt_clk(node);else if (!strcmp(node->name, "uart_ipg"))ret = mx5_add_uart_ipg_clk(node);else if (!strcmp(node->name, "uart_root"))ret = mx5_add_uart_root_clk(node);else if (!strcmp(node->name, "uart"))ret = mx5_add_uart_clk(node);
You can simplify this whole table is you take advantage of the .data field in struct of_device_id, and use for_each_matching_node() to search for relevant nodes.
elsepr_warn("%s: unknown clock node %s\n",__func__, node->name);if (ret) {pr_err("%s: clock %s failed %d\n",__func__, node->name, ret);break;}- }
- return ret;
} void __init mx5_clk_dt_init(void) -- 1.7.1
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
On Tue, Mar 15, 2011 at 01:37:31AM -0600, Grant Likely wrote:
On Tue, Mar 08, 2011 at 12:22:10AM +0800, Shawn Guo wrote:
This patch is to change the static clock creating and registering to the dynamic way, which scans dt clock nodes, associate clk with device_node, and then add them to clkdev accordingly.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
arch/arm/mach-mx5/clock-mx51-mx53.c | 436 +++++++++++++++++++++++++++++++++-- 1 files changed, 422 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c index dedb7f9..1940171 100644 --- a/arch/arm/mach-mx5/clock-mx51-mx53.c +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c @@ -135,6 +135,9 @@ static inline u32 _get_mux(struct clk *parent, struct clk *m0, static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { +#ifdef CONFIG_OF
- return pll->pll_base;
+#else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) @@ -145,6 +148,7 @@ static inline void __iomem *_mx51_get_pll_base(struct clk *pll) BUG(); return NULL; +#endif } static inline void __iomem *_mx53_get_pll_base(struct clk *pll) @@ -1439,33 +1443,437 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc, return 0; } +/*
- Dynamically create and register clks per dt nodes
- */
#ifdef CONFIG_OF -static struct clk *mx5_dt_clk_get(struct device_node *np,
const char *output_id, void *data)
+#define ALLOC_CLK_LOOKUP() \
- struct clk_lookup *cl; \
- struct clk *clk; \
- int ret; \
\- do { \
cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL); \if (!cl) \return -ENOMEM; \clk = (struct clk *) (cl + 1); \\clk->parent = mx5_get_source_clk(node); \clk->secondary = mx5_get_source_clk(node); \- } while (0)
+#define ADD_CLK_LOOKUP() \
- do { \
node->data = clk; \cl->dev_id = of_get_property(node, \"clock-outputs", NULL); \cl->con_id = of_get_property(node, \"clock-alias", NULL); \if (!cl->dev_id && !cl->con_id) { \ret = -EINVAL; \goto out_kfree; \} \cl->clk = clk; \clkdev_add(cl); \\return 0; \\- out_kfree: \
kfree(cl); \return ret; \- } while (0)
Yikes! Doing this as a macro will be a nightmare for debugging. This needs refactoring into functions.
+static unsigned long get_fixed_clk_rate(struct clk *clk) {
- return data;
- return clk->rate;
} -static __init void mx5_dt_scan_clks(void) +static __init int mx5_scan_fixed_clks(void) { struct device_node *node;
- struct clk_lookup *cl; struct clk *clk;
- const char *id;
- int rc;
- const __be32 *rate;
- int ret = 0;
- for_each_compatible_node(node, NULL, "clock") {
id = of_get_property(node, "clock-outputs", NULL);if (!id)
- for_each_compatible_node(node, NULL, "fixed-clock") {
cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL);if (!cl) {ret = -ENOMEM;break;}clk = (struct clk *) (cl + 1);rate = of_get_property(node, "clock-frequency", NULL);if (!rate) {kfree(cl); continue;}clk->rate = be32_to_cpu(*rate);clk->get_rate = get_fixed_clk_rate;node->data = clk;
clk = clk_get_sys(id, NULL);if (IS_ERR(clk))
cl->dev_id = of_get_property(node, "clock-outputs", NULL);cl->con_id = of_get_property(node, "clock-alias", NULL);As discussed briefly earlier, clock-alias looks like it is encoding Linux-specific implementation details into the device tree, and it shouldn't be necessary when explicit links to clock providers are supplied in the device tree.
if (!cl->dev_id && !cl->con_id) {kfree(cl); continue;}cl->clk = clk;clkdev_add(cl);- }
- return ret;
+}
+static struct clk *mx5_prop_name_to_clk(struct device_node *node,
const char *prop_name)+{
- struct device_node *provnode;
- struct clk *clk;
- const void *prop;
- u32 provhandle;
- prop = of_get_property(node, prop_name, NULL);
- if (!prop)
return NULL;- provhandle = be32_to_cpup(prop);
- provnode = of_find_node_by_phandle(provhandle);
- if (!provnode)
return NULL;- clk = provnode->data;
- of_node_put(provnode);
- return clk;
+}
+static inline struct clk *mx5_get_source_clk(struct device_node *node) +{
- return mx5_prop_name_to_clk(node, "clock-source");
+}
+static inline struct clk *mx5_get_depend_clk(struct device_node *node) +{
- return mx5_prop_name_to_clk(node, "clock-depend");
+}
Ditto here. 'clock-depend' seems to be Linux specifc. I need to look at the usage model for these properties.
This is not Linux but hardware specific. Let's look at the eSDHC1 example below.
esdhc1_clk: esdhc@0 { compatible = "fsl,mxc-clock"; reg = <0>; clock-outputs = "sdhci-esdhc-imx.0"; clock-source = <&pll2_sw_clk>; clock-depend = <&esdhc1_ipg_clk>; };
We have esdhc1_clk added to clkdev standing for the clock for hardware block eSDHC1. This clock is actually the serial clock of eSDHC1, while eSDHC1 internal working clock esdhc1_ipg_clk has also to be on to get the block functional.
On Wed, Mar 16, 2011 at 08:04:56PM +0800, Shawn Guo wrote:
On Tue, Mar 15, 2011 at 01:37:31AM -0600, Grant Likely wrote:
On Tue, Mar 08, 2011 at 12:22:10AM +0800, Shawn Guo wrote:
This patch is to change the static clock creating and registering to the dynamic way, which scans dt clock nodes, associate clk with device_node, and then add them to clkdev accordingly.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
arch/arm/mach-mx5/clock-mx51-mx53.c | 436 +++++++++++++++++++++++++++++++++-- 1 files changed, 422 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c index dedb7f9..1940171 100644 --- a/arch/arm/mach-mx5/clock-mx51-mx53.c +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c @@ -135,6 +135,9 @@ static inline u32 _get_mux(struct clk *parent, struct clk *m0, static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { +#ifdef CONFIG_OF
- return pll->pll_base;
+#else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) @@ -145,6 +148,7 @@ static inline void __iomem *_mx51_get_pll_base(struct clk *pll) BUG(); return NULL; +#endif } static inline void __iomem *_mx53_get_pll_base(struct clk *pll) @@ -1439,33 +1443,437 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc, return 0; } +/*
- Dynamically create and register clks per dt nodes
- */
#ifdef CONFIG_OF -static struct clk *mx5_dt_clk_get(struct device_node *np,
const char *output_id, void *data)
+#define ALLOC_CLK_LOOKUP() \
- struct clk_lookup *cl; \
- struct clk *clk; \
- int ret; \
\- do { \
cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL); \if (!cl) \return -ENOMEM; \clk = (struct clk *) (cl + 1); \\clk->parent = mx5_get_source_clk(node); \clk->secondary = mx5_get_source_clk(node); \- } while (0)
+#define ADD_CLK_LOOKUP() \
- do { \
node->data = clk; \cl->dev_id = of_get_property(node, \"clock-outputs", NULL); \cl->con_id = of_get_property(node, \"clock-alias", NULL); \if (!cl->dev_id && !cl->con_id) { \ret = -EINVAL; \goto out_kfree; \} \cl->clk = clk; \clkdev_add(cl); \\return 0; \\- out_kfree: \
kfree(cl); \return ret; \- } while (0)
Yikes! Doing this as a macro will be a nightmare for debugging. This needs refactoring into functions.
+static unsigned long get_fixed_clk_rate(struct clk *clk) {
- return data;
- return clk->rate;
} -static __init void mx5_dt_scan_clks(void) +static __init int mx5_scan_fixed_clks(void) { struct device_node *node;
- struct clk_lookup *cl; struct clk *clk;
- const char *id;
- int rc;
- const __be32 *rate;
- int ret = 0;
- for_each_compatible_node(node, NULL, "clock") {
id = of_get_property(node, "clock-outputs", NULL);if (!id)
- for_each_compatible_node(node, NULL, "fixed-clock") {
cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL);if (!cl) {ret = -ENOMEM;break;}clk = (struct clk *) (cl + 1);rate = of_get_property(node, "clock-frequency", NULL);if (!rate) {kfree(cl); continue;}clk->rate = be32_to_cpu(*rate);clk->get_rate = get_fixed_clk_rate;node->data = clk;
clk = clk_get_sys(id, NULL);if (IS_ERR(clk))
cl->dev_id = of_get_property(node, "clock-outputs", NULL);cl->con_id = of_get_property(node, "clock-alias", NULL);As discussed briefly earlier, clock-alias looks like it is encoding Linux-specific implementation details into the device tree, and it shouldn't be necessary when explicit links to clock providers are supplied in the device tree.
if (!cl->dev_id && !cl->con_id) {kfree(cl); continue;}cl->clk = clk;clkdev_add(cl);- }
- return ret;
+}
+static struct clk *mx5_prop_name_to_clk(struct device_node *node,
const char *prop_name)+{
- struct device_node *provnode;
- struct clk *clk;
- const void *prop;
- u32 provhandle;
- prop = of_get_property(node, prop_name, NULL);
- if (!prop)
return NULL;- provhandle = be32_to_cpup(prop);
- provnode = of_find_node_by_phandle(provhandle);
- if (!provnode)
return NULL;- clk = provnode->data;
- of_node_put(provnode);
- return clk;
+}
+static inline struct clk *mx5_get_source_clk(struct device_node *node) +{
- return mx5_prop_name_to_clk(node, "clock-source");
+}
+static inline struct clk *mx5_get_depend_clk(struct device_node *node) +{
- return mx5_prop_name_to_clk(node, "clock-depend");
+}
Ditto here. 'clock-depend' seems to be Linux specifc. I need to look at the usage model for these properties.
This is not Linux but hardware specific. Let's look at the eSDHC1 example below.
esdhc1_clk: esdhc@0 { compatible = "fsl,mxc-clock"; reg = <0>; clock-outputs = "sdhci-esdhc-imx.0"; clock-source = <&pll2_sw_clk>; clock-depend = <&esdhc1_ipg_clk>; };We have esdhc1_clk added to clkdev standing for the clock for hardware block eSDHC1. This clock is actually the serial clock of eSDHC1, while eSDHC1 internal working clock esdhc1_ipg_clk has also to be on to get the block functional.
Actually, part of what I think is throwing me off here is that this node is only using half the clock binding. A single node can be both a clock provider and a clock consumer, which will often be the case for clock controllers like this. So in this case, it is using the correct "clock-outputs" property to declare the clocks that it provides, but it isn't using the *-clock binding to reference the clocks that it needs. This really should be something like:
esdhc1_clk: esdhc@0 { compatible = "fsl,mxc-clock"; reg = <0>; clock-outputs = "sdhci-esdhc-imx.0"; src-clock = <&pll2_sw_clk>, "sw-clk"; ipg-clock = <&esdhc1_ipg_clk>, "ipg-clk"; };
Also remember that a single clock node can provide multiple clock outputs. I don't know if this is a factor for imx51, but if it is then you should layout the clock nodes to replicate the actual clock hardware topology in the hardware (as opposed to the software layout that Linux is currently using).
g.
On Thu, Mar 17, 2011 at 02:47:49PM -0600, Grant Likely wrote:
On Wed, Mar 16, 2011 at 08:04:56PM +0800, Shawn Guo wrote:
On Tue, Mar 15, 2011 at 01:37:31AM -0600, Grant Likely wrote:
On Tue, Mar 08, 2011 at 12:22:10AM +0800, Shawn Guo wrote:
This patch is to change the static clock creating and registering to the dynamic way, which scans dt clock nodes, associate clk with device_node, and then add them to clkdev accordingly.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
arch/arm/mach-mx5/clock-mx51-mx53.c | 436 +++++++++++++++++++++++++++++++++-- 1 files changed, 422 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c index dedb7f9..1940171 100644 --- a/arch/arm/mach-mx5/clock-mx51-mx53.c +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c @@ -135,6 +135,9 @@ static inline u32 _get_mux(struct clk *parent, struct clk *m0, static inline void __iomem *_mx51_get_pll_base(struct clk *pll) { +#ifdef CONFIG_OF
- return pll->pll_base;
+#else if (pll == &pll1_main_clk) return MX51_DPLL1_BASE; else if (pll == &pll2_sw_clk) @@ -145,6 +148,7 @@ static inline void __iomem *_mx51_get_pll_base(struct clk *pll) BUG(); return NULL; +#endif } static inline void __iomem *_mx53_get_pll_base(struct clk *pll) @@ -1439,33 +1443,437 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc, return 0; } +/*
- Dynamically create and register clks per dt nodes
- */
#ifdef CONFIG_OF -static struct clk *mx5_dt_clk_get(struct device_node *np,
const char *output_id, void *data)
+#define ALLOC_CLK_LOOKUP() \
- struct clk_lookup *cl; \
- struct clk *clk; \
- int ret; \
\- do { \
cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL); \if (!cl) \return -ENOMEM; \clk = (struct clk *) (cl + 1); \\clk->parent = mx5_get_source_clk(node); \clk->secondary = mx5_get_source_clk(node); \- } while (0)
+#define ADD_CLK_LOOKUP() \
- do { \
node->data = clk; \cl->dev_id = of_get_property(node, \"clock-outputs", NULL); \cl->con_id = of_get_property(node, \"clock-alias", NULL); \if (!cl->dev_id && !cl->con_id) { \ret = -EINVAL; \goto out_kfree; \} \cl->clk = clk; \clkdev_add(cl); \\return 0; \\- out_kfree: \
kfree(cl); \return ret; \- } while (0)
Yikes! Doing this as a macro will be a nightmare for debugging. This needs refactoring into functions.
+static unsigned long get_fixed_clk_rate(struct clk *clk) {
- return data;
- return clk->rate;
} -static __init void mx5_dt_scan_clks(void) +static __init int mx5_scan_fixed_clks(void) { struct device_node *node;
- struct clk_lookup *cl; struct clk *clk;
- const char *id;
- int rc;
- const __be32 *rate;
- int ret = 0;
- for_each_compatible_node(node, NULL, "clock") {
id = of_get_property(node, "clock-outputs", NULL);if (!id)
- for_each_compatible_node(node, NULL, "fixed-clock") {
cl = kzalloc(sizeof(*cl) + sizeof(*clk), GFP_KERNEL);if (!cl) {ret = -ENOMEM;break;}clk = (struct clk *) (cl + 1);rate = of_get_property(node, "clock-frequency", NULL);if (!rate) {kfree(cl); continue;}clk->rate = be32_to_cpu(*rate);clk->get_rate = get_fixed_clk_rate;node->data = clk;
clk = clk_get_sys(id, NULL);if (IS_ERR(clk))
cl->dev_id = of_get_property(node, "clock-outputs", NULL);cl->con_id = of_get_property(node, "clock-alias", NULL);As discussed briefly earlier, clock-alias looks like it is encoding Linux-specific implementation details into the device tree, and it shouldn't be necessary when explicit links to clock providers are supplied in the device tree.
if (!cl->dev_id && !cl->con_id) {kfree(cl); continue;}cl->clk = clk;clkdev_add(cl);- }
- return ret;
+}
+static struct clk *mx5_prop_name_to_clk(struct device_node *node,
const char *prop_name)+{
- struct device_node *provnode;
- struct clk *clk;
- const void *prop;
- u32 provhandle;
- prop = of_get_property(node, prop_name, NULL);
- if (!prop)
return NULL;- provhandle = be32_to_cpup(prop);
- provnode = of_find_node_by_phandle(provhandle);
- if (!provnode)
return NULL;- clk = provnode->data;
- of_node_put(provnode);
- return clk;
+}
+static inline struct clk *mx5_get_source_clk(struct device_node *node) +{
- return mx5_prop_name_to_clk(node, "clock-source");
+}
+static inline struct clk *mx5_get_depend_clk(struct device_node *node) +{
- return mx5_prop_name_to_clk(node, "clock-depend");
+}
Ditto here. 'clock-depend' seems to be Linux specifc. I need to look at the usage model for these properties.
This is not Linux but hardware specific. Let's look at the eSDHC1 example below.
esdhc1_clk: esdhc@0 { compatible = "fsl,mxc-clock"; reg = <0>; clock-outputs = "sdhci-esdhc-imx.0"; clock-source = <&pll2_sw_clk>; clock-depend = <&esdhc1_ipg_clk>; };We have esdhc1_clk added to clkdev standing for the clock for hardware block eSDHC1. This clock is actually the serial clock of eSDHC1, while eSDHC1 internal working clock esdhc1_ipg_clk has also to be on to get the block functional.
Actually, part of what I think is throwing me off here is that this node is only using half the clock binding. A single node can be both a clock provider and a clock consumer, which will often be the case for clock controllers like this. So in this case, it is using the correct "clock-outputs" property to declare the clocks that it provides, but it isn't using the *-clock binding to reference the clocks that it needs. This really should be something like:
esdhc1_clk: esdhc@0 { compatible = "fsl,mxc-clock"; reg = <0>; clock-outputs = "sdhci-esdhc-imx.0"; src-clock = <&pll2_sw_clk>, "sw-clk"; ipg-clock = <&esdhc1_ipg_clk>, "ipg-clk";
The name 'ipg-clock' is too specific to be a property naming which should be generic. So I would have something like:
src-clock = <&pll2_sw_clk>, "sw-clk"; dep-clock = <&esdhc1_ipg_clk>, "ipg-clk";
};Also remember that a single clock node can provide multiple clock outputs. I don't know if this is a factor for imx51, but if it is
I do not see this is a factor for imx51 so far.
The big deal here is that it removes the call to mx51_clocks_init, where all the possible clocks are added to clkdev in static way.
Signed-off-by: Shawn Guo shawn.guo@linaro.org --- arch/arm/mach-mx5/board-dt.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-mx5/board-dt.c b/arch/arm/mach-mx5/board-dt.c index 90593f5..6b2f11b 100644 --- a/arch/arm/mach-mx5/board-dt.c +++ b/arch/arm/mach-mx5/board-dt.c @@ -15,6 +15,7 @@ #include <linux/dma-mapping.h> #include <linux/of_platform.h> #include <linux/of_fdt.h> +#include <linux/clk.h>
#include <mach/common.h> #include <mach/hardware.h> @@ -41,8 +42,14 @@ static void __init mx51_dt_board_init(void)
static void __init mx51_dt_timer_init(void) { - mx51_clocks_init(32768, 24000000, 22579200, 0); + struct clk *clk; + mx5_clk_dt_init(); + + clk = clk_get_sys("gpt", NULL); + if (!IS_ERR(clk)) + mxc_timer_init(clk, MX51_IO_ADDRESS(MX51_GPT1_BASE_ADDR), + MX51_MXC_INT_GPT); }
static struct sys_timer mxc_timer = {
With the platform clock support, the 'struct clk' should have been associated with device_node->data. So the use of function __of_clk_get_from_provider can be eliminated.
Signed-off-by: Shawn Guo shawn.guo@linaro.org --- drivers/of/clock.c | 23 ++--------------------- 1 files changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/of/clock.c b/drivers/of/clock.c index 7b5ea67..f124d0a 100644 --- a/drivers/of/clock.c +++ b/drivers/of/clock.c @@ -71,24 +71,6 @@ void of_clk_del_provider(struct device_node *np, mutex_unlock(&of_clk_lock); }
-static struct clk *__of_clk_get_from_provider(struct device_node *np, const char *clk_output) -{ - struct of_clk_provider *provider; - struct clk *clk = NULL; - - /* Check if we have such a provider in our array */ - mutex_lock(&of_clk_lock); - list_for_each_entry(provider, &of_clk_providers, link) { - if (provider->node == np) - clk = provider->get(np, clk_output, provider->data); - if (clk) - break; - } - mutex_unlock(&of_clk_lock); - - return clk; -} - struct clk *of_clk_get(struct device *dev, const char *id) { struct device_node *provnode; @@ -123,9 +105,8 @@ struct clk *of_clk_get(struct device *dev, const char *id) __func__, prop_name, dev->of_node->full_name); return NULL; } - clk = __of_clk_get_from_provider(provnode, prop); - if (clk) - dev_dbg(dev, "Using clock from %s\n", provnode->full_name); + + clk = provnode->data;
of_node_put(provnode);
-
Grant Likely -
Jason Hui -
Shawn Guo -
Shawn Guo