Message ID | 1363703220-4777-2-git-send-email-rogerq@ti.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 03/19/2013 04:26 PM, Roger Quadros wrote: > Register a device tree clock provider for AUX clocks > on the OMAP4 SoC. Also provide the binding information. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../devicetree/bindings/clock/omap4-clock.txt | 32 ++++++++++++++++++ > arch/arm/boot/dts/omap4.dtsi | 5 +++ > arch/arm/mach-omap2/board-generic.c | 2 +- > arch/arm/mach-omap2/cclock44xx_data.c | 35 ++++++++++++++++++++ > arch/arm/mach-omap2/clock44xx.h | 1 + > arch/arm/mach-omap2/common.h | 9 +++++ > arch/arm/mach-omap2/io.c | 6 +++ > 7 files changed, 89 insertions(+), 1 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/omap4-clock.txt > > diff --git a/Documentation/devicetree/bindings/clock/omap4-clock.txt b/Documentation/devicetree/bindings/clock/omap4-clock.txt > new file mode 100644 > index 0000000..9d5448b > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/omap4-clock.txt > @@ -0,0 +1,32 @@ > +* Clock bindings for Texas Instruments OMAP4 SCRM clocks > + > +Required properties: > +- compatible: Should be "ti,omap4-scrm" > +- #clock-cells: Should be <1> > + > +The clock consumer should specify the desired clock by having the clock > +ID in its "clocks" phandle cell. The following is a full list of SCRM > +clocks and IDs. > + > + Clock ID > + ------------------ > + auxclk0_ck 0 > + auxclk1_ck 1 > + auxclk1_ck 1 > + auxclk1_ck 1 > + auxclk1_ck 1 Argh! should be 2,3,4,5 > + > +Example: > + > +aux_clks: scrmclks { > + compatible = "ti,omap4-scrm"; > + #clock-cells = <1>; > +}; > + > +hsusb1_phy: hsusb1_phy { > + compatible = "usb-nop-xceiv"; > + reset-supply = <&hsusb1_reset>; > + clocks = <&aux_clks 3>; > + clock-names = "main_clk"; > + clock-frequency = <19200000>; > +}; > diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi > index b7db1a2..97de56c 100644 > --- a/arch/arm/boot/dts/omap4.dtsi > +++ b/arch/arm/boot/dts/omap4.dtsi > @@ -101,6 +101,11 @@ > ti,hwmods = "counter_32k"; > }; > > + aux_clks: scrmclks { > + compatible = "ti,omap4-scrm"; > + #clock-cells = <1>; > + }; > + > omap4_pmx_core: pinmux@4a100040 { > compatible = "ti,omap4-padconf", "pinctrl-single"; > reg = <0x4a100040 0x0196>; > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c > index 0274ff7..23f2064 100644 > --- a/arch/arm/mach-omap2/board-generic.c > +++ b/arch/arm/mach-omap2/board-generic.c > @@ -158,7 +158,7 @@ DT_MACHINE_START(OMAP4_DT, "Generic OMAP4 (Flattened Device Tree)") > .init_irq = omap_gic_of_init, > .init_machine = omap_generic_init, > .init_late = omap4430_init_late, > - .init_time = omap4_local_timer_init, > + .init_time = omap4_init_time, > .dt_compat = omap4_boards_compat, > .restart = omap44xx_restart, > MACHINE_END > diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c > index 3d58f33..bfc46c1 100644 > --- a/arch/arm/mach-omap2/cclock44xx_data.c > +++ b/arch/arm/mach-omap2/cclock44xx_data.c > @@ -27,6 +27,7 @@ > #include <linux/clk-private.h> > #include <linux/clkdev.h> > #include <linux/io.h> > +#include <linux/of.h> > > #include "soc.h" > #include "iomap.h" > @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { > CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), > }; > > +static struct clk *scrm_clks[] = { > + &auxclk0_ck, > + &auxclk1_ck, > + &auxclk2_ck, > + &auxclk3_ck, > + &auxclk4_ck, > + &auxclk5_ck, > +}; > + > +static struct clk_onecell_data scrm_data; > + > +#ifdef CONFIG_OF > +int __init omap4_clk_init_dt(void) > +{ > + struct device_node *np; > + > + np = of_find_compatible_node(NULL, NULL, "ti,omap4-scrm"); > + if (np) { > + scrm_data.clks = scrm_clks; > + scrm_data.clk_num = ARRAY_SIZE(scrm_clks); > + of_clk_add_provider(np, of_clk_src_onecell_get, &scrm_data); > + } > + > + return 0; > +} > + > +#else > + > +int __init omap4_clk_init_dt(void) > +{ > + return 0; > +} > +#endif /* CONFIG_OF */ > + > int __init omap4xxx_clk_init(void) > { > u32 cpu_clkflg; > diff --git a/arch/arm/mach-omap2/clock44xx.h b/arch/arm/mach-omap2/clock44xx.h > index 287a46f..6395f63 100644 > --- a/arch/arm/mach-omap2/clock44xx.h > +++ b/arch/arm/mach-omap2/clock44xx.h > @@ -16,5 +16,6 @@ > #define OMAP4430_REGM4XEN_MULT 4 > > int omap4xxx_clk_init(void); > +int omap4_clk_init_dt(void); > > #endif > diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h > index 0a6b9c7..1941d1c 100644 > --- a/arch/arm/mach-omap2/common.h > +++ b/arch/arm/mach-omap2/common.h > @@ -98,6 +98,7 @@ void am35xx_init_early(void); > void ti81xx_init_early(void); > void am33xx_init_early(void); > void omap4430_init_early(void); > +void omap4_init_time(void); > void omap5_init_early(void); > void omap3_init_late(void); /* Do not use this one */ > void omap4430_init_late(void); > @@ -143,6 +144,14 @@ static inline void omap44xx_restart(char mode, const char *cmd) > } > #endif > > +#ifdef CONFIG_ARCH_OMAP4 > +void omap4_init_time(void); > +#else > +static inline void omap4_init_time(void) > +{ > +} > +#endif > + > /* This gets called from mach-omap2/io.c, do not call this */ > void __init omap2_set_globals_tap(u32 class, void __iomem *tap); > > diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c > index 2c3fdd6..c504363 100644 > --- a/arch/arm/mach-omap2/io.c > +++ b/arch/arm/mach-omap2/io.c > @@ -603,6 +603,12 @@ void __init omap4430_init_late(void) > omap4_pm_init(); > omap2_clk_enable_autoidle_all(); > } > + > +void __init omap4_init_time(void) > +{ > + omap4_clk_init_dt(); > + omap4_local_timer_init(); > +} > #endif > > #ifdef CONFIG_SOC_OMAP5 >
[].. > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c > index 0274ff7..23f2064 100644 > --- a/arch/arm/mach-omap2/board-generic.c > +++ b/arch/arm/mach-omap2/board-generic.c > @@ -158,7 +158,7 @@ DT_MACHINE_START(OMAP4_DT, "Generic OMAP4 (Flattened Device Tree)") > .init_irq = omap_gic_of_init, > .init_machine = omap_generic_init, > .init_late = omap4430_init_late, > - .init_time = omap4_local_timer_init, > + .init_time = omap4_init_time, > .dt_compat = omap4_boards_compat, > .restart = omap44xx_restart, > MACHINE_END [].. > +#ifdef CONFIG_OF > +int __init omap4_clk_init_dt(void) > +{ > + struct device_node *np; > + > + np = of_find_compatible_node(NULL, NULL, "ti,omap4-scrm"); > + if (np) { > + scrm_data.clks = scrm_clks; > + scrm_data.clk_num = ARRAY_SIZE(scrm_clks); > + of_clk_add_provider(np, of_clk_src_onecell_get, &scrm_data); > + } > + > + return 0; > +} [].. > + > +void __init omap4_init_time(void) > +{ > + omap4_clk_init_dt(); > + omap4_local_timer_init(); > +} I guess you did all this because of_clk_add_provider() needs slab to be initialized. With the below patch[1], now clk inits happen within .init_timer already, so none of this would be needed. [1] http://www.spinics.net/lists/arm-kernel/msg231288.html
On 03/21/2013 03:08 PM, Rajendra Nayak wrote: > [].. > >> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c >> index 0274ff7..23f2064 100644 >> --- a/arch/arm/mach-omap2/board-generic.c >> +++ b/arch/arm/mach-omap2/board-generic.c >> @@ -158,7 +158,7 @@ DT_MACHINE_START(OMAP4_DT, "Generic OMAP4 (Flattened Device Tree)") >> .init_irq = omap_gic_of_init, >> .init_machine = omap_generic_init, >> .init_late = omap4430_init_late, >> - .init_time = omap4_local_timer_init, >> + .init_time = omap4_init_time, >> .dt_compat = omap4_boards_compat, >> .restart = omap44xx_restart, >> MACHINE_END > > [].. >> +#ifdef CONFIG_OF >> +int __init omap4_clk_init_dt(void) >> +{ >> + struct device_node *np; >> + >> + np = of_find_compatible_node(NULL, NULL, "ti,omap4-scrm"); >> + if (np) { >> + scrm_data.clks = scrm_clks; >> + scrm_data.clk_num = ARRAY_SIZE(scrm_clks); >> + of_clk_add_provider(np, of_clk_src_onecell_get, &scrm_data); >> + } >> + >> + return 0; >> +} > > [].. >> + >> +void __init omap4_init_time(void) >> +{ >> + omap4_clk_init_dt(); >> + omap4_local_timer_init(); >> +} > > I guess you did all this because of_clk_add_provider() needs > slab to be initialized. With the below patch[1], now clk inits > happen within .init_timer already, so none of this would > be needed. > > [1] http://www.spinics.net/lists/arm-kernel/msg231288.html > Right. I can then call omap4_clk_init_dt() from within omap4xxx_clk_init(). Any comments about the main subject? Does the approach look fine? cheers, -roger
On Thursday 21 March 2013 07:24 PM, Roger Quadros wrote: > On 03/21/2013 03:08 PM, Rajendra Nayak wrote: >> [].. >> >>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c >>> index 0274ff7..23f2064 100644 >>> --- a/arch/arm/mach-omap2/board-generic.c >>> +++ b/arch/arm/mach-omap2/board-generic.c >>> @@ -158,7 +158,7 @@ DT_MACHINE_START(OMAP4_DT, "Generic OMAP4 (Flattened Device Tree)") >>> .init_irq = omap_gic_of_init, >>> .init_machine = omap_generic_init, >>> .init_late = omap4430_init_late, >>> - .init_time = omap4_local_timer_init, >>> + .init_time = omap4_init_time, >>> .dt_compat = omap4_boards_compat, >>> .restart = omap44xx_restart, >>> MACHINE_END >> >> [].. >>> +#ifdef CONFIG_OF >>> +int __init omap4_clk_init_dt(void) >>> +{ >>> + struct device_node *np; >>> + >>> + np = of_find_compatible_node(NULL, NULL, "ti,omap4-scrm"); >>> + if (np) { >>> + scrm_data.clks = scrm_clks; >>> + scrm_data.clk_num = ARRAY_SIZE(scrm_clks); >>> + of_clk_add_provider(np, of_clk_src_onecell_get, &scrm_data); >>> + } >>> + >>> + return 0; >>> +} >> >> [].. >>> + >>> +void __init omap4_init_time(void) >>> +{ >>> + omap4_clk_init_dt(); >>> + omap4_local_timer_init(); >>> +} >> >> I guess you did all this because of_clk_add_provider() needs >> slab to be initialized. With the below patch[1], now clk inits >> happen within .init_timer already, so none of this would >> be needed. >> >> [1] http://www.spinics.net/lists/arm-kernel/msg231288.html >> > > Right. I can then call omap4_clk_init_dt() from within omap4xxx_clk_init(). > > Any comments about the main subject? Does the approach look fine? It looks fine, except for the fact that I was wondering if the clock provider needs to restrict itself to SCRM. Nishant Menon brought up a need for specifying the mpu clock source from within DT, to be able to use a generic cpufreq driver. It could be a provider (not specific to scrm, but having only scrm clocks for now) which we could add clocks as and when we see a need for them to be specified from DT. Btw, you need to copy Paul Walmsley for any clock related patches as he is the OMAP clock maintainer. > > cheers, > -roger >
+Paul & Nishant On 03/19/2013 04:26 PM, Roger Quadros wrote: > Register a device tree clock provider for AUX clocks > on the OMAP4 SoC. Also provide the binding information. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../devicetree/bindings/clock/omap4-clock.txt | 32 ++++++++++++++++++ > arch/arm/boot/dts/omap4.dtsi | 5 +++ > arch/arm/mach-omap2/board-generic.c | 2 +- > arch/arm/mach-omap2/cclock44xx_data.c | 35 ++++++++++++++++++++ > arch/arm/mach-omap2/clock44xx.h | 1 + > arch/arm/mach-omap2/common.h | 9 +++++ > arch/arm/mach-omap2/io.c | 6 +++ > 7 files changed, 89 insertions(+), 1 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/omap4-clock.txt > > diff --git a/Documentation/devicetree/bindings/clock/omap4-clock.txt b/Documentation/devicetree/bindings/clock/omap4-clock.txt > new file mode 100644 > index 0000000..9d5448b > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/omap4-clock.txt > @@ -0,0 +1,32 @@ > +* Clock bindings for Texas Instruments OMAP4 SCRM clocks > + > +Required properties: > +- compatible: Should be "ti,omap4-scrm" > +- #clock-cells: Should be <1> > + > +The clock consumer should specify the desired clock by having the clock > +ID in its "clocks" phandle cell. The following is a full list of SCRM > +clocks and IDs. > + > + Clock ID > + ------------------ > + auxclk0_ck 0 > + auxclk1_ck 1 > + auxclk1_ck 1 > + auxclk1_ck 1 > + auxclk1_ck 1 > + > +Example: > + > +aux_clks: scrmclks { > + compatible = "ti,omap4-scrm"; > + #clock-cells = <1>; > +}; > + > +hsusb1_phy: hsusb1_phy { > + compatible = "usb-nop-xceiv"; > + reset-supply = <&hsusb1_reset>; > + clocks = <&aux_clks 3>; > + clock-names = "main_clk"; > + clock-frequency = <19200000>; > +}; > diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi > index b7db1a2..97de56c 100644 > --- a/arch/arm/boot/dts/omap4.dtsi > +++ b/arch/arm/boot/dts/omap4.dtsi > @@ -101,6 +101,11 @@ > ti,hwmods = "counter_32k"; > }; > > + aux_clks: scrmclks { > + compatible = "ti,omap4-scrm"; > + #clock-cells = <1>; > + }; > + > omap4_pmx_core: pinmux@4a100040 { > compatible = "ti,omap4-padconf", "pinctrl-single"; > reg = <0x4a100040 0x0196>; > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c > index 0274ff7..23f2064 100644 > --- a/arch/arm/mach-omap2/board-generic.c > +++ b/arch/arm/mach-omap2/board-generic.c > @@ -158,7 +158,7 @@ DT_MACHINE_START(OMAP4_DT, "Generic OMAP4 (Flattened Device Tree)") > .init_irq = omap_gic_of_init, > .init_machine = omap_generic_init, > .init_late = omap4430_init_late, > - .init_time = omap4_local_timer_init, > + .init_time = omap4_init_time, > .dt_compat = omap4_boards_compat, > .restart = omap44xx_restart, > MACHINE_END > diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c > index 3d58f33..bfc46c1 100644 > --- a/arch/arm/mach-omap2/cclock44xx_data.c > +++ b/arch/arm/mach-omap2/cclock44xx_data.c > @@ -27,6 +27,7 @@ > #include <linux/clk-private.h> > #include <linux/clkdev.h> > #include <linux/io.h> > +#include <linux/of.h> > > #include "soc.h" > #include "iomap.h" > @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { > CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), > }; > > +static struct clk *scrm_clks[] = { > + &auxclk0_ck, > + &auxclk1_ck, > + &auxclk2_ck, > + &auxclk3_ck, > + &auxclk4_ck, > + &auxclk5_ck, > +}; > + > +static struct clk_onecell_data scrm_data; > + > +#ifdef CONFIG_OF > +int __init omap4_clk_init_dt(void) > +{ > + struct device_node *np; > + > + np = of_find_compatible_node(NULL, NULL, "ti,omap4-scrm"); > + if (np) { > + scrm_data.clks = scrm_clks; > + scrm_data.clk_num = ARRAY_SIZE(scrm_clks); > + of_clk_add_provider(np, of_clk_src_onecell_get, &scrm_data); > + } > + > + return 0; > +} > + > +#else > + > +int __init omap4_clk_init_dt(void) > +{ > + return 0; > +} > +#endif /* CONFIG_OF */ > + > int __init omap4xxx_clk_init(void) > { > u32 cpu_clkflg; > diff --git a/arch/arm/mach-omap2/clock44xx.h b/arch/arm/mach-omap2/clock44xx.h > index 287a46f..6395f63 100644 > --- a/arch/arm/mach-omap2/clock44xx.h > +++ b/arch/arm/mach-omap2/clock44xx.h > @@ -16,5 +16,6 @@ > #define OMAP4430_REGM4XEN_MULT 4 > > int omap4xxx_clk_init(void); > +int omap4_clk_init_dt(void); > > #endif > diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h > index 0a6b9c7..1941d1c 100644 > --- a/arch/arm/mach-omap2/common.h > +++ b/arch/arm/mach-omap2/common.h > @@ -98,6 +98,7 @@ void am35xx_init_early(void); > void ti81xx_init_early(void); > void am33xx_init_early(void); > void omap4430_init_early(void); > +void omap4_init_time(void); > void omap5_init_early(void); > void omap3_init_late(void); /* Do not use this one */ > void omap4430_init_late(void); > @@ -143,6 +144,14 @@ static inline void omap44xx_restart(char mode, const char *cmd) > } > #endif > > +#ifdef CONFIG_ARCH_OMAP4 > +void omap4_init_time(void); > +#else > +static inline void omap4_init_time(void) > +{ > +} > +#endif > + > /* This gets called from mach-omap2/io.c, do not call this */ > void __init omap2_set_globals_tap(u32 class, void __iomem *tap); > > diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c > index 2c3fdd6..c504363 100644 > --- a/arch/arm/mach-omap2/io.c > +++ b/arch/arm/mach-omap2/io.c > @@ -603,6 +603,12 @@ void __init omap4430_init_late(void) > omap4_pm_init(); > omap2_clk_enable_autoidle_all(); > } > + > +void __init omap4_init_time(void) > +{ > + omap4_clk_init_dt(); > + omap4_local_timer_init(); > +} > #endif > > #ifdef CONFIG_SOC_OMAP5 >
On 03/21/2013 04:04 PM, Rajendra Nayak wrote: > On Thursday 21 March 2013 07:24 PM, Roger Quadros wrote: >> On 03/21/2013 03:08 PM, Rajendra Nayak wrote: >>> [].. >>> >>>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c >>>> index 0274ff7..23f2064 100644 >>>> --- a/arch/arm/mach-omap2/board-generic.c >>>> +++ b/arch/arm/mach-omap2/board-generic.c >>>> @@ -158,7 +158,7 @@ DT_MACHINE_START(OMAP4_DT, "Generic OMAP4 (Flattened Device Tree)") >>>> .init_irq = omap_gic_of_init, >>>> .init_machine = omap_generic_init, >>>> .init_late = omap4430_init_late, >>>> - .init_time = omap4_local_timer_init, >>>> + .init_time = omap4_init_time, >>>> .dt_compat = omap4_boards_compat, >>>> .restart = omap44xx_restart, >>>> MACHINE_END >>> >>> [].. >>>> +#ifdef CONFIG_OF >>>> +int __init omap4_clk_init_dt(void) >>>> +{ >>>> + struct device_node *np; >>>> + >>>> + np = of_find_compatible_node(NULL, NULL, "ti,omap4-scrm"); >>>> + if (np) { >>>> + scrm_data.clks = scrm_clks; >>>> + scrm_data.clk_num = ARRAY_SIZE(scrm_clks); >>>> + of_clk_add_provider(np, of_clk_src_onecell_get, &scrm_data); >>>> + } >>>> + >>>> + return 0; >>>> +} >>> >>> [].. >>>> + >>>> +void __init omap4_init_time(void) >>>> +{ >>>> + omap4_clk_init_dt(); >>>> + omap4_local_timer_init(); >>>> +} >>> >>> I guess you did all this because of_clk_add_provider() needs >>> slab to be initialized. With the below patch[1], now clk inits >>> happen within .init_timer already, so none of this would >>> be needed. >>> >>> [1] http://www.spinics.net/lists/arm-kernel/msg231288.html >>> >> >> Right. I can then call omap4_clk_init_dt() from within omap4xxx_clk_init(). >> >> Any comments about the main subject? Does the approach look fine? > > It looks fine, except for the fact that I was wondering if the clock > provider needs to restrict itself to SCRM. > Nishant Menon brought up a need for specifying the mpu clock source > from within DT, to be able to use a generic cpufreq driver. > It could be a provider (not specific to scrm, but having only scrm > clocks for now) which we could add clocks as and when we see a need for > them to be specified from DT. OK, I will revise the patch to not make it SCRM specific. > > Btw, you need to copy Paul Walmsley for any clock related patches as > he is the OMAP clock maintainer. OK. Thanks for letting know. cheers, -roger
Hi, * Roger Quadros <rogerq@ti.com> [130319 07:31]: > Register a device tree clock provider for AUX clocks > on the OMAP4 SoC. Also provide the binding information. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../devicetree/bindings/clock/omap4-clock.txt | 32 ++++++++++++++++++ > arch/arm/boot/dts/omap4.dtsi | 5 +++ > arch/arm/mach-omap2/board-generic.c | 2 +- > arch/arm/mach-omap2/cclock44xx_data.c | 35 ++++++++++++++++++++ > arch/arm/mach-omap2/clock44xx.h | 1 + > arch/arm/mach-omap2/common.h | 9 +++++ > arch/arm/mach-omap2/io.c | 6 +++ > 7 files changed, 89 insertions(+), 1 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/omap4-clock.txt > ... > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/omap4-clock.txt Is this really specific to omap4 and auxclk only? Shouldn't it be just omap-clock.txt? > --- a/arch/arm/mach-omap2/cclock44xx_data.c > +++ b/arch/arm/mach-omap2/cclock44xx_data.c > @@ -27,6 +27,7 @@ > #include <linux/clk-private.h> > #include <linux/clkdev.h> > #include <linux/io.h> > +#include <linux/of.h> > > #include "soc.h" > #include "iomap.h" > @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { > CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), > }; > > +static struct clk *scrm_clks[] = { > + &auxclk0_ck, > + &auxclk1_ck, > + &auxclk2_ck, > + &auxclk3_ck, > + &auxclk4_ck, > + &auxclk5_ck, > +}; Hmm I don't like the idea of specifying the auxclk both in the cclock44xx_data.c and in DT.. > +static struct clk_onecell_data scrm_data; > + > +#ifdef CONFIG_OF > +int __init omap4_clk_init_dt(void) > +{ > + struct device_node *np; > + > + np = of_find_compatible_node(NULL, NULL, "ti,omap4-scrm"); > + if (np) { > + scrm_data.clks = scrm_clks; > + scrm_data.clk_num = ARRAY_SIZE(scrm_clks); > + of_clk_add_provider(np, of_clk_src_onecell_get, &scrm_data); > + } > + > + return 0; > +} > + > +#else > + > +int __init omap4_clk_init_dt(void) > +{ > + return 0; > +} > +#endif /* CONFIG_OF */ > + > int __init omap4xxx_clk_init(void) > { > u32 cpu_clkflg; .. and I'm not too keen on adding driver specific stuff to this file. How about just add a minimal drivers/clk/omap/clk-xyz.c that takes the configuration from DT and is based on the binding we already have in Documentation/devicetree/bindings/clock/clock-bindings.txt? Then as we add new bindings there we can drop them from current cclock44xx_data.c, no? That is after omap4 is DT only.. Regards, Tony
Hi Tony, On Thursday 04 April 2013 05:12 AM, Tony Lindgren wrote: > Hi, > [].. >> @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { >> CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), >> }; >> >> +static struct clk *scrm_clks[] = { >> + &auxclk0_ck, >> + &auxclk1_ck, >> + &auxclk2_ck, >> + &auxclk3_ck, >> + &auxclk4_ck, >> + &auxclk5_ck, >> +}; > > Hmm I don't like the idea of specifying the auxclk both in the > cclock44xx_data.c and in DT.. > >> +static struct clk_onecell_data scrm_data; >> + >> +#ifdef CONFIG_OF >> +int __init omap4_clk_init_dt(void) >> +{ >> + struct device_node *np; >> + >> + np = of_find_compatible_node(NULL, NULL, "ti,omap4-scrm"); >> + if (np) { >> + scrm_data.clks = scrm_clks; >> + scrm_data.clk_num = ARRAY_SIZE(scrm_clks); >> + of_clk_add_provider(np, of_clk_src_onecell_get, &scrm_data); >> + } >> + >> + return 0; >> +} >> + >> +#else >> + >> +int __init omap4_clk_init_dt(void) >> +{ >> + return 0; >> +} >> +#endif /* CONFIG_OF */ >> + >> int __init omap4xxx_clk_init(void) >> { >> u32 cpu_clkflg; > > .. and I'm not too keen on adding driver specific stuff to this file. > > How about just add a minimal drivers/clk/omap/clk-xyz.c that takes > the configuration from DT and is based on the binding we already have in > Documentation/devicetree/bindings/clock/clock-bindings.txt? > > Then as we add new bindings there we can drop them from current > cclock44xx_data.c, no? That is after omap4 is DT only.. The patch just provides an alternative for clkdev mapping in case of DT. Are you suggesting we move all *clock data* related to auxclks (and eventually all clocks) into DT? We have discussed this multiple times in the past, and moving 250 clock nodes with each needing multiple register offsets, masks, shifts etc into DT makes it completely un-readable. For me, having a way for devices to reference a clock that they use for a device using DT makes sense, but not moving all clock data into dts files. regards, Rajendra > > Regards, > > Tony >
Hi, On 04/04/2013 02:42 AM, Tony Lindgren wrote: > Hi, > > * Roger Quadros <rogerq@ti.com> [130319 07:31]: >> Register a device tree clock provider for AUX clocks >> on the OMAP4 SoC. Also provide the binding information. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> .../devicetree/bindings/clock/omap4-clock.txt | 32 ++++++++++++++++++ >> arch/arm/boot/dts/omap4.dtsi | 5 +++ >> arch/arm/mach-omap2/board-generic.c | 2 +- >> arch/arm/mach-omap2/cclock44xx_data.c | 35 ++++++++++++++++++++ >> arch/arm/mach-omap2/clock44xx.h | 1 + >> arch/arm/mach-omap2/common.h | 9 +++++ >> arch/arm/mach-omap2/io.c | 6 +++ >> 7 files changed, 89 insertions(+), 1 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/clock/omap4-clock.txt >> > ... > >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/omap4-clock.txt > > Is this really specific to omap4 and auxclk only? > > Shouldn't it be just omap-clock.txt? Yes, I've fixed this up in v2 of this patch. > >> --- a/arch/arm/mach-omap2/cclock44xx_data.c >> +++ b/arch/arm/mach-omap2/cclock44xx_data.c >> @@ -27,6 +27,7 @@ >> #include <linux/clk-private.h> >> #include <linux/clkdev.h> >> #include <linux/io.h> >> +#include <linux/of.h> >> >> #include "soc.h" >> #include "iomap.h" >> @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { >> CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), >> }; >> >> +static struct clk *scrm_clks[] = { >> + &auxclk0_ck, >> + &auxclk1_ck, >> + &auxclk2_ck, >> + &auxclk3_ck, >> + &auxclk4_ck, >> + &auxclk5_ck, >> +}; > > Hmm I don't like the idea of specifying the auxclk both in the > cclock44xx_data.c and in DT.. Right, but till we have all clocks moved to DT we only need this approach for general purpose clocks that are not mapped to devices by hwmod. e.g. auxclk are required to be specified in DT nodes for USB PHY. Without this we can't get USB host working on Panda. As Rajendra points out, it seems moving entire clock data to DT is not going to happen soon. So this is the simplistic way things can work. cheers, -roger
* Rajendra Nayak <rnayak@ti.com> [130403 22:25]: > On Thursday 04 April 2013 05:12 AM, Tony Lindgren wrote: > > > > How about just add a minimal drivers/clk/omap/clk-xyz.c that takes > > the configuration from DT and is based on the binding we already have in > > Documentation/devicetree/bindings/clock/clock-bindings.txt? > > > > Then as we add new bindings there we can drop them from current > > cclock44xx_data.c, no? That is after omap4 is DT only.. > > The patch just provides an alternative for clkdev mapping in case of DT. > Are you suggesting we move all *clock data* related to auxclks (and eventually > all clocks) into DT? Well I think we should have a driver that can take any defined clock binding from DT at least for boottime clocks. We need at least the timer clocks early during the boot, and probably the root device like MMC or possibly Ethernet depending on the board. The rest of the huge amount of clocks we can just load from /lib/firmware after mounting intramfs or root on MMC. As long as we can define any boottime clock in DT also, loading the rest of the clock data from /lib/firmware should not cause issues with booting. And as we get the clocks moved, we can drop them from cclock44xx_data.c. AFAIK the new driver can just clk_get the parent clocks so those can stay in cclock44xx_data.c for now? So basically we can do the same as we are already doing with pinctrl-single.c, but with addition of loading large amounts of data from /lib/firmware. And eventually we can do the same with hwmod too. Regarding the readability issue, we now have preprocessing support for .dts files merged AFAIK, so they can be as readable as data structures :) > We have discussed this multiple times in the past, and moving 250 clock nodes > with each needing multiple register offsets, masks, shifts etc into DT makes it > completely un-readable. For me, having a way for devices to reference a clock that they > use for a device using DT makes sense, but not moving all clock data into dts files. Yes that's why we should also support loading clocks from /lib/firmware. Naturally the parent clocks can be allocated by the clock driver(s) at least initially. But the main reason I think we should do this is so we get out of the flame path with these huge data files for every new SoC. Regards, Tony
* Roger Quadros <rogerq@ti.com> [130404 00:39]: > On 04/04/2013 02:42 AM, Tony Lindgren wrote: > >> --- a/arch/arm/mach-omap2/cclock44xx_data.c > >> +++ b/arch/arm/mach-omap2/cclock44xx_data.c > >> @@ -27,6 +27,7 @@ > >> #include <linux/clk-private.h> > >> #include <linux/clkdev.h> > >> #include <linux/io.h> > >> +#include <linux/of.h> > >> > >> #include "soc.h" > >> #include "iomap.h" > >> @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { > >> CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), > >> }; > >> > >> +static struct clk *scrm_clks[] = { > >> + &auxclk0_ck, > >> + &auxclk1_ck, > >> + &auxclk2_ck, > >> + &auxclk3_ck, > >> + &auxclk4_ck, > >> + &auxclk5_ck, > >> +}; > > > > Hmm I don't like the idea of specifying the auxclk both in the > > cclock44xx_data.c and in DT.. > > Right, but till we have all clocks moved to DT we only need this > approach for general purpose clocks that are not mapped to devices > by hwmod. For v3.10, let's just make sure that USB works with DT as then after v3.10 we can make omap4 DT only and get rid of estimated 7K lines of code and data. I guess this is the last piece missing for that, or are we also missing something else? Can't you set up a clock alias for your device so it can find the auxclk when requesting it with the dev entry? For the DT clock driver if needed for v3.10, how about just do a minimal drivers/clock/omap/ that uses the standard binding? Then that driver can just do clk_get() from cclock44xx_data.c for now? And then later on we'll just move all the clocks to a combination of DT + /lib/firmware. > e.g. auxclk are required to be specified in DT nodes for USB PHY. > Without this we can't get USB host working on Panda. OK. So if the USB PHY has a dev entry, can't you just set up a clock alias in struct omap_clk omap44xx_clks[] for it? > As Rajendra points out, it seems moving entire clock data to DT is not > going to happen soon. So this is the simplistic way things can work. Right but seems like we can get started there without moving them all at once. Regards, Tony
On 04/04/2013 07:41 PM, Tony Lindgren wrote: > * Roger Quadros <rogerq@ti.com> [130404 00:39]: >> On 04/04/2013 02:42 AM, Tony Lindgren wrote: >>>> --- a/arch/arm/mach-omap2/cclock44xx_data.c >>>> +++ b/arch/arm/mach-omap2/cclock44xx_data.c >>>> @@ -27,6 +27,7 @@ >>>> #include <linux/clk-private.h> >>>> #include <linux/clkdev.h> >>>> #include <linux/io.h> >>>> +#include <linux/of.h> >>>> >>>> #include "soc.h" >>>> #include "iomap.h" >>>> @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { >>>> CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), >>>> }; >>>> >>>> +static struct clk *scrm_clks[] = { >>>> + &auxclk0_ck, >>>> + &auxclk1_ck, >>>> + &auxclk2_ck, >>>> + &auxclk3_ck, >>>> + &auxclk4_ck, >>>> + &auxclk5_ck, >>>> +}; >>> >>> Hmm I don't like the idea of specifying the auxclk both in the >>> cclock44xx_data.c and in DT.. >> >> Right, but till we have all clocks moved to DT we only need this >> approach for general purpose clocks that are not mapped to devices >> by hwmod. > > For v3.10, let's just make sure that USB works with DT as then > after v3.10 we can make omap4 DT only and get rid of estimated > 7K lines of code and data. I guess this is the last piece missing > for that, or are we also missing something else? For panda we just need a way to map the auxclk to the USB PHY device and the relevant dts data to get USB host working with DT. Beagle USB host should work already with DT without any changes. > > Can't you set up a clock alias for your device so it can find the > auxclk when requesting it with the dev entry? > which clock is mapped to which PHY device depends on board design and that cannot be per-determined at one place. This information needs to come from the board.dts file. There was an earlier attempt to provide a way of building clock aliases at runtime from device tree [1], but the current approach is way better [1] https://lkml.org/lkml/2013/3/12/241 > For the DT clock driver if needed for v3.10, how about just do a > minimal drivers/clock/omap/ that uses the standard binding? > Then that driver can just do clk_get() from cclock44xx_data.c I don't understand how to do it and why it is better than the current approach. How can that driver do clk_get() from cclock44xx_data.c? from where does it get the clk_id to pass into clk_get()? > for now? And then later on we'll just move all the clocks to a > combination of DT + /lib/firmware. What is the benefit of moving clock data to /lib/firmware? We could as well just move it to DT only, no? > >> e.g. auxclk are required to be specified in DT nodes for USB PHY. >> Without this we can't get USB host working on Panda. > > OK. So if the USB PHY has a dev entry, can't you just set up a > clock alias in struct omap_clk omap44xx_clks[] for it? I've explained why this can't be done above. > >> As Rajendra points out, it seems moving entire clock data to DT is not >> going to happen soon. So this is the simplistic way things can work. > > Right but seems like we can get started there without moving > them all at once. > What if we provide a complete clock list instead of only auxclks in dt_clks[]? This approach is similar to arch/arm/mach-imx/clk-imx35.c Device drivers can then use them as they migrate to DT. Then later we could migrate clock data to DT, without impacting device drivers. cheers, -roger
* Roger Quadros <rogerq@ti.com> [130405 03:44]: > On 04/04/2013 07:41 PM, Tony Lindgren wrote: > > * Roger Quadros <rogerq@ti.com> [130404 00:39]: > >> On 04/04/2013 02:42 AM, Tony Lindgren wrote: > > For v3.10, let's just make sure that USB works with DT as then > > after v3.10 we can make omap4 DT only and get rid of estimated > > 7K lines of code and data. I guess this is the last piece missing > > for that, or are we also missing something else? > > For panda we just need a way to map the auxclk to the USB PHY device > and the relevant dts data to get USB host working with DT. > Beagle USB host should work already with DT without any changes. > > > > > Can't you set up a clock alias for your device so it can find the > > auxclk when requesting it with the dev entry? > > > > which clock is mapped to which PHY device depends on board design > and that cannot be per-determined at one place. This information > needs to come from the board.dts file. OK that makes sense. > There was an earlier attempt to provide a way of building clock aliases > at runtime from device tree [1], but the current approach is way better > > [1] > https://lkml.org/lkml/2013/3/12/241 > > > For the DT clock driver if needed for v3.10, how about just do a > > minimal drivers/clock/omap/ that uses the standard binding? > > Then that driver can just do clk_get() from cclock44xx_data.c > > I don't understand how to do it and why it is better than the current > approach. Well your approach is fine as a first step moving all the clock code, but it needs to be a real driver under drivers/clock/omap. And the DT binding needs to stay the same for the driver(s) in the long term as we start moving clocks to DT + /lib/firmware. If this all is too late for v3.10, I suggest you just set up the right clock alias for panda with machine_is_compatible flag in board-generic.c so we get EHCI working with DT for v3.10. Then it's easy to to deal with it properly for v3.11. > How can that driver do clk_get() from cclock44xx_data.c? > from where does it get the clk_id to pass into clk_get()? Can't you just use the clock name there to get it? > > for now? And then later on we'll just move all the clocks to a > > combination of DT + /lib/firmware. > > What is the benefit of moving clock data to /lib/firmware? We could > as well just move it to DT only, no? DT only clocks option is naturally available with this too. It just gets easily inefficient with such a huge number of clocks. > >> e.g. auxclk are required to be specified in DT nodes for USB PHY. > >> Without this we can't get USB host working on Panda. > > > > OK. So if the USB PHY has a dev entry, can't you just set up a > > clock alias in struct omap_clk omap44xx_clks[] for it? > > I've explained why this can't be done above. Yes I understand now, the clock is board specific. > >> As Rajendra points out, it seems moving entire clock data to DT is not > >> going to happen soon. So this is the simplistic way things can work. > > > > Right but seems like we can get started there without moving > > them all at once. > > > What if we provide a complete clock list instead of only auxclks in > dt_clks[]? > > This approach is similar to arch/arm/mach-imx/clk-imx35.c > > Device drivers can then use them as they migrate to DT. Then later > we could migrate clock data to DT, without impacting device drivers. As long as the binding stays the same in the long run too, this clock remapping approach is just fine as a starting point. And the driver needs to go to drivers/clock/omap. But in the long run we just want to get the huge amounts static data out of the kernel for clocks and hwmod data to fix things for good. Regards, Tony
On 04/04/2013 07:41 PM, Tony Lindgren wrote: > * Roger Quadros <rogerq@ti.com> [130404 00:39]: >> On 04/04/2013 02:42 AM, Tony Lindgren wrote: >>>> --- a/arch/arm/mach-omap2/cclock44xx_data.c >>>> +++ b/arch/arm/mach-omap2/cclock44xx_data.c >>>> @@ -27,6 +27,7 @@ >>>> #include <linux/clk-private.h> >>>> #include <linux/clkdev.h> >>>> #include <linux/io.h> >>>> +#include <linux/of.h> >>>> >>>> #include "soc.h" >>>> #include "iomap.h" >>>> @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { >>>> CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), >>>> }; >>>> >>>> +static struct clk *scrm_clks[] = { >>>> + &auxclk0_ck, >>>> + &auxclk1_ck, >>>> + &auxclk2_ck, >>>> + &auxclk3_ck, >>>> + &auxclk4_ck, >>>> + &auxclk5_ck, >>>> +}; >>> Hmm I don't like the idea of specifying the auxclk both in the >>> cclock44xx_data.c and in DT.. >> Right, but till we have all clocks moved to DT we only need this >> approach for general purpose clocks that are not mapped to devices >> by hwmod. > For v3.10, let's just make sure that USB works with DT as then > after v3.10 we can make omap4 DT only and get rid of estimated > 7K lines of code and data. I guess this is the last piece missing > for that, or are we also missing something else? > > Can't you set up a clock alias for your device so it can find the > auxclk when requesting it with the dev entry? > > For the DT clock driver if needed for v3.10, how about just do a > minimal drivers/clock/omap/ that uses the standard binding? > Then that driver can just do clk_get() from cclock44xx_data.c > for now? And then later on we'll just move all the clocks to a > combination of DT + /lib/firmware. > Hi Roger, Rajendra, All Sorry that disturbing you. I'm supporting Android OMAP4 kernels (K3.0/K3.4) and like to clarify few points regarding this approach (having into account possible future migrations on newer Kernels and OMAP5). If I understand everything right, this patch series allows to create clock binding in DT using following syntax: clocks = <&aux_clks 3> - does it means that in worst case there will be ~200 clock IDs defined? - does it means that clock nodes binding using phandles (human-friendly notation) isn't going to be supported? for example: clocks = <&sys_clkin_ck> clocks = <&dss_sys_clk &dss_tv_clk &dss_dss_clk>) I was horrified to think about the problems of this approach support (in case if there would be more then ~30 IDs) - just miss with on digit and weeks of debugging would be guaranteed. Please, say me that i'm wrong. And why clock DT data can't be auto-generated like all other OMAP data to close this questions? Thanks. >> e.g. auxclk are required to be specified in DT nodes for USB PHY. >> Without this we can't get USB host working on Panda. > OK. So if the USB PHY has a dev entry, can't you just set up a > clock alias in struct omap_clk omap44xx_clks[] for it? > >> As Rajendra points out, it seems moving entire clock data to DT is not >> going to happen soon. So this is the simplistic way things can work. > Right but seems like we can get started there without moving > them all at once. > > Regards, > > Tony > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/05/2013 06:58 PM, Tony Lindgren wrote: > * Roger Quadros <rogerq@ti.com> [130405 03:44]: >> On 04/04/2013 07:41 PM, Tony Lindgren wrote: >>> * Roger Quadros <rogerq@ti.com> [130404 00:39]: >>>> On 04/04/2013 02:42 AM, Tony Lindgren wrote: >>> For v3.10, let's just make sure that USB works with DT as then >>> after v3.10 we can make omap4 DT only and get rid of estimated >>> 7K lines of code and data. I guess this is the last piece missing >>> for that, or are we also missing something else? >> >> For panda we just need a way to map the auxclk to the USB PHY device >> and the relevant dts data to get USB host working with DT. >> Beagle USB host should work already with DT without any changes. >> >>> >>> Can't you set up a clock alias for your device so it can find the >>> auxclk when requesting it with the dev entry? >>> >> >> which clock is mapped to which PHY device depends on board design >> and that cannot be per-determined at one place. This information >> needs to come from the board.dts file. > > OK that makes sense. > >> There was an earlier attempt to provide a way of building clock aliases >> at runtime from device tree [1], but the current approach is way better >> >> [1] >> https://lkml.org/lkml/2013/3/12/241 >> >>> For the DT clock driver if needed for v3.10, how about just do a >>> minimal drivers/clock/omap/ that uses the standard binding? >>> Then that driver can just do clk_get() from cclock44xx_data.c >> >> I don't understand how to do it and why it is better than the current >> approach. > > Well your approach is fine as a first step moving all the clock > code, but it needs to be a real driver under drivers/clock/omap. > And the DT binding needs to stay the same for the driver(s) in the > long term as we start moving clocks to DT + /lib/firmware. The code needs to be there were the clock structs are defined. Currently they are in arch/arm/mach-omap2/cclock44xx_data.c for OMAP4. > > If this all is too late for v3.10, I suggest you just set up the > right clock alias for panda with machine_is_compatible flag in > board-generic.c so we get EHCI working with DT for v3.10. Then > it's easy to to deal with it properly for v3.11. OK, let's do it this way for Panda for 3.10. > >> How can that driver do clk_get() from cclock44xx_data.c? >> from where does it get the clk_id to pass into clk_get()? > > Can't you just use the clock name there to get it? In device tree we don't pass around clock names. You can either get a phandle or an index to the clock. e.g. Documentation/devicetree/bindings/clock/imx31-clock.txt > >>> for now? And then later on we'll just move all the clocks to a >>> combination of DT + /lib/firmware. >> >> What is the benefit of moving clock data to /lib/firmware? We could >> as well just move it to DT only, no? > > DT only clocks option is naturally available with this too. It > just gets easily inefficient with such a huge number of clocks. > OK. >>>> e.g. auxclk are required to be specified in DT nodes for USB PHY. >>>> Without this we can't get USB host working on Panda. >>> >>> OK. So if the USB PHY has a dev entry, can't you just set up a >>> clock alias in struct omap_clk omap44xx_clks[] for it? >> >> I've explained why this can't be done above. > > Yes I understand now, the clock is board specific. > >>>> As Rajendra points out, it seems moving entire clock data to DT is not >>>> going to happen soon. So this is the simplistic way things can work. >>> >>> Right but seems like we can get started there without moving >>> them all at once. >>> >> What if we provide a complete clock list instead of only auxclks in >> dt_clks[]? >> >> This approach is similar to arch/arm/mach-imx/clk-imx35.c >> >> Device drivers can then use them as they migrate to DT. Then later >> we could migrate clock data to DT, without impacting device drivers. > > As long as the binding stays the same in the long run too, this > clock remapping approach is just fine as a starting point. And > the driver needs to go to drivers/clock/omap. But in the long run > we just want to get the huge amounts static data out of the kernel > for clocks and hwmod data to fix things for good. In that case we need to identify what clocks need to be supported. If it is all (~200) of them, is this method good enough? cheers, -roger
On 04/05/2013 08:56 PM, Grygorii Strashko wrote: > On 04/04/2013 07:41 PM, Tony Lindgren wrote: >> * Roger Quadros <rogerq@ti.com> [130404 00:39]: >>> On 04/04/2013 02:42 AM, Tony Lindgren wrote: >>>>> --- a/arch/arm/mach-omap2/cclock44xx_data.c >>>>> +++ b/arch/arm/mach-omap2/cclock44xx_data.c >>>>> @@ -27,6 +27,7 @@ >>>>> #include <linux/clk-private.h> >>>>> #include <linux/clkdev.h> >>>>> #include <linux/io.h> >>>>> +#include <linux/of.h> >>>>> #include "soc.h" >>>>> #include "iomap.h" >>>>> @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { >>>>> CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), >>>>> }; >>>>> +static struct clk *scrm_clks[] = { >>>>> + &auxclk0_ck, >>>>> + &auxclk1_ck, >>>>> + &auxclk2_ck, >>>>> + &auxclk3_ck, >>>>> + &auxclk4_ck, >>>>> + &auxclk5_ck, >>>>> +}; >>>> Hmm I don't like the idea of specifying the auxclk both in the >>>> cclock44xx_data.c and in DT.. >>> Right, but till we have all clocks moved to DT we only need this >>> approach for general purpose clocks that are not mapped to devices >>> by hwmod. >> For v3.10, let's just make sure that USB works with DT as then >> after v3.10 we can make omap4 DT only and get rid of estimated >> 7K lines of code and data. I guess this is the last piece missing >> for that, or are we also missing something else? >> >> Can't you set up a clock alias for your device so it can find the >> auxclk when requesting it with the dev entry? >> >> For the DT clock driver if needed for v3.10, how about just do a >> minimal drivers/clock/omap/ that uses the standard binding? >> Then that driver can just do clk_get() from cclock44xx_data.c >> for now? And then later on we'll just move all the clocks to a >> combination of DT + /lib/firmware. >> > Hi Roger, Rajendra, All > > Sorry that disturbing you. Hi Grygorri, Nothing to disturb at all ;). > > I'm supporting Android OMAP4 kernels (K3.0/K3.4) and like to clarify few > points regarding this approach (having into account possible future migrations > on newer Kernels and OMAP5). > If I understand everything right, this patch series allows to create clock binding > in DT using following syntax: clocks = <&aux_clks 3> Actually in v2 of the patch this would be clocks = <&clks 3> > - does it means that in worst case there will be ~200 clock IDs defined? I'm afraid so yes. But when I wrote this I was only thinking of generic clocks, i.e. AUXCLKS. So when we start talking of all clocks we might need to reconsider the approach. > - does it means that clock nodes binding using phandles (human-friendly notation) > isn't going to be supported? > for example: > clocks = <&sys_clkin_ck> > clocks = <&dss_sys_clk &dss_tv_clk &dss_dss_clk>) This would depend if we define the clock nodes within DT or not. From what Tony mentioned (i.e. inefficiency) it seems that the clock nodes won't be defined in the device tree. Then we are left with using an index. > > I was horrified to think about the problems of this approach support > (in case if there would be more then ~30 IDs) - just miss with on digit > and weeks of debugging would be guaranteed. > > Please, say me that i'm wrong. It is still not written in stone so if you have a better idea we could go that route. cheers, -roger > And why clock DT data can't be auto-generated like all other OMAP data > to close this questions? > > Thanks. > >>> e.g. auxclk are required to be specified in DT nodes for USB PHY. >>> Without this we can't get USB host working on Panda. >> OK. So if the USB PHY has a dev entry, can't you just set up a >> clock alias in struct omap_clk omap44xx_clks[] for it? >> >>> As Rajendra points out, it seems moving entire clock data to DT is not >>> going to happen soon. So this is the simplistic way things can work. >> Right but seems like we can get started there without moving >> them all at once. >> >> Regards, >> >> Tony >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >
* Roger Quadros <rogerq@ti.com> [130409 03:00]: > On 04/05/2013 06:58 PM, Tony Lindgren wrote: > > > > Well your approach is fine as a first step moving all the clock > > code, but it needs to be a real driver under drivers/clock/omap. > > And the DT binding needs to stay the same for the driver(s) in the > > long term as we start moving clocks to DT + /lib/firmware. > > The code needs to be there were the clock structs are defined. > Currently they are in arch/arm/mach-omap2/cclock44xx_data.c for OMAP4. But if you do just a passthrough driver then that should not be needed. > > If this all is too late for v3.10, I suggest you just set up the > > right clock alias for panda with machine_is_compatible flag in > > board-generic.c so we get EHCI working with DT for v3.10. Then > > it's easy to to deal with it properly for v3.11. > > OK, let's do it this way for Panda for 3.10. Yes otherwise we'll be delaying omap4 DT conversion again. > >> How can that driver do clk_get() from cclock44xx_data.c? > >> from where does it get the clk_id to pass into clk_get()? > > > > Can't you just use the clock name there to get it? > > In device tree we don't pass around clock names. You can either get > a phandle or an index to the clock. > > e.g. Documentation/devicetree/bindings/clock/imx31-clock.txt Yes I understand that. But the driver/clock/omap driver can just remap the DT device initially so the board specific clock is found from the clock alias table. Basically initially a passthrough driver that can be enhanced to parse DT clock bindings and load data from /lib/firmware. > > As long as the binding stays the same in the long run too, this > > clock remapping approach is just fine as a starting point. And > > the driver needs to go to drivers/clock/omap. But in the long run > > we just want to get the huge amounts static data out of the kernel > > for clocks and hwmod data to fix things for good. > > In that case we need to identify what clocks need to be supported. > If it is all (~200) of them, is this method good enough? We should support any clock we need for booting the device with just DT bindings to get timers, console and rootfs working. Then we just need to load the complete set from /lib/firmware. It seems that the binding can be the same for all the clocks. For now, we can just use the standard clock binding and do the remapping in the clock driver. Regards, Tony
* Tony Lindgren <tony@atomide.com> [130409 09:54]: > * Roger Quadros <rogerq@ti.com> [130409 03:00]: > > On 04/05/2013 06:58 PM, Tony Lindgren wrote: > > > > > > Can't you just use the clock name there to get it? > > > > In device tree we don't pass around clock names. You can either get > > a phandle or an index to the clock. > > > > e.g. Documentation/devicetree/bindings/clock/imx31-clock.txt > > Yes I understand that. But the driver/clock/omap driver can just > remap the DT device initially so the board specific clock is > found from the clock alias table. Basically initially a passthrough > driver that can be enhanced to parse DT clock bindings and load > data from /lib/firmware. Actually probably the driver/clock/omap can even do even less initially. There probably even no need to remap clocks there. As long as the DT clock driver understands that a board specific auxclk is specified in the DT it can just call clk_add_alias() so the driver will get the right auxclk from cclock44xx_data.c. Then other features can be added later on like to allocate a clock entirely based on the binding etc. Regards, Tony
diff --git a/Documentation/devicetree/bindings/clock/omap4-clock.txt b/Documentation/devicetree/bindings/clock/omap4-clock.txt new file mode 100644 index 0000000..9d5448b --- /dev/null +++ b/Documentation/devicetree/bindings/clock/omap4-clock.txt @@ -0,0 +1,32 @@ +* Clock bindings for Texas Instruments OMAP4 SCRM clocks + +Required properties: +- compatible: Should be "ti,omap4-scrm" +- #clock-cells: Should be <1> + +The clock consumer should specify the desired clock by having the clock +ID in its "clocks" phandle cell. The following is a full list of SCRM +clocks and IDs. + + Clock ID + ------------------ + auxclk0_ck 0 + auxclk1_ck 1 + auxclk1_ck 1 + auxclk1_ck 1 + auxclk1_ck 1 + +Example: + +aux_clks: scrmclks { + compatible = "ti,omap4-scrm"; + #clock-cells = <1>; +}; + +hsusb1_phy: hsusb1_phy { + compatible = "usb-nop-xceiv"; + reset-supply = <&hsusb1_reset>; + clocks = <&aux_clks 3>; + clock-names = "main_clk"; + clock-frequency = <19200000>; +}; diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi index b7db1a2..97de56c 100644 --- a/arch/arm/boot/dts/omap4.dtsi +++ b/arch/arm/boot/dts/omap4.dtsi @@ -101,6 +101,11 @@ ti,hwmods = "counter_32k"; }; + aux_clks: scrmclks { + compatible = "ti,omap4-scrm"; + #clock-cells = <1>; + }; + omap4_pmx_core: pinmux@4a100040 { compatible = "ti,omap4-padconf", "pinctrl-single"; reg = <0x4a100040 0x0196>; diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c index 0274ff7..23f2064 100644 --- a/arch/arm/mach-omap2/board-generic.c +++ b/arch/arm/mach-omap2/board-generic.c @@ -158,7 +158,7 @@ DT_MACHINE_START(OMAP4_DT, "Generic OMAP4 (Flattened Device Tree)") .init_irq = omap_gic_of_init, .init_machine = omap_generic_init, .init_late = omap4430_init_late, - .init_time = omap4_local_timer_init, + .init_time = omap4_init_time, .dt_compat = omap4_boards_compat, .restart = omap44xx_restart, MACHINE_END diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c index 3d58f33..bfc46c1 100644 --- a/arch/arm/mach-omap2/cclock44xx_data.c +++ b/arch/arm/mach-omap2/cclock44xx_data.c @@ -27,6 +27,7 @@ #include <linux/clk-private.h> #include <linux/clkdev.h> #include <linux/io.h> +#include <linux/of.h> #include "soc.h" #include "iomap.h" @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), }; +static struct clk *scrm_clks[] = { + &auxclk0_ck, + &auxclk1_ck, + &auxclk2_ck, + &auxclk3_ck, + &auxclk4_ck, + &auxclk5_ck, +}; + +static struct clk_onecell_data scrm_data; + +#ifdef CONFIG_OF +int __init omap4_clk_init_dt(void) +{ + struct device_node *np; + + np = of_find_compatible_node(NULL, NULL, "ti,omap4-scrm"); + if (np) { + scrm_data.clks = scrm_clks; + scrm_data.clk_num = ARRAY_SIZE(scrm_clks); + of_clk_add_provider(np, of_clk_src_onecell_get, &scrm_data); + } + + return 0; +} + +#else + +int __init omap4_clk_init_dt(void) +{ + return 0; +} +#endif /* CONFIG_OF */ + int __init omap4xxx_clk_init(void) { u32 cpu_clkflg; diff --git a/arch/arm/mach-omap2/clock44xx.h b/arch/arm/mach-omap2/clock44xx.h index 287a46f..6395f63 100644 --- a/arch/arm/mach-omap2/clock44xx.h +++ b/arch/arm/mach-omap2/clock44xx.h @@ -16,5 +16,6 @@ #define OMAP4430_REGM4XEN_MULT 4 int omap4xxx_clk_init(void); +int omap4_clk_init_dt(void); #endif diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h index 0a6b9c7..1941d1c 100644 --- a/arch/arm/mach-omap2/common.h +++ b/arch/arm/mach-omap2/common.h @@ -98,6 +98,7 @@ void am35xx_init_early(void); void ti81xx_init_early(void); void am33xx_init_early(void); void omap4430_init_early(void); +void omap4_init_time(void); void omap5_init_early(void); void omap3_init_late(void); /* Do not use this one */ void omap4430_init_late(void); @@ -143,6 +144,14 @@ static inline void omap44xx_restart(char mode, const char *cmd) } #endif +#ifdef CONFIG_ARCH_OMAP4 +void omap4_init_time(void); +#else +static inline void omap4_init_time(void) +{ +} +#endif + /* This gets called from mach-omap2/io.c, do not call this */ void __init omap2_set_globals_tap(u32 class, void __iomem *tap); diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c index 2c3fdd6..c504363 100644 --- a/arch/arm/mach-omap2/io.c +++ b/arch/arm/mach-omap2/io.c @@ -603,6 +603,12 @@ void __init omap4430_init_late(void) omap4_pm_init(); omap2_clk_enable_autoidle_all(); } + +void __init omap4_init_time(void) +{ + omap4_clk_init_dt(); + omap4_local_timer_init(); +} #endif #ifdef CONFIG_SOC_OMAP5
Register a device tree clock provider for AUX clocks on the OMAP4 SoC. Also provide the binding information. Signed-off-by: Roger Quadros <rogerq@ti.com> --- .../devicetree/bindings/clock/omap4-clock.txt | 32 ++++++++++++++++++ arch/arm/boot/dts/omap4.dtsi | 5 +++ arch/arm/mach-omap2/board-generic.c | 2 +- arch/arm/mach-omap2/cclock44xx_data.c | 35 ++++++++++++++++++++ arch/arm/mach-omap2/clock44xx.h | 1 + arch/arm/mach-omap2/common.h | 9 +++++ arch/arm/mach-omap2/io.c | 6 +++ 7 files changed, 89 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/omap4-clock.txt