Armada 388 GP minor fixes

PATCH New Thomas Petazzoni 2 participants 7 messages Dec 21, 2015 – Jan 11, 2016 Device tree bindings

lore

Share & tools

lore

Thomas Petazzoni

author

2015-12-21 14:41 UTC #0 lore raw

cover Armada 388 GP minor fixes

Hello,

Here are two minor fixes/improvements to the support of the Armada 388 GP platform.

The first patch was already sent in June and July this year, but was left on the side because it was part of a larger patch series about suspend/resume on Armada 38x, which has not been fully merged yet. However, this patch is completely independent from suspend/resume, so it can be merged.

Thanks,

Thomas

Thomas Petazzoni (2):
ARM: mvebu: adjust board name and compatible for Armada 388 GP
ARM: mvebu: use regulator-boot-on on Armada 388 GP

 arch/arm/boot/dts/armada-388-gp.dts | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

Signature · 1 line

2.6.4

Thomas Petazzoni

author

2015-12-21 14:41 UTC #1 lore raw

PATCH 1/2 ARM: mvebu: adjust board name and compatible for Armada 388 GP

As the name of the Device Tree file name suggests, the Armada 388 GP really contains an Armada 388 SoC, so this commit updates the board name and compatible string in the Device Tree file.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 arch/arm/boot/dts/armada-388-gp.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/boot/dts/armada-388-gp.dts +2 -2

index a633be3..d8dab0f 100644
@@ -44,8 +44,8 @@
 #include <dt-bindings/gpio/gpio.h>
 
 / {
-	model = "Marvell Armada 385 GP";
-	compatible = "marvell,a385-gp", "marvell,armada388", "marvell,armada380";
+	model = "Marvell Armada 388 DB-88F6820-GP";
+	compatible = "marvell,a388-gp", "marvell,armada388", "marvell,armada380";
 
 	chosen {
 		stdout-path = "serial0:115200n8";

Signature · 1 line

2.6.4

Gregory CLEMENT

2015-12-21 16:52 UTC #2 lore raw

PATCH 1/2 ARM: mvebu: adjust board name and compatible for Armada 388 GP

Hi Thomas,

Quoted 5 lines · Thomas Petazzoni

As the name of the Device Tree file name suggests, the Armada 388 GP really contains an Armada 388 SoC, so this commit updates the board name and compatible string in the Device Tree file.
Signed-off-by Thomas Petazzoni <[email protected]>

Applied on mvebu/dt

Thanks,

Gregory

Quoted 22 lines

---
 arch/arm/boot/dts/armada-388-gp.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/boot/dts/armada-388-gp.dts +2 -2

index a633be3..d8dab0f 100644
@@ -44,8 +44,8 @@
 #include <dt-bindings/gpio/gpio.h>
 
 / {
-	model = "Marvell Armada 385 GP";
-	compatible = "marvell,a385-gp", "marvell,armada388", "marvell,armada380";
+	model = "Marvell Armada 388 DB-88F6820-GP";
+	compatible = "marvell,a388-gp", "marvell,armada388", "marvell,armada380";
 
 	chosen {
 		stdout-path = "serial0:115200n8";

Signature · 1 line

2.6.4

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Gregory CLEMENT

2016-01-11 16:24 UTC #3 lore raw

PATCH 1/2 ARM: mvebu: adjust board name and compatible for Armada 388 GP

Hi,

Quoted 11 lines · Gregory CLEMENT

Hi Thomas,
Quoted 5 lines · Thomas Petazzoni
As the name of the Device Tree file name suggests, the Armada 388 GP really contains an Armada 388 SoC, so this commit updates the board name and compatible string in the Device Tree file.
Signed-off-by Thomas Petazzoni <[email protected]>
Applied on mvebu/dt

I didn't managed to do a pull request before the holidays period so I moved this commit to mvebu/dt-for-4.6

Gregory

Quoted 37 lines

Thanks,

Gregory

Quoted 22 lines

---
 arch/arm/boot/dts/armada-388-gp.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/boot/dts/armada-388-gp.dts +2 -2

index a633be3..d8dab0f 100644
@@ -44,8 +44,8 @@
 #include <dt-bindings/gpio/gpio.h>
 
 / {
-	model = "Marvell Armada 385 GP";
-	compatible = "marvell,a385-gp", "marvell,armada388", "marvell,armada380";
+	model = "Marvell Armada 388 DB-88F6820-GP";
+	compatible = "marvell,a388-gp", "marvell,armada388", "marvell,armada380";
 
 	chosen {
 		stdout-path = "serial0:115200n8";

Signature · 1 line

2.6.4

Signature · 9 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Thomas Petazzoni

author

2015-12-21 14:41 UTC #4 lore raw

PATCH 2/2 ARM: mvebu: use regulator-boot-on on Armada 388 GP

In reply to Thomas Petazzoni

Really, what we meant by regulator-always-on is that the regulators are already turned on by the bootloader, for which regulator-boot-on is a better description.

A net advantage of using regulator-boot-on is that the regulator is not touched at boot time by the kernel, which avoids having the hard drives spinning down and then up again, taking several (~5) seconds of additional boot time.

In addition, there is no need to have such properties on the child regulators used for SATA. Having it on the parent regulator that really controls the GPIO is sufficient.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 arch/arm/boot/dts/armada-388-gp.dts | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

arch/arm/boot/dts/armada-388-gp.dts +8 ~~-16~~

index d8dab0f..1ef6cc6 100644
@@ -279,7 +279,7 @@
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander1 15 GPIO_ACTIVE_HIGH>;
 	};

@@ -289,7 +289,7 @@
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander1 14 GPIO_ACTIVE_HIGH>;
 	};

@@ -299,7 +299,7 @@
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander0 4 GPIO_ACTIVE_HIGH>;
 	};

@@ -309,7 +309,7 @@
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander0 4 GPIO_ACTIVE_HIGH>;
 	};

@@ -319,7 +319,7 @@
 		regulator-min-microvolt = <12000000>;
 		regulator-max-microvolt = <12000000>;
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander0 2 GPIO_ACTIVE_HIGH>;
 	};

@@ -328,7 +328,6 @@
 		regulator-name = "v5.0-sata0";
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
-		regulator-always-on;
 		vin-supply = <&reg_sata0>;
 	};

@@ -337,7 +336,6 @@
 		regulator-name = "v12.0-sata0";
 		regulator-min-microvolt = <12000000>;
 		regulator-max-microvolt = <12000000>;
-		regulator-always-on;
 		vin-supply = <&reg_sata0>;
 	};

@@ -347,7 +345,7 @@
 		regulator-min-microvolt = <12000000>;
 		regulator-max-microvolt = <12000000>;
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander0 3 GPIO_ACTIVE_HIGH>;
 	};

@@ -356,7 +354,6 @@
 		regulator-name = "v5.0-sata1";
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
-		regulator-always-on;
 		vin-supply = <&reg_sata1>;
 	};

@@ -365,7 +362,6 @@
 		regulator-name = "v12.0-sata1";
 		regulator-min-microvolt = <12000000>;
 		regulator-max-microvolt = <12000000>;
-		regulator-always-on;
 		vin-supply = <&reg_sata1>;
 	};

@@ -373,7 +369,7 @@
 		compatible = "regulator-fixed";
 		regulator-name = "pwr_en_sata2";
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander0 11 GPIO_ACTIVE_HIGH>;
 	};

@@ -382,7 +378,6 @@
 		regulator-name = "v5.0-sata2";
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
-		regulator-always-on;
 		vin-supply = <&reg_sata2>;
 	};

@@ -391,7 +386,6 @@
 		regulator-name = "v12.0-sata2";
 		regulator-min-microvolt = <12000000>;
 		regulator-max-microvolt = <12000000>;
-		regulator-always-on;
 		vin-supply = <&reg_sata2>;
 	};

@@ -399,7 +393,7 @@
 		compatible = "regulator-fixed";
 		regulator-name = "pwr_en_sata3";
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander0 12 GPIO_ACTIVE_HIGH>;
 	};

@@ -408,7 +402,6 @@
 		regulator-name = "v5.0-sata3";
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
-		regulator-always-on;
 		vin-supply = <&reg_sata3>;
 	};

@@ -417,7 +410,6 @@
 		regulator-name = "v12.0-sata3";
 		regulator-min-microvolt = <12000000>;
 		regulator-max-microvolt = <12000000>;
-		regulator-always-on;
 		vin-supply = <&reg_sata3>;
 	};
 };

Signature · 1 line

2.6.4

Gregory CLEMENT

2015-12-22 12:23 UTC #5 lore raw

PATCH 2/2 ARM: mvebu: use regulator-boot-on on Armada 388 GP

Hi Thomas,

Quoted 4 lines · Thomas Petazzoni

Really, what we meant by regulator-always-on is that the regulators are already turned on by the bootloader, for which regulator-boot-on is a better description.

What happened if the bootloader do not turn the regulator on? I fear that in this case the regulator won't be turned on at all.

Quoted 19 lines

A net advantage of using regulator-boot-on is that the regulator is not touched at boot time by the kernel, which avoids having the hard drives spinning down and then up again, taking several (~5) seconds of additional boot time.
In addition, there is no need to have such properties on the child regulators used for SATA. Having it on the parent regulator that really controls the GPIO is sufficient.
Signed-off-by Thomas Petazzoni <[email protected]>
---
 arch/arm/boot/dts/armada-388-gp.dts | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)
arch/arm/boot/dts/armada-388-gp.dts +0 -0
index d8dab0f..1ef6cc6 100644

arch/arm/boot/dts/armada-388-gp.dts line 314

Quoted 7 lines

arch/arm/boot/dts/armada-388-gp.dts +1 -1

@@ -309,7 +309,7 @@
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander0 4 GPIO_ACTIVE_HIGH>;
 	};

This node had been removed by a patch you sent few days ago:
"ARM: mvebu: remove duplicated regulator definition in Armada 388 GP"

Thanks,

Gregory

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Gregory CLEMENT

2015-12-22 14:04 UTC #6 lore raw

PATCH 2/2 ARM: mvebu: use regulator-boot-on on Armada 388 GP

Hi Thomas,

Quoted 4 lines · Gregory CLEMENT

Hi Thomas,
On lun., d?c. 21 2015, Thomas Petazzoni <[email protected]> wrote:

Quoted 12 lines

Quoted 4 lines
Really, what we meant by regulator-always-on is that the regulators are already turned on by the bootloader, for which regulator-boot-on is a better description.
What happened if the bootloader do not turn the regulator on? I fear that in this case the regulator won't be turned on at all.
Quoted 4 lines
A net advantage of using regulator-boot-on is that the regulator is not touched at boot time by the kernel, which avoids having the hard drives spinning down and then up again, taking several (~5) seconds of additional boot time.

I tested your patch, I didn't see so much time lost in boot time: with one hdd the difference was only about 0.3s. But with a second hdd it was around 1.9 (so 1.6s more for the second hhd). It looks like it really depends on the hdd. However, the most annoying point with the regulator-always-on for the SATA it turns the power down and up for the hdd during boot which is not very nice for the hdd. So, at least for this, it is useful.

For the USB part the regulators are completely disabled:
usb3-vbus: disabling
v5.0-vbus0: disabling
v5.0-vbus1: disabling

And indeed the USB does not work with your patch applied.

Could you send a new version with the changes only done for the SATA regulator nodes?

Thanks,

Gregory

Quoted 39 lines

Quoted 15 lines
In addition, there is no need to have such properties on the child regulators used for SATA. Having it on the parent regulator that really controls the GPIO is sufficient.
Signed-off-by Thomas Petazzoni <[email protected]>
---
 arch/arm/boot/dts/armada-388-gp.dts | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)
arch/arm/boot/dts/armada-388-gp.dts +0 -0
index d8dab0f..1ef6cc6 100644
arch/arm/boot/dts/armada-388-gp.dts line 314
Quoted 7 lines
arch/arm/boot/dts/armada-388-gp.dts +1 -1
@@ -309,7 +309,7 @@
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
 		enable-active-high;
-		regulator-always-on;
+		regulator-boot-on;
 		gpio = <&expander0 4 GPIO_ACTIVE_HIGH>;
 	};
This node had been removed by a patch you sent few days ago:
"ARM: mvebu: remove duplicated regulator definition in Armada 388 GP"
Thanks,
Gregory
Signature · 9 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com