Patchwork [2/8] ARM: dts: OMAP36xx: move CPU OPP tables to device tree

login
register
mail settings
Submitter Nishanth Menon
Date March 14, 2013, 8:58 p.m.
Message ID <1363294695-658-3-git-send-email-nm@ti.com>
Download mbox | patch
Permalink /patch/227799/
State New
Headers show

Comments

Nishanth Menon - March 14, 2013, 8:58 p.m.
Add DT OPP table for OMAP36xx family of devices. This data is
decoded by OF with of_init_opp_table() helper function. This
overrides the default OMAP34xx CPU OPP table definition.

This is in preparation to use generic cpu0-cpufreq driver.

Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: "BenoƮt Cousson" <b-cousson@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Keerthy <j-keerthy@ti.com>
Cc: linux-omap@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: cpufreq@vger.kernel.org
Cc: linux-pm@vger.kernel.org

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/boot/dts/omap36xx.dtsi    |   12 ++++++++++++
 arch/arm/mach-omap2/opp3xxx_data.c |    9 ---------
 2 files changed, 12 insertions(+), 9 deletions(-)
Hunter, Jon - March 14, 2013, 9:44 p.m.
On 03/14/2013 03:58 PM, Nishanth Menon wrote:
> Add DT OPP table for OMAP36xx family of devices. This data is
> decoded by OF with of_init_opp_table() helper function. This
> overrides the default OMAP34xx CPU OPP table definition.

Not sure I following the last sentence. The tables are in a different
dtsi file and only the relevant file should be included, right?

Jon
Nishanth Menon - March 15, 2013, 1:56 p.m.
On 16:44-20130314, Jon Hunter wrote:
> 
> On 03/14/2013 03:58 PM, Nishanth Menon wrote:
> > Add DT OPP table for OMAP36xx family of devices. This data is
> > decoded by OF with of_init_opp_table() helper function. This
> > overrides the default OMAP34xx CPU OPP table definition.
> 
> Not sure I following the last sentence. The tables are in a different
> dtsi file and only the relevant file should be included, right?
omap3630.dsi includes omap3.dtsi (which is meant for OMAP34xx).
The opp tables introduced by this patch in omap36xx.dtsi will override
the ones defined on omap3.dtsi. Will the following rephrase help clarify
this?

Original:
This overrides the default OMAP34xx CPU OPP table definition.
Suggested;
This overrides the default OMAP34xx CPU OPP table definition in
omap3.dtsi.
Hunter, Jon - March 15, 2013, 2:26 p.m.
On 03/15/2013 08:56 AM, Nishanth Menon wrote:
> On 16:44-20130314, Jon Hunter wrote:
>>
>> On 03/14/2013 03:58 PM, Nishanth Menon wrote:
>>> Add DT OPP table for OMAP36xx family of devices. This data is
>>> decoded by OF with of_init_opp_table() helper function. This
>>> overrides the default OMAP34xx CPU OPP table definition.
>>
>> Not sure I following the last sentence. The tables are in a different
>> dtsi file and only the relevant file should be included, right?
> omap3630.dsi includes omap3.dtsi (which is meant for OMAP34xx).
> The opp tables introduced by this patch in omap36xx.dtsi will override
> the ones defined on omap3.dtsi. Will the following rephrase help clarify
> this?
> 
> Original:
> This overrides the default OMAP34xx CPU OPP table definition.
> Suggested;
> This overrides the default OMAP34xx CPU OPP table definition in
> omap3.dtsi.

Sorry, I just missed that the omap3430 opps were in omap3.dtsi and not
omap34xx.dtsi. I guess I am not familiar with how the DTC overrides
nodes, however, at least from a readability standpoint it would seem
nice to have the omap3430 opps in a omap3430 specific dtsi and not
omap3.dtsi. However, thats just my opinion.

Cheers
Jon
Nishanth Menon - March 15, 2013, 2:38 p.m.
On 09:26-20130315, Jon Hunter wrote:
> 
> On 03/15/2013 08:56 AM, Nishanth Menon wrote:
> > On 16:44-20130314, Jon Hunter wrote:
> >>
> >> On 03/14/2013 03:58 PM, Nishanth Menon wrote:
> >>> Add DT OPP table for OMAP36xx family of devices. This data is
> >>> decoded by OF with of_init_opp_table() helper function. This
> >>> overrides the default OMAP34xx CPU OPP table definition.
> >>
> >> Not sure I following the last sentence. The tables are in a different
> >> dtsi file and only the relevant file should be included, right?
> > omap3630.dsi includes omap3.dtsi (which is meant for OMAP34xx).
> > The opp tables introduced by this patch in omap36xx.dtsi will override
> > the ones defined on omap3.dtsi. Will the following rephrase help clarify
> > this?
> > 
> > Original:
> > This overrides the default OMAP34xx CPU OPP table definition.
> > Suggested;
> > This overrides the default OMAP34xx CPU OPP table definition in
> > omap3.dtsi.
> 
> Sorry, I just missed that the omap3430 opps were in omap3.dtsi and not
> omap34xx.dtsi. I guess I am not familiar with how the DTC overrides
> nodes, however, at least from a readability standpoint it would seem
> nice to have the omap3430 opps in a omap3430 specific dtsi and not
> omap3.dtsi. However, thats just my opinion.
most of omap3630 is based off omap3430. I know from an readability point
of view, it might have been good to split that to omap3-common.dtsi,
omap34xx.dtsi, omap36xx.dtsi etc.. But there is no real need at this
point in time to do that. Unless, ofcourse, we'd like to set that up as
an standard for all OMAP SoCs...
Hunter, Jon - March 15, 2013, 2:58 p.m.
On 03/15/2013 09:38 AM, Nishanth Menon wrote:
> On 09:26-20130315, Jon Hunter wrote:
>>
>> On 03/15/2013 08:56 AM, Nishanth Menon wrote:
>>> On 16:44-20130314, Jon Hunter wrote:
>>>>
>>>> On 03/14/2013 03:58 PM, Nishanth Menon wrote:
>>>>> Add DT OPP table for OMAP36xx family of devices. This data is
>>>>> decoded by OF with of_init_opp_table() helper function. This
>>>>> overrides the default OMAP34xx CPU OPP table definition.
>>>>
>>>> Not sure I following the last sentence. The tables are in a different
>>>> dtsi file and only the relevant file should be included, right?
>>> omap3630.dsi includes omap3.dtsi (which is meant for OMAP34xx).
>>> The opp tables introduced by this patch in omap36xx.dtsi will override
>>> the ones defined on omap3.dtsi. Will the following rephrase help clarify
>>> this?
>>>
>>> Original:
>>> This overrides the default OMAP34xx CPU OPP table definition.
>>> Suggested;
>>> This overrides the default OMAP34xx CPU OPP table definition in
>>> omap3.dtsi.
>>
>> Sorry, I just missed that the omap3430 opps were in omap3.dtsi and not
>> omap34xx.dtsi. I guess I am not familiar with how the DTC overrides
>> nodes, however, at least from a readability standpoint it would seem
>> nice to have the omap3430 opps in a omap3430 specific dtsi and not
>> omap3.dtsi. However, thats just my opinion.
> most of omap3630 is based off omap3430. I know from an readability point
> of view, it might have been good to split that to omap3-common.dtsi,
> omap34xx.dtsi, omap36xx.dtsi etc.. But there is no real need at this
> point in time to do that. Unless, ofcourse, we'd like to set that up as
> an standard for all OMAP SoCs...

How would omap3-common.dtsi be any different from omap3.dtsi? I don't
wish us to go nuts with creating dtsi files either, but having an
omap3430.dtsi does not seem unreasonable to me, but that is just my opinion.

Jon
Nishanth Menon - March 15, 2013, 3:02 p.m.
On 09:58-20130315, Jon Hunter wrote:
> 
> On 03/15/2013 09:38 AM, Nishanth Menon wrote:
> > On 09:26-20130315, Jon Hunter wrote:
> >>
> >> On 03/15/2013 08:56 AM, Nishanth Menon wrote:
> >>> On 16:44-20130314, Jon Hunter wrote:
> >>>>
> >>>> On 03/14/2013 03:58 PM, Nishanth Menon wrote:
> >>>>> Add DT OPP table for OMAP36xx family of devices. This data is
> >>>>> decoded by OF with of_init_opp_table() helper function. This
> >>>>> overrides the default OMAP34xx CPU OPP table definition.
> >>>>
> >>>> Not sure I following the last sentence. The tables are in a different
> >>>> dtsi file and only the relevant file should be included, right?
> >>> omap3630.dsi includes omap3.dtsi (which is meant for OMAP34xx).
> >>> The opp tables introduced by this patch in omap36xx.dtsi will override
> >>> the ones defined on omap3.dtsi. Will the following rephrase help clarify
> >>> this?
> >>>
> >>> Original:
> >>> This overrides the default OMAP34xx CPU OPP table definition.
> >>> Suggested;
> >>> This overrides the default OMAP34xx CPU OPP table definition in
> >>> omap3.dtsi.
> >>
> >> Sorry, I just missed that the omap3430 opps were in omap3.dtsi and not
> >> omap34xx.dtsi. I guess I am not familiar with how the DTC overrides
> >> nodes, however, at least from a readability standpoint it would seem
> >> nice to have the omap3430 opps in a omap3430 specific dtsi and not
> >> omap3.dtsi. However, thats just my opinion.
> > most of omap3630 is based off omap3430. I know from an readability point
> > of view, it might have been good to split that to omap3-common.dtsi,
> > omap34xx.dtsi, omap36xx.dtsi etc.. But there is no real need at this
> > point in time to do that. Unless, ofcourse, we'd like to set that up as
> > an standard for all OMAP SoCs...
> 
> How would omap3-common.dtsi be any different from omap3.dtsi? I don't
> wish us to go nuts with creating dtsi files either, but having an
> omap3430.dtsi does not seem unreasonable to me, but that is just my opinion.
considering omap34xx variants, omap343x.dtsi ;). Will do in v2.

Patch

diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
index 96bf028..743eaa1 100644
--- a/arch/arm/boot/dts/omap36xx.dtsi
+++ b/arch/arm/boot/dts/omap36xx.dtsi
@@ -15,6 +15,18 @@ 
 		serial3 = &uart4;
 	};
 
+	cpus {
+		/* OMAP3630 'standard device' variants OPP50 to OPP130 */
+		cpu@0 {
+			operating-points = <
+				/* kHz    uV */
+				300000   975000
+				600000  1075000
+				800000  1200000
+			>;
+			clock-latency = <300000>; /* From omap-cpufreq driver */
+		};
+	};
 	ocp {
 		uart4: serial@49042000 {
 			compatible = "ti,omap3-uart";
diff --git a/arch/arm/mach-omap2/opp3xxx_data.c b/arch/arm/mach-omap2/opp3xxx_data.c
index 1de18c2..cf7b3ab 100644
--- a/arch/arm/mach-omap2/opp3xxx_data.c
+++ b/arch/arm/mach-omap2/opp3xxx_data.c
@@ -114,15 +114,6 @@  static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
 };
 
 static struct omap_opp_def __initdata omap36xx_opp_def_list[] = {
-	/* MPU OPP1 - OPP50 */
-	OPP_INITIALIZER("mpu", true,  300000000, OMAP3630_VDD_MPU_OPP50_UV),
-	/* MPU OPP2 - OPP100 */
-	OPP_INITIALIZER("mpu", true,  600000000, OMAP3630_VDD_MPU_OPP100_UV),
-	/* MPU OPP3 - OPP-Turbo */
-	OPP_INITIALIZER("mpu", false, 800000000, OMAP3630_VDD_MPU_OPP120_UV),
-	/* MPU OPP4 - OPP-SB */
-	OPP_INITIALIZER("mpu", false, 1000000000, OMAP3630_VDD_MPU_OPP1G_UV),
-
 	/* L3 OPP1 - OPP50 */
 	OPP_INITIALIZER("l3_main", true, 100000000, OMAP3630_VDD_CORE_OPP50_UV),
 	/* L3 OPP2 - OPP100, OPP-Turbo, OPP-SB */