Skip to contentSkip to conversation

arm: boot: dts: am4372: add operating points

PATCH New Felipe Balbi 4 participants 14 messages May 8, 2015 – Jun 16, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 arm: boot: dts: am4372: add operating points

By adding operating points, cpufreq-dt has a
chance of running and doing something useful.

---
 arch/arm/boot/dts/am4372.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/am4372.dtsi +9 -0
index c80a3e233792..ea1db20f64fc 100644
@@ -38,6 +38,15 @@
 			clocks = <&dpll_mpu_ck>;
 			clock-names = "cpu";
 
+			operating-points = <
+				/* kHz		uV */
+				1000000		1325000 /* OPP_NITRO */
+				 800000		1260000 /* OPP_TURBO */
+				 720000		1200000 /* OPP_120 */
+				 600000		1100000 /* OPP_100 */
+				 300000		 950000 /* OPP_50 */
+			>;
+
 			clock-latency = <300000>; /* From omap-cpufreq driver */
 		};
 	};
Signature · 1 line
2.4.0.rc3
#1 lore raw

PATCH 2/2 cpufreq: dt: allow driver to boot automatically

by adding the missing MODULE_ALIAS(), cpufreq-dt
can be autoloaded by udev/systemd.

---
 drivers/cpufreq/cpufreq-dt.c | 1 +
 1 file changed, 1 insertion(+)
drivers/cpufreq/cpufreq-dt.c +1 -0
index bab67db54b7e..528a82bf5038 100644
@@ -416,6 +416,7 @@ static struct platform_driver dt_cpufreq_platdrv = {
 };
 module_platform_driver(dt_cpufreq_platdrv);
 
+MODULE_ALIAS("platform:cpufreq-dt");
 MODULE_AUTHOR("Viresh Kumar <[email protected]>");
 MODULE_AUTHOR("Shawn Guo <[email protected]>");
 MODULE_DESCRIPTION("Generic cpufreq driver");
Signature · 1 line
2.4.0.rc3
#2 lore raw

PATCH 2/2 cpufreq: dt: allow driver to boot automatically

Quoted 14 lines · Felipe Balbi

by adding the missing MODULE_ALIAS(), cpufreq-dt
can be autoloaded by udev/systemd.

---
 drivers/cpufreq/cpufreq-dt.c | 1 +
 1 file changed, 1 insertion(+)
drivers/cpufreq/cpufreq-dt.c +0 -0
index bab67db54b7e..528a82bf5038 100644

drivers/cpufreq/cpufreq-dt.c static struct platform_driver dt_cpufreq_platdrv = { · line 422

Quoted 7 lines
drivers/cpufreq/cpufreq-dt.c +1 -0
@@ -416,6 +416,7 @@ static struct platform_driver dt_cpufreq_platdrv = {
 };
 module_platform_driver(dt_cpufreq_platdrv);
 
+MODULE_ALIAS("platform:cpufreq-dt");
 MODULE_AUTHOR("Viresh Kumar <[email protected]>");
 MODULE_AUTHOR("Shawn Guo <[email protected]>");
 MODULE_DESCRIPTION("Generic cpufreq driver");

This patch does not depend on patch #1 of the series.

other than that..

Signature · 2 lines
Regards,
Nishanth Menon
#4 lore raw

PATCH 2/2 cpufreq: dt: allow driver to boot automatically

In reply to Felipe Balbi
Quoted 4 lines · Felipe Balbi

by adding the missing MODULE_ALIAS(), cpufreq-dt
can be autoloaded by udev/systemd.

looks like this wasn't applied anywhere. Viresh, can you apply this patch please ?

Quoted 19 lines
---
 drivers/cpufreq/cpufreq-dt.c | 1 +
 1 file changed, 1 insertion(+)
drivers/cpufreq/cpufreq-dt.c +1 -0
index bab67db54b7e..528a82bf5038 100644
@@ -416,6 +416,7 @@ static struct platform_driver dt_cpufreq_platdrv = {
 };
 module_platform_driver(dt_cpufreq_platdrv);
 
+MODULE_ALIAS("platform:cpufreq-dt");
 MODULE_AUTHOR("Viresh Kumar <[email protected]>");
 MODULE_AUTHOR("Shawn Guo <[email protected]>");
 MODULE_DESCRIPTION("Generic cpufreq driver");
Signature · 1 line
2.4.0.rc3
Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150616/83a6496d/attachment.sig>
#5 lore raw

PATCH 2/2 cpufreq: dt: allow driver to boot automatically

Quoted 8 lines · Felipe Balbi
Quoted 4 lines · Felipe Balbi

by adding the missing MODULE_ALIAS(), cpufreq-dt
can be autoloaded by udev/systemd.

looks like this wasn't applied anywhere. Viresh, can you apply this patch please ?

I've overlooked it, sorry about that. Applied now, thanks!

Quoted 22 lines
Quoted 17 lines
---
 drivers/cpufreq/cpufreq-dt.c | 1 +
 1 file changed, 1 insertion(+)
drivers/cpufreq/cpufreq-dt.c +1 -1
index bab67db54b7e..528a82bf5038 100644
@@ -416,6 +416,7 @@ static struct platform_driver dt_cpufreq_platdrv = {
 };
 module_platform_driver(dt_cpufreq_platdrv);
 
+MODULE_ALIAS("platform:cpufreq-dt");
 MODULE_AUTHOR("Viresh Kumar <[email protected]>");
 MODULE_AUTHOR("Shawn Guo <[email protected]>");
 MODULE_DESCRIPTION("Generic cpufreq driver");
--
Quoted 2 lines

2.4.0.rc3

--
balbi

#6 lore raw

PATCH 2/2 cpufreq: dt: allow driver to boot automatically

Quoted 11 lines · Rafael J. Wysocki
Quoted 8 lines · Felipe Balbi
Quoted 4 lines · Felipe Balbi

by adding the missing MODULE_ALIAS(), cpufreq-dt
can be autoloaded by udev/systemd.

looks like this wasn't applied anywhere. Viresh, can you apply this patch please ?

I've overlooked it, sorry about that. Applied now, thanks!

Thanks Rafael.

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150616/de9514a8/attachment-0001.sig>
#7 lore raw

PATCH 1/2 arm: boot: dts: am4372: add operating points

In reply to Felipe Balbi
Quoted 19 lines · Felipe Balbi

By adding operating points, cpufreq-dt has a
chance of running and doing something useful.

---
 arch/arm/boot/dts/am4372.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/am4372.dtsi +2 -0
index c80a3e233792..ea1db20f64fc 100644
@@ -38,6 +38,15 @@
 			clocks = <&dpll_mpu_ck>;
 			clock-names = "cpu";
 
+			operating-points = <
+				/* kHz		uV */

arch/arm/boot/dts/am4372.dtsi line 52

Quoted 10 lines
arch/arm/boot/dts/am4372.dtsi +7 -0
+				1000000		1325000 /* OPP_NITRO */
+				 800000		1260000 /* OPP_TURBO */
+				 720000		1200000 /* OPP_120 */
+				 600000		1100000 /* OPP_100 */
+				 300000		 950000 /* OPP_50 */
+			>;
+
 			clock-latency = <300000>; /* From omap-cpufreq driver */
 		};
 	};

which of these OPPs need AVS? which of these are dependent on Efuse bit dependent?

Signature · 2 lines
Regards,
Nishanth Menon
#8 lore raw

PATCH 1/2 arm: boot: dts: am4372: add operating points

Quoted 17 lines · Nishanth Menon
Quoted 16 lines · Felipe Balbi

By adding operating points, cpufreq-dt has a
chance of running and doing something useful.

---
 arch/arm/boot/dts/am4372.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/am4372.dtsi +0 -0
index c80a3e233792..ea1db20f64fc 100644
@@ -38,6 +38,15 @@
 			clocks = <&dpll_mpu_ck>;
 			clock-names = "cpu";
 
Quoted 16 lines

arch/arm/boot/dts/am4372.dtsi line 52

Quoted 13 lines
arch/arm/boot/dts/am4372.dtsi +9 -0
+			operating-points = <
+				/* kHz		uV */
+				1000000		1325000 /* OPP_NITRO */
+				 800000		1260000 /* OPP_TURBO */
+				 720000		1200000 /* OPP_120 */
+				 600000		1100000 /* OPP_100 */
+				 300000		 950000 /* OPP_50 */
+			>;
+
 			clock-latency = <300000>; /* From omap-cpufreq driver */
 		};
 	};

which of these OPPs need AVS? which of these are dependent on Efuse bit dependent?

You can use
http://git.ti.com/ti-linux-kernel/ti-linux-kernel/blobs/ti-linux-3.14.y/arch/arm/mach-omap2/opp43xx_data.c
for reference.

Signature · 2 lines
Regards,
Nishanth Menon
#9 lore raw

PATCH 1/2 arm: boot: dts: am4372: add operating points

Quoted 36 lines · Nishanth Menon
Quoted 17 lines · Nishanth Menon
Quoted 16 lines · Felipe Balbi

By adding operating points, cpufreq-dt has a
chance of running and doing something useful.

---
 arch/arm/boot/dts/am4372.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/am4372.dtsi +0 -0
index c80a3e233792..ea1db20f64fc 100644
@@ -38,6 +38,15 @@
 			clocks = <&dpll_mpu_ck>;
 			clock-names = "cpu";
 
Quoted 16 lines

arch/arm/boot/dts/am4372.dtsi line 52

Quoted 13 lines
arch/arm/boot/dts/am4372.dtsi +9 -0
+			operating-points = <
+				/* kHz		uV */
+				1000000		1325000 /* OPP_NITRO */
+				 800000		1260000 /* OPP_TURBO */
+				 720000		1200000 /* OPP_120 */
+				 600000		1100000 /* OPP_100 */
+				 300000		 950000 /* OPP_50 */
+			>;
+
 			clock-latency = <300000>; /* From omap-cpufreq driver */
 		};
 	};

which of these OPPs need AVS? which of these are dependent on Efuse bit dependent?

Quoted 3 lines

You can use
http://git.ti.com/ti-linux-kernel/ti-linux-kernel/blobs/ti-linux-3.14.y/arch/arm/mach-omap2/opp43xx_data.c
for reference.

heh, why isn't that upstream yet ? Seems to be ready already. The point is that as of now, u-boot will set maximum OPP it can find and, for AM437x, that will be 800MHz or 1GHz depending on your board. 1GHz might not be supported in all SoCs and letting that be used all the time is likely going to reduce silicon lifetime.

At least allowing ondemand governor run, we will be mostly running at 300MHz and only jump to "invalid" OPPs under load which, granted, is still not perfect, but better than running at 1GHz all the time, don't you agree ?

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150508/84baa977/attachment.sig>
#10 lore raw

PATCH 1/2 arm: boot: dts: am4372: add operating points

Quoted 37 lines · Felipe Balbi
Quoted 36 lines · Nishanth Menon
Quoted 33 lines · Nishanth Menon
Quoted 19 lines · Felipe Balbi

By adding operating points, cpufreq-dt has a
chance of running and doing something useful.

---
 arch/arm/boot/dts/am4372.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/am4372.dtsi +2 -0
index c80a3e233792..ea1db20f64fc 100644
@@ -38,6 +38,15 @@
 			clocks = <&dpll_mpu_ck>;
 			clock-names = "cpu";
 
+			operating-points = <
+				/* kHz		uV */

arch/arm/boot/dts/am4372.dtsi line 52

Quoted 10 lines
arch/arm/boot/dts/am4372.dtsi +7 -0
+				1000000		1325000 /* OPP_NITRO */
+				 800000		1260000 /* OPP_TURBO */
+				 720000		1200000 /* OPP_120 */
+				 600000		1100000 /* OPP_100 */
+				 300000		 950000 /* OPP_50 */
+			>;
+
 			clock-latency = <300000>; /* From omap-cpufreq driver */
 		};
 	};

which of these OPPs need AVS? which of these are dependent on Efuse bit dependent?

Quoted 5 lines
Quoted 3 lines

You can use
http://git.ti.com/ti-linux-kernel/ti-linux-kernel/blobs/ti-linux-3.14.y/arch/arm/mach-omap2/opp43xx_data.c
for reference.

heh, why isn't that upstream yet ? Seems to be ready already. The point

we have tried in the past[1] - unfortunately that was just bandaid since the existing OPP device tree bindings have very limiting behavior across multiple SoCs.This was one of the reasons why we stopped adding more OPPs, since we are hoping to see the new bindings[2] which is under discussion settle down and help enable support for cases like that we have on TI SoCs, iMX SoCs etc.

Quoted 9 lines

is that as of now, u-boot will set maximum OPP it can find and, for AM437x, that will be 800MHz or 1GHz depending on your board. 1GHz might not be supported in all SoCs and letting that be used all the time is likely going to reduce silicon lifetime.

At least allowing ondemand governor run, we will be mostly running at 300MHz and only jump to "invalid" OPPs under load which, granted, is still not perfect, but better than running at 1GHz all the time, don't you agree ?

The fix for this should ideally be in u-boot - we are trying not to introduce dts changes in the hopes that the new proposed bindings that Viresh has on discussion comes to conclusion and help introduce new dtb support with proper SoC description. allowing an SoC to enter an invalid OPP is broken by design. we must attempt to curb it. unfortunately, we already do have a broken implementation for AM335x, DRA7 in place which went with the assumption that we will be able to do modifiers on top of existing dt description and the hopes that [1] will eventually get upstream. But, as is clear now, [1] has no future path in upstream kernel. following the same broken path for newer SoC definitions will probably very short sighted by us.

in my opinion, doing a temporary hack in upstream kernel is not an elegant approach. I suggest helping review and approving Viresh's new bindings so that we have a longer term solution is more the way to do this.

Just my 2 cents. Thanks once again for identifying the urgent need for helping Viresh's series along - will be great if folks could help review and approve his series to get them upstream and help all of us ARM SoC variations along.

[1] http://comments.gmane.org/gmane.linux.ports.arm.kernel/309466
[2] http://marc.info/?l=devicetree&m=143039569104090&w=2

Signature · 2 lines
Regards,
Nishanth Menon
#11 lore raw

PATCH 1/2 arm: boot: dts: am4372: add operating points

Hi,

Quoted 42 lines · Nishanth Menon
Quoted 41 lines · Felipe Balbi
Quoted 39 lines · Nishanth Menon
Quoted 17 lines · Nishanth Menon
Quoted 16 lines · Felipe Balbi

By adding operating points, cpufreq-dt has a
chance of running and doing something useful.

---
 arch/arm/boot/dts/am4372.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/am4372.dtsi +0 -0
index c80a3e233792..ea1db20f64fc 100644
@@ -38,6 +38,15 @@
 			clocks = <&dpll_mpu_ck>;
 			clock-names = "cpu";
 
Quoted 16 lines

arch/arm/boot/dts/am4372.dtsi line 52

Quoted 13 lines
arch/arm/boot/dts/am4372.dtsi +9 -0
+			operating-points = <
+				/* kHz		uV */
+				1000000		1325000 /* OPP_NITRO */
+				 800000		1260000 /* OPP_TURBO */
+				 720000		1200000 /* OPP_120 */
+				 600000		1100000 /* OPP_100 */
+				 300000		 950000 /* OPP_50 */
+			>;
+
 			clock-latency = <300000>; /* From omap-cpufreq driver */
 		};
 	};

which of these OPPs need AVS? which of these are dependent on Efuse bit dependent?

You can use
http://git.ti.com/ti-linux-kernel/ti-linux-kernel/blobs/ti-linux-3.14.y/arch/arm/mach-omap2/opp43xx_data.c
for reference.

Quoted 8 lines
Quoted 1 line

heh, why isn't that upstream yet ? Seems to be ready already. The point

we have tried in the past[1] - unfortunately that was just bandaid since the existing OPP device tree bindings have very limiting behavior across multiple SoCs.This was one of the reasons why we stopped adding more OPPs, since we are hoping to see the new bindings[2] which is under discussion settle down and help enable support for cases like that we have on TI SoCs, iMX SoCs etc.

fair enough.

Quoted 10 lines
Quoted 9 lines

is that as of now, u-boot will set maximum OPP it can find and, for AM437x, that will be 800MHz or 1GHz depending on your board. 1GHz might not be supported in all SoCs and letting that be used all the time is likely going to reduce silicon lifetime.

At least allowing ondemand governor run, we will be mostly running at 300MHz and only jump to "invalid" OPPs under load which, granted, is still not perfect, but better than running at 1GHz all the time, don't you agree ?

The fix for this should ideally be in u-boot - we are trying not to

right, ideally, yeah; but then we go back to the discussion regarding kernel vs bootloader dependencies :-)

Quoted 11 lines

introduce dts changes in the hopes that the new proposed bindings that Viresh has on discussion comes to conclusion and help introduce new dtb support with proper SoC description. allowing an SoC to enter an invalid OPP is broken by design. we must attempt to curb it. unfortunately, we already do have a broken implementation for AM335x, DRA7 in place which went with the assumption that we will be able to do modifiers on top of existing dt description and the hopes that [1] will eventually get upstream. But, as is clear now, [1] has no future path in upstream kernel. following the same broken path for newer SoC definitions will probably very short sighted by us.

Quoted 2 lines

in my opinion, doing a temporary hack in upstream kernel is not an elegant approach. I suggest helping review and approving Viresh's new

however this is not a hack, right ? If we get rid of OPP_NITRO and OPP_TURBO, then we will more than likely always be dealing with safe OPPs (yeah, I need to confirm this since it's not on public TRM, so as of now, take this statement with a grain of salt :-), moreover, even though we're trying to change opp bindings, the current situation is still very much accepted and will remain valid even after changing binding :-)

Not to mention that people using AM43xx today might be using it under invalid OPPs and decreasing silicon life; I'd assume that's a very urgent detail to sort out.

Quoted 6 lines

bindings so that we have a longer term solution is more the way to do this.

Just my 2 cents. Thanks once again for identifying the urgent need for helping Viresh's series along - will be great if folks could help review and approve his series to get them upstream and help all of us ARM SoC variations along.

np.

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150511/c2b9f3dd/attachment.sig>
#12 lore raw

PATCH 1/2 arm: boot: dts: am4372: add operating points

Quoted 11 lines · Felipe Balbi
Quoted 3 lines

in my opinion, doing a temporary hack in upstream kernel is not an elegant approach. I suggest helping review and approving Viresh's new

however this is not a hack, right ? If we get rid of OPP_NITRO and OPP_TURBO, then we will more than likely always be dealing with safe OPPs (yeah, I need to confirm this since it's not on public TRM, so as of now, take this statement with a grain of salt :-), moreover, even though we're trying to change opp bindings, the current situation is still very much accepted and will remain valid even after changing binding :-)

yes - if we do have a documented subset of OPPs that are valid for all "variants" of AM437x, we could add that in using the legacy bindings, but, we will have to do a transition over to the new bindings when they are finalized to support all OPPs appropriately.

Quoted 3 lines

Not to mention that people using AM43xx today might be using it under invalid OPPs and decreasing silicon life; I'd assume that's a very urgent detail to sort out.

While I do agree that there is always a debate between fixing things in kernel for bootloader issues, but it does not mean that we should just postpone fixing the bootloader in this case - since, at this very moment, we are already in broken configuration - example sitting on a bootloader shell does have the same impact we have at this time.

Lets try to help Viresh in getting his series sorted out meanwhile - maybe others can help as well :(

---
Regards,
Nishanth Menon

#13 lore raw

PATCH 1/2 arm: boot: dts: am4372: add operating points

HI,

Quoted 13 lines · Nishanth Menon
Quoted 11 lines · Felipe Balbi
Quoted 3 lines

in my opinion, doing a temporary hack in upstream kernel is not an elegant approach. I suggest helping review and approving Viresh's new

however this is not a hack, right ? If we get rid of OPP_NITRO and OPP_TURBO, then we will more than likely always be dealing with safe OPPs (yeah, I need to confirm this since it's not on public TRM, so as of now, take this statement with a grain of salt :-), moreover, even though we're trying to change opp bindings, the current situation is still very much accepted and will remain valid even after changing binding :-)

Quoted 4 lines

yes - if we do have a documented subset of OPPs that are valid for all "variants" of AM437x, we could add that in using the legacy bindings, but, we will have to do a transition over to the new bindings when they are finalized to support all OPPs appropriately.

sounds fair to me.

Quoted 9 lines
Quoted 3 lines

Not to mention that people using AM43xx today might be using it under invalid OPPs and decreasing silicon life; I'd assume that's a very urgent detail to sort out.

While I do agree that there is always a debate between fixing things in kernel for bootloader issues, but it does not mean that we should just postpone fixing the bootloader in this case - since, at this very moment, we are already in broken configuration - example sitting on a bootloader shell does have the same impact we have at this time.

agreed, I'll cook up a patch for bootloader too.

Quoted 2 lines

Lets try to help Viresh in getting his series sorted out meanwhile - maybe others can help as well :(

sure.

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150511/b970e4dc/attachment.sig>