| Message ID | 1364507576-19345-2-git-send-email-nm@ti.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Nishanth Menon <nm@ti.com> writes: > With OMAP3+ and AM33xx supported SoC having defined CPU device tree > entries with operating-points defined, we can now use the SoC > generic cpufreq-cpu0 driver by registering appropriate device. > > As part of this change, add dummy clock node to use cpufreq-cpu0. > This is an suggested solution till we have OMAP clock nodes in device > tree. > Once the OMAP device tree conversion is complete, we can then do: > clocks = <&dpll_mpu_ck>; or the SoC specific equivalent. > Inspired by patch: https://patchwork.kernel.org/patch/2067841/ > now made generic. > > Cc: Kevin Hilman <khilman@linaro.org> > Cc: Rajendra Nayak <rnayak@ti.com> > Cc: Paul Walmsley <paul@pwsan.com> > Cc: "Benoît Cousson" <b-cousson@ti.com> > Cc: Jon Hunter <jon-hunter@ti.com> > Cc: Keerthy <j-keerthy@ti.com> > Cc: Santosh Shilimkar <santosh.shilimkar@ti.com> > Cc: Shawn Guo <shawn.guo@linaro.org> > Signed-off-by: Nishanth Menon <nm@ti.com> One more thought on this patch... > --- > Changes in v3: > - modified CC list. > - no functional change > V2: https://patchwork.kernel.org/patch/2303471/ > V1: https://patchwork.kernel.org/patch/2273571/ > > arch/arm/mach-omap2/board-generic.c | 5 +++++ > arch/arm/mach-omap2/cclock33xx_data.c | 2 +- > arch/arm/mach-omap2/cclock3xxx_data.c | 3 ++- > arch/arm/mach-omap2/cclock44xx_data.c | 3 ++- > 4 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c > index afa509a..5b147ef 100644 > --- a/arch/arm/mach-omap2/board-generic.c > +++ b/arch/arm/mach-omap2/board-generic.c > @@ -49,6 +49,11 @@ static void __init omap_generic_init(void) > omap4_panda_display_init_of(); > else if (of_machine_is_compatible("ti,omap4-sdp")) > omap_4430sdp_display_init_of(); > + > + if (IS_ENABLED(CONFIG_GENERIC_CPUFREQ_CPU0)) { > + struct platform_device_info devinfo = { .name = "cpufreq-cpu0", }; > + platform_device_register_full(&devinfo); > + } Rather than adding new clkdev nodes below, how about using clk add_alias here? Kevin > } > > #ifdef CONFIG_SOC_OMAP2420 > diff --git a/arch/arm/mach-omap2/cclock33xx_data.c b/arch/arm/mach-omap2/cclock33xx_data.c > index 476b820..cf7e736 100644 > --- a/arch/arm/mach-omap2/cclock33xx_data.c > +++ b/arch/arm/mach-omap2/cclock33xx_data.c > @@ -852,7 +852,7 @@ static struct omap_clk am33xx_clks[] = { > CLK(NULL, "dpll_core_m5_ck", &dpll_core_m5_ck, CK_AM33XX), > CLK(NULL, "dpll_core_m6_ck", &dpll_core_m6_ck, CK_AM33XX), > CLK(NULL, "dpll_mpu_ck", &dpll_mpu_ck, CK_AM33XX), > - CLK("cpu0", NULL, &dpll_mpu_ck, CK_AM33XX), > + CLK("cpufreq-cpu0.0", NULL, &dpll_mpu_ck, CK_AM33XX), > CLK(NULL, "dpll_mpu_m2_ck", &dpll_mpu_m2_ck, CK_AM33XX), > CLK(NULL, "dpll_ddr_ck", &dpll_ddr_ck, CK_AM33XX), > CLK(NULL, "dpll_ddr_m2_ck", &dpll_ddr_m2_ck, CK_AM33XX), > diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c > index 4579c3c..5a5b471 100644 > --- a/arch/arm/mach-omap2/cclock3xxx_data.c > +++ b/arch/arm/mach-omap2/cclock3xxx_data.c > @@ -3501,7 +3501,8 @@ static struct omap_clk omap3xxx_clks[] = { > CLK(NULL, "uart4_ick", &uart4_ick_am35xx, CK_AM35XX), > CLK(NULL, "timer_32k_ck", &omap_32k_fck, CK_3XXX), > CLK(NULL, "timer_sys_ck", &sys_ck, CK_3XXX), > - CLK(NULL, "cpufreq_ck", &dpll1_ck, CK_3XXX), > + CLK(NULL, "cpufreq_ck", &dpll1_ck, CK_3XXX), /* used in non-device tree boot */ > + CLK("cpufreq-cpu0.0", NULL, &dpll1_ck, CK_3XXX), /* used in device tree boot */ > }; > > static const char *enable_init_clks[] = { > diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c > index 3d58f33..bcea785 100644 > --- a/arch/arm/mach-omap2/cclock44xx_data.c > +++ b/arch/arm/mach-omap2/cclock44xx_data.c > @@ -1660,7 +1660,8 @@ static struct omap_clk omap44xx_clks[] = { > CLK("4013a000.timer", "timer_sys_ck", &syc_clk_div_ck, CK_443X), > CLK("4013c000.timer", "timer_sys_ck", &syc_clk_div_ck, CK_443X), > CLK("4013e000.timer", "timer_sys_ck", &syc_clk_div_ck, CK_443X), > - CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), > + CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), /* used in non-device tree boot */ > + CLK("cpufreq-cpu0.0", NULL, &dpll_mpu_ck, CK_443X), /* used in device tree boot */ > }; > > int __init omap4xxx_clk_init(void) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11:47-20130403, Kevin Hilman wrote: > Nishanth Menon <nm@ti.com> writes: > [...] > > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c > > index afa509a..5b147ef 100644 > > --- a/arch/arm/mach-omap2/board-generic.c > > +++ b/arch/arm/mach-omap2/board-generic.c > > @@ -49,6 +49,11 @@ static void __init omap_generic_init(void) > > omap4_panda_display_init_of(); > > else if (of_machine_is_compatible("ti,omap4-sdp")) > > omap_4430sdp_display_init_of(); > > + > > + if (IS_ENABLED(CONFIG_GENERIC_CPUFREQ_CPU0)) { > > + struct platform_device_info devinfo = { .name = "cpufreq-cpu0", }; > > + platform_device_register_full(&devinfo); > > + } > > Rather than adding new clkdev nodes below, how about using clk add_alias > here? Thanks for pointing this out, I spend some time implementing such a scheme and following is my opinion: Summary: There is one major problem which forces us to introduce this "clock hack" - clock nodes are not in device tree yet. Yes, clock add alias can be used (see option b,c,d..) - but the maintenance burden while we transition into DT based clock node is just not worth the effort. The current patch(option a) seems to be least of the compromise approaches available, IMHO. (testing example: options a, b, c all generate the log: http://pastebin.com/dJe7G9Q9 with the updated test script: http://pastebin.com/c3ZiU2Ng (now prints voltage as well)) So this means we have to be able to choose one of the available hacks: option a) - add clock nodes in cclock_xyz_data.c the current patch under discussion. Pros: - we keep board-generic.c intact - DT entries as needed example: clocks = <&dpll1>; can be used. So conversion of DT clock nodes and delete of cclock_xyz_data.c could be done in a smooth manner - we do can continue to support with the same code some TI platforms which have been transitioned to DT clock nodes, while in the same code others remain with _data.c and at the end of complete transition we dont need to cleanup code. - Allows us to have different DPLL names for controlling cpu0 clock as SoC needs in DT/_data.c Cons: yes, we have those ugly clock entries in clock_xyz_data.c files - but it anyways needs an HACK to work with current data files - why spread the hack around to other files and DT as well? option b) using *only* clock alias cpufreq_ck http://pastebin.com/BHLNsfib Pros: - we could maintain clock_xyz_data.c without much modification Cons: - forced to maintain cpufreq_ck clock node even for DT conversion - tons of cleanup in code, DT entries to be done while we are in-transition and completion of transition to DT clock nodes. option c) detect if clock node is populated, else use clock alias: http://pastebin.com/WpP8CSL8 Pros: - we could add DT nodes without cleanup later on. Cons: - replicated code (which needs to be eventually cleaned up) just to detect cpu nodes with clock nodes registered - cleanup needs to be done anyways in Pros: - we could maintain clock_xyz_data.c without much modification Cons: - forced to maintain cpufreq_ck clock node even for DT conversion - tons of cleanup in code, DT entries to be done while we are in-transition and completion of transition to DT clock nodes. option d) pass the dummy pdev created for cpufreq to clock_xyz_data init/ io.c early_init. I did not implement this, but is an theoretical possibility we create the clock nodes in early_init, yeah we could register the platform_device and pass the node over to clock_xyz_data as an option, but that just means we have to cleanup in more than one place now - board-generic, io.c, clock_xyz_data.c etc.. [...] > > diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c > > index 4579c3c..5a5b471 100644 > > --- a/arch/arm/mach-omap2/cclock3xxx_data.c > > +++ b/arch/arm/mach-omap2/cclock3xxx_data.c > > @@ -3501,7 +3501,8 @@ static struct omap_clk omap3xxx_clks[] = { > > CLK(NULL, "uart4_ick", &uart4_ick_am35xx, CK_AM35XX), > > CLK(NULL, "timer_32k_ck", &omap_32k_fck, CK_3XXX), > > CLK(NULL, "timer_sys_ck", &sys_ck, CK_3XXX), > > - CLK(NULL, "cpufreq_ck", &dpll1_ck, CK_3XXX), > > + CLK(NULL, "cpufreq_ck", &dpll1_ck, CK_3XXX), /* used in non-device tree boot */ > > + CLK("cpufreq-cpu0.0", NULL, &dpll1_ck, CK_3XXX), /* used in device tree boot */ > > }; [...]
On Thursday 04 April 2013 08:22 AM, Nishanth Menon wrote: > On 11:47-20130403, Kevin Hilman wrote: >> Nishanth Menon <nm@ti.com> writes: >> > [...] >>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c >>> index afa509a..5b147ef 100644 >>> --- a/arch/arm/mach-omap2/board-generic.c >>> +++ b/arch/arm/mach-omap2/board-generic.c >>> @@ -49,6 +49,11 @@ static void __init omap_generic_init(void) >>> omap4_panda_display_init_of(); >>> else if (of_machine_is_compatible("ti,omap4-sdp")) >>> omap_4430sdp_display_init_of(); >>> + >>> + if (IS_ENABLED(CONFIG_GENERIC_CPUFREQ_CPU0)) { >>> + struct platform_device_info devinfo = { .name = "cpufreq-cpu0", }; >>> + platform_device_register_full(&devinfo); >>> + } >> >> Rather than adding new clkdev nodes below, how about using clk add_alias >> here? > Thanks for pointing this out, I spend some time implementing such a > scheme and following is my opinion: > > Summary: > There is one major problem which forces us to introduce this "clock > hack" - clock nodes are not in device tree yet. Yes, clock add alias There's already a patch floating around for this.. https://lkml.org/lkml/2013/4/2/84 > can be used (see option b,c,d..) - but the maintenance burden while we > transition into DT based clock node is just not worth the effort. The > current patch(option a) seems to be least of the compromise approaches > available, IMHO. > > (testing example: > options a, b, c all generate the log: http://pastebin.com/dJe7G9Q9 > with the updated test script: http://pastebin.com/c3ZiU2Ng (now prints > voltage as well)) > > So this means we have to be able to choose one of the available hacks: > option a) - add clock nodes in cclock_xyz_data.c > the current patch under discussion. > Pros: > - we keep board-generic.c intact > - DT entries as needed example: > clocks = <&dpll1>; can be used. So conversion of DT clock nodes > and delete of cclock_xyz_data.c could be done in a smooth manner > - we do can continue to support with the same code some TI > platforms which have been transitioned to DT clock nodes, while > in the same code others remain with _data.c and at the end of > complete transition we dont need to cleanup code. > - Allows us to have different DPLL names for controlling cpu0 > clock as SoC needs in DT/_data.c > Cons: > yes, we have those ugly clock entries in clock_xyz_data.c files > - but it anyways needs an HACK to work with current data files - > why spread the hack around to other files and DT as well? > > option b) using *only* clock alias cpufreq_ck > http://pastebin.com/BHLNsfib > Pros: > - we could maintain clock_xyz_data.c without much modification > Cons: > - forced to maintain cpufreq_ck clock node even for DT > conversion > - tons of cleanup in code, DT entries to be done while we > are in-transition and completion of transition to DT clock > nodes. > > option c) detect if clock node is populated, else use clock alias: > http://pastebin.com/WpP8CSL8 > Pros: > - we could add DT nodes without cleanup later on. > Cons: > - replicated code (which needs to be eventually cleaned up) just > to detect cpu nodes with clock nodes registered > - cleanup needs to be done anyways in > Pros: > - we could maintain clock_xyz_data.c without much modification > Cons: > - forced to maintain cpufreq_ck clock node even for DT > conversion > - tons of cleanup in code, DT entries to be done while we > are in-transition and completion of transition to DT clock > nodes. > > option d) pass the dummy pdev created for cpufreq to clock_xyz_data > init/ io.c early_init. > I did not implement this, but is an theoretical possibility > we create the clock nodes in early_init, yeah we could register > the platform_device and pass the node over to clock_xyz_data as > an option, but that just means we have to cleanup in more than > one place now - board-generic, io.c, clock_xyz_data.c etc.. > > [...] >>> diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c >>> index 4579c3c..5a5b471 100644 >>> --- a/arch/arm/mach-omap2/cclock3xxx_data.c >>> +++ b/arch/arm/mach-omap2/cclock3xxx_data.c >>> @@ -3501,7 +3501,8 @@ static struct omap_clk omap3xxx_clks[] = { >>> CLK(NULL, "uart4_ick", &uart4_ick_am35xx, CK_AM35XX), >>> CLK(NULL, "timer_32k_ck", &omap_32k_fck, CK_3XXX), >>> CLK(NULL, "timer_sys_ck", &sys_ck, CK_3XXX), >>> - CLK(NULL, "cpufreq_ck", &dpll1_ck, CK_3XXX), >>> + CLK(NULL, "cpufreq_ck", &dpll1_ck, CK_3XXX), /* used in non-device tree boot */ >>> + CLK("cpufreq-cpu0.0", NULL, &dpll1_ck, CK_3XXX), /* used in device tree boot */ >>> }; > [...] > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c index afa509a..5b147ef 100644 --- a/arch/arm/mach-omap2/board-generic.c +++ b/arch/arm/mach-omap2/board-generic.c @@ -49,6 +49,11 @@ static void __init omap_generic_init(void) omap4_panda_display_init_of(); else if (of_machine_is_compatible("ti,omap4-sdp")) omap_4430sdp_display_init_of(); + + if (IS_ENABLED(CONFIG_GENERIC_CPUFREQ_CPU0)) { + struct platform_device_info devinfo = { .name = "cpufreq-cpu0", }; + platform_device_register_full(&devinfo); + } } #ifdef CONFIG_SOC_OMAP2420 diff --git a/arch/arm/mach-omap2/cclock33xx_data.c b/arch/arm/mach-omap2/cclock33xx_data.c index 476b820..cf7e736 100644 --- a/arch/arm/mach-omap2/cclock33xx_data.c +++ b/arch/arm/mach-omap2/cclock33xx_data.c @@ -852,7 +852,7 @@ static struct omap_clk am33xx_clks[] = { CLK(NULL, "dpll_core_m5_ck", &dpll_core_m5_ck, CK_AM33XX), CLK(NULL, "dpll_core_m6_ck", &dpll_core_m6_ck, CK_AM33XX), CLK(NULL, "dpll_mpu_ck", &dpll_mpu_ck, CK_AM33XX), - CLK("cpu0", NULL, &dpll_mpu_ck, CK_AM33XX), + CLK("cpufreq-cpu0.0", NULL, &dpll_mpu_ck, CK_AM33XX), CLK(NULL, "dpll_mpu_m2_ck", &dpll_mpu_m2_ck, CK_AM33XX), CLK(NULL, "dpll_ddr_ck", &dpll_ddr_ck, CK_AM33XX), CLK(NULL, "dpll_ddr_m2_ck", &dpll_ddr_m2_ck, CK_AM33XX), diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c index 4579c3c..5a5b471 100644 --- a/arch/arm/mach-omap2/cclock3xxx_data.c +++ b/arch/arm/mach-omap2/cclock3xxx_data.c @@ -3501,7 +3501,8 @@ static struct omap_clk omap3xxx_clks[] = { CLK(NULL, "uart4_ick", &uart4_ick_am35xx, CK_AM35XX), CLK(NULL, "timer_32k_ck", &omap_32k_fck, CK_3XXX), CLK(NULL, "timer_sys_ck", &sys_ck, CK_3XXX), - CLK(NULL, "cpufreq_ck", &dpll1_ck, CK_3XXX), + CLK(NULL, "cpufreq_ck", &dpll1_ck, CK_3XXX), /* used in non-device tree boot */ + CLK("cpufreq-cpu0.0", NULL, &dpll1_ck, CK_3XXX), /* used in device tree boot */ }; static const char *enable_init_clks[] = { diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c index 3d58f33..bcea785 100644 --- a/arch/arm/mach-omap2/cclock44xx_data.c +++ b/arch/arm/mach-omap2/cclock44xx_data.c @@ -1660,7 +1660,8 @@ static struct omap_clk omap44xx_clks[] = { CLK("4013a000.timer", "timer_sys_ck", &syc_clk_div_ck, CK_443X), CLK("4013c000.timer", "timer_sys_ck", &syc_clk_div_ck, CK_443X), CLK("4013e000.timer", "timer_sys_ck", &syc_clk_div_ck, CK_443X), - CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), + CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), /* used in non-device tree boot */ + CLK("cpufreq-cpu0.0", NULL, &dpll_mpu_ck, CK_443X), /* used in device tree boot */ }; int __init omap4xxx_clk_init(void)
With OMAP3+ and AM33xx supported SoC having defined CPU device tree entries with operating-points defined, we can now use the SoC generic cpufreq-cpu0 driver by registering appropriate device. As part of this change, add dummy clock node to use cpufreq-cpu0. This is an suggested solution till we have OMAP clock nodes in device tree. Once the OMAP device tree conversion is complete, we can then do: clocks = <&dpll_mpu_ck>; or the SoC specific equivalent. Inspired by patch: https://patchwork.kernel.org/patch/2067841/ now made generic. Cc: Kevin Hilman <khilman@linaro.org> Cc: Rajendra Nayak <rnayak@ti.com> Cc: Paul Walmsley <paul@pwsan.com> Cc: "Benoît Cousson" <b-cousson@ti.com> Cc: Jon Hunter <jon-hunter@ti.com> Cc: Keerthy <j-keerthy@ti.com> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com> Cc: Shawn Guo <shawn.guo@linaro.org> Signed-off-by: Nishanth Menon <nm@ti.com> --- Changes in v3: - modified CC list. - no functional change V2: https://patchwork.kernel.org/patch/2303471/ V1: https://patchwork.kernel.org/patch/2273571/ arch/arm/mach-omap2/board-generic.c | 5 +++++ arch/arm/mach-omap2/cclock33xx_data.c | 2 +- arch/arm/mach-omap2/cclock3xxx_data.c | 3 ++- arch/arm/mach-omap2/cclock44xx_data.c | 3 ++- 4 files changed, 10 insertions(+), 3 deletions(-)