| Message ID | 20130405163254.GA7259@kahuna (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Nishanth Menon <nm@ti.com> [130405 09:37]: > > on the first step angle, > Applying current approach that Roger has taken: > Benoit's tree: > git://git.kernel.org/pub/scm/linux/kernel/git/bcousson/linux-omap-dt.git > for_3.10/dts d114294 ARM: dts: AM33XX: Corrects typo in interrupt field in SPI node > +: > https://patchwork.kernel.org/patch/2312211/ > https://patchwork.kernel.org/patch/2335671/ > And the diff below[1] as replacement for this patch - does indeed work on > PandaBoard. Now the question is which direction should I take - will > wait on decision in https://lkml.org/lkml/2013/4/5/173 thread. No need to wait AFAIK :) It seems we can already work on this: 1. Limit the DT clock use to the standard binding as defined in the Documentation/devicetree/bindings/clock/clock-bindings.txt until we have a clear plan of the possible additional bindings needed 2. Do a minimal drivers/clock/omap driver that remaps the bindings to the existing clocks. It seems that Roger already has all the code, it just needs to become a proper driver > I have similar issues: clock alias is a mess of a code to cleanup and keep alive > for mpu dplls - I have posted on this mail thread various variants of trying to > do this - there is no simple solution. I can see an issue there if the clocks are board specific, but what's the clock alias issue with mpu dplls? > If we cannot go any further, we are essentially stalled on moving > cpufreq (OPP entries along with it, regulators, voltage ....) to device > tree. Heh I doubt that :) > [1] > diff --git a/arch/arm/boot/dts/omap443x.dtsi b/arch/arm/boot/dts/omap443x.dtsi > index cccf39a..1fddb1e 100644 > --- a/arch/arm/boot/dts/omap443x.dtsi > +++ b/arch/arm/boot/dts/omap443x.dtsi > @@ -21,6 +21,8 @@ > 800000 1313000 > 1008000 1375000 > >; > + clocks = <&clks 6>; > + clock-names = "cpu"; > clock-latency = <300000>; /* From legacy driver */ > }; > }; So the clock-latency is non-standard here, but that's a separate issue. > 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); > + } > } Hmm why would the driver need this? Sounds like the driver is missing support for DT? > #ifdef CONFIG_SOC_OMAP2420 > diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c > index a93617b..54530d0 100644 > --- a/arch/arm/mach-omap2/cclock44xx_data.c > +++ b/arch/arm/mach-omap2/cclock44xx_data.c > @@ -1675,6 +1675,7 @@ static struct clk *dt_clks[] = { > &auxclk3_ck, > &auxclk4_ck, > &auxclk5_ck, > + &dpll_mpu_ck, > }; > > static struct clk_onecell_data clock_data; The dt_clks[] should be something that's allocated in the driver/clock/omap driver, let's not stuff that into cclock44xx_data.c. 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 10:05-20130405, Tony Lindgren wrote: > * Nishanth Menon <nm@ti.com> [130405 09:37]: > > > > on the first step angle, > > Applying current approach that Roger has taken: > > Benoit's tree: > > git://git.kernel.org/pub/scm/linux/kernel/git/bcousson/linux-omap-dt.git > > for_3.10/dts d114294 ARM: dts: AM33XX: Corrects typo in interrupt field in SPI node > > +: > > https://patchwork.kernel.org/patch/2312211/ > > https://patchwork.kernel.org/patch/2335671/ > > And the diff below[1] as replacement for this patch - does indeed work on > > PandaBoard. Now the question is which direction should I take - will > > wait on decision in https://lkml.org/lkml/2013/4/5/173 thread. > > No need to wait AFAIK :) It seems we can already work on this: /me perks ears :) > > 1. Limit the DT clock use to the standard binding as defined in > the Documentation/devicetree/bindings/clock/clock-bindings.txt > until we have a clear plan of the possible additional bindings > needed > > 2. Do a minimal drivers/clock/omap driver that remaps the bindings > to the existing clocks. It seems that Roger already has all > the code, it just needs to become a proper driver Cool, I can afford to wait for it if that is what I should build upon. > > > I have similar issues: clock alias is a mess of a code to cleanup and keep alive > > for mpu dplls - I have posted on this mail thread various variants of trying to > > do this - there is no simple solution. > > I can see an issue there if the clocks are board specific, but > what's the clock alias issue with mpu dplls? I highlighted it on this thread here: http://marc.info/?l=linux-pm&m=136504393718316&w=2 > > > If we cannot go any further, we are essentially stalled on moving > > cpufreq (OPP entries along with it, regulators, voltage ....) to device > > tree. > > Heh I doubt that :) I have to go top bottom -> do the entire dvfs track to be of any use.. I have already started an thread for voltage using regulator chaining here http://marc.info/?t=136513865200005&r=1&w=2 - need to dig in more and bring it along as well.. > > > [1] > > diff --git a/arch/arm/boot/dts/omap443x.dtsi b/arch/arm/boot/dts/omap443x.dtsi > > index cccf39a..1fddb1e 100644 > > --- a/arch/arm/boot/dts/omap443x.dtsi > > +++ b/arch/arm/boot/dts/omap443x.dtsi > > @@ -21,6 +21,8 @@ > > 800000 1313000 > > 1008000 1375000 > > >; > > + clocks = <&clks 6>; > > + clock-names = "cpu"; > > clock-latency = <300000>; /* From legacy driver */ > > }; > > }; > > So the clock-latency is non-standard here, but that's a > separate issue. yep. > > > 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); > > + } > > } > > Hmm why would the driver need this? Sounds like the driver is > missing support for DT? Nope, this was a long chain of discussion in previous iterations of this patch.. more or less started here: https://patchwork.kernel.org/patch/2251821/ Suggested as the generic approach for cpufreq drivers. Paul questioned this approach in: http://marc.info/?l=linux-pm&m=136485349218809&w=2 > > > #ifdef CONFIG_SOC_OMAP2420 > > diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c > > index a93617b..54530d0 100644 > > --- a/arch/arm/mach-omap2/cclock44xx_data.c > > +++ b/arch/arm/mach-omap2/cclock44xx_data.c > > @@ -1675,6 +1675,7 @@ static struct clk *dt_clks[] = { > > &auxclk3_ck, > > &auxclk4_ck, > > &auxclk5_ck, > > + &dpll_mpu_ck, > > }; > > > > static struct clk_onecell_data clock_data; > > The dt_clks[] should be something that's allocated in the > driver/clock/omap driver, let's not stuff that into > cclock44xx_data.c. I agree, I dont like it either - just following Roger's hint here.
* Nishanth Menon <nm@ti.com> [130405 10:22]: > On 10:05-20130405, Tony Lindgren wrote: > > > > > 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); > > > + } > > > } > > > > Hmm why would the driver need this? Sounds like the driver is > > missing support for DT? > Nope, this was a long chain of discussion in previous iterations of this > patch.. more or less started here: > https://patchwork.kernel.org/patch/2251821/ > Suggested as the generic approach for cpufreq drivers. > Paul questioned this approach in: > http://marc.info/?l=linux-pm&m=136485349218809&w=2 How about just set it up in omap2_common_pm_init instead of the board-generic? 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 12:28-20130405, Tony Lindgren wrote: > * Nishanth Menon <nm@ti.com> [130405 10:22]: > > On 10:05-20130405, Tony Lindgren wrote: > > > > > > > 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); > > > > + } > > > > } > > > > > > Hmm why would the driver need this? Sounds like the driver is > > > missing support for DT? > > Nope, this was a long chain of discussion in previous iterations of this > > patch.. more or less started here: > > https://patchwork.kernel.org/patch/2251821/ > > Suggested as the generic approach for cpufreq drivers. > > Paul questioned this approach in: > > http://marc.info/?l=linux-pm&m=136485349218809&w=2 > > How about just set it up in omap2_common_pm_init instead > of the board-generic? umm.. We want to eventually want to get rid of mach-omap2/pm.c (all those create processor devices etc should go away with proper representation of devices as nodes in DT if possible. But, I think you mean something like in the "else" condition of https://patchwork.kernel.org/patch/2359711/ ? - I'd again have the same request of not having anything to do with pm.c and keeping as little as possible for all TI processors in mach-omap2. Could you enlighten me about why you'd not like it in board-generic.c? will creating an function ti_generic_cpufreq_init() in board-generic.c and calling it from omap_generic_init help?
* Nishanth Menon <nm@ti.com> [130405 13:06]: > On 12:28-20130405, Tony Lindgren wrote: > > * Nishanth Menon <nm@ti.com> [130405 10:22]: > > > On 10:05-20130405, Tony Lindgren wrote: > > > > > > > > > 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); > > > > > + } > > > > > } > > > > > > > > Hmm why would the driver need this? Sounds like the driver is > > > > missing support for DT? > > > Nope, this was a long chain of discussion in previous iterations of this > > > patch.. more or less started here: > > > https://patchwork.kernel.org/patch/2251821/ > > > Suggested as the generic approach for cpufreq drivers. > > > Paul questioned this approach in: > > > http://marc.info/?l=linux-pm&m=136485349218809&w=2 > > > > How about just set it up in omap2_common_pm_init instead > > of the board-generic? > umm.. We want to eventually want to get rid of mach-omap2/pm.c (all > those create processor devices etc should go away with proper > representation of devices as nodes in DT if possible. > But, I think you mean something like in the "else" condition of > https://patchwork.kernel.org/patch/2359711/ ? - I'd again have the same > request of not having anything to do with pm.c and keeping as little as > possible for all TI processors in mach-omap2. > > Could you enlighten me about why you'd not like it in board-generic.c? > will creating an function ti_generic_cpufreq_init() in board-generic.c > and calling it from omap_generic_init help? I'd like to keep board-generic.c down to minimum. Can't you set it up in omap_init_cpufreq() in your second patch of this series? 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
diff --git a/arch/arm/boot/dts/omap443x.dtsi b/arch/arm/boot/dts/omap443x.dtsi index cccf39a..1fddb1e 100644 --- a/arch/arm/boot/dts/omap443x.dtsi +++ b/arch/arm/boot/dts/omap443x.dtsi @@ -21,6 +21,8 @@ 800000 1313000 1008000 1375000 >; + clocks = <&clks 6>; + clock-names = "cpu"; clock-latency = <300000>; /* From legacy driver */ }; }; 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/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c index a93617b..54530d0 100644 --- a/arch/arm/mach-omap2/cclock44xx_data.c +++ b/arch/arm/mach-omap2/cclock44xx_data.c @@ -1675,6 +1675,7 @@ static struct clk *dt_clks[] = { &auxclk3_ck, &auxclk4_ck, &auxclk5_ck, + &dpll_mpu_ck, }; static struct clk_onecell_data clock_data;