Skip to contentSkip to conversation

ARM: dts: Fix fixed regulators enable GPIO polarity

PATCH Applied Laurent Pinchart 9 participants 56 messages Oct 12, 2015 – Oct 13, 2015

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

cover ARM: dts: Fix fixed regulators enable GPIO polarity

Hello,

While working on regulators, GPIOs and DT I noticed that many of our DT source files incorrectly describe fixed regulators. The common error patterns are

- Usage of the undefined (and never parsed) enable-active-low property
- Usage of the enable-active-high property without specifying an enable GPIO
- Typos in the enabl GPIO property name (gpios instead of gpio)
- Mismatch between the enable-active-high property (or the lack thereof) and the enable GPIO flags

This patch series fixes those issues in all the DT sources after locating the errors using the following script.

------------------------------------------------------------------------------
#!/bin/sh

echo $1
cat $1 | awk '

BEGIN {
	open_drain = 0;
	active_high = 0;
	gpio = 0;
	flags = 0;
}
match($0, /([a-zA-Z0-9 at _-]*) {/, ary) {
	name = ary[1];
}
/compatible.*"regulator-fixed"/ {
	found = 1;
}
/enable-active-high/ {
	active_high = 1;
}
/gpio-open-drain/ {
	open_drain = 1;
}
match($0, /gpio += <.* ([^ ]*)>/, ary) {
	gpio = 1;
	flags = ary[1];
	if (flags == 0)
		flags = "GPIO_ACTIVE_HIGH";
}
/}/ {
	if (found) {
		if (gpio) {
			print "\t" name ": active high " active_high " " flags " open drain " open_drain;
			if ((active_high && flags == "GPIO_ACTIVE_LOW") ||
			    (!active_high && flags == "GPIO_ACTIVE_HIGH"))
				print "WARNING: enable-active-high and flags do not match"
		} else {
			if (active_high)
				print "WARNING: active high without GPIO"
			if (open_drain)
				print "WARNING: open drain without GPIO"
		}
	}

	gpio = 0;
	found = 0;
	active_high = 0;
	open_drain = 0;
	flags = 0;
}

'
------------------------------------------------------------------------------

All patches except for the ones touching omap3-beagle-xm and omap3-overo-base are untested as I lack test hardware.

As there's no dependency between the patches touching different source files the appropriate maintainers could take their share of the patches in their tree. Alternatively I could send a single pull request after collecting all acks but that might be more complex.

Cc devicetree at vger.kernel.org
Cc linux-omap at vger.kernel.org
Cc linux-samsung-soc at vger.kernel.org
Cc linux-tegra at vger.kernel.org
Cc linux-gpio at vger.kernel.org
Cc linux-kernel at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc Jason Cooper <[email protected]>
Cc Andrew Lunn <[email protected]>
Cc Gregory Clement <[email protected]>
Cc Sebastian Hesselbarth <[email protected]>
Cc Kukjin Kim <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Alexandre Courbot <[email protected]>
Cc Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Linus Walleij <[email protected]>

Laurent Pinchart (37):
ARM: dts: am437x-gp-evm: Remove unneeded regulator property
ARM: dts: am43xx-epos-evm: Remove unneeded regulator property
ARM: mvebu: Armada 388 GP: Remove unneeded regulator property
ARM: imx6sx-sdb: Fix typo in regulator enable GPIO property
ARM: dts: s5pv210-aquila: Fix typo in regulator enable GPIO property
ARM: dts: s5pv210-goni: Fix typo in regulator enable GPIO property
ARM: dts: omap3-evm: Remove invalid enable-active-low regulator
property
ARM: dts: omap3-sb-t35: Remove invalid enable-active-low regulator
property
ARM: dts: omap3-tao3530: Remove invalid enable-active-low regulator
property
ARM: dts: imx6qdl-tx6: Fix regulator enable GPIO polarity
ARM: dts: dove-cm-a510: Fix regulator enable GPIO polarity
ARM: dts: dove-sbc-a510: Fix regulator enable GPIO polarity
ARM: dts: exynos5250-arndale: Fix regulator enable GPIO polarity
ARM: dts: imx23-evk: Fix regulator enable GPIO polarity
ARM: dts: imx23-stmp378x_devb: Fix regulator enable GPIO polarity
ARM: dts: imx25-pdk: Fix regulator enable GPIO polarity
ARM: dts: imx28-cfa10036: Fix regulator enable GPIO polarity
ARM: dts: imx28-evk: Fix regulator enable GPIO polarity
ARM: dts: imx28-m28cu3: Fix regulator enable GPIO polarity
ARM: dts: imx28-m28evk: Fix regulator enable GPIO polarity
ARM: dts: imx28-sps1: Fix regulator enable GPIO polarity
ARM: dts: imx28-tx28: Fix regulator enable GPIO polarity
ARM: dts: imx53-m53evk: Fix regulator enable GPIO polarity
ARM: dts: imx53-mba53: Fix regulator enable GPIO polarity
ARM: dts: imx53-tx53: Fix regulator enable GPIO polarity
ARM: dts: imx6q-dmo-edmqmx6: Fix regulator enable GPIO polarity
ARM: dts: kirkwood-blackarmor-nas220: Fix regulator enable GPIO
polarity
ARM: dts: omap4-duovero: Fix regulator enable GPIO polarity
ARM: dts: kirkwood-nsa3x0-common: Fix regulator enable GPIO polarity
ARM: dts: omap3-beagle-xm: Fix regulator enable GPIO polarity
ARM: dts: omap3-beagle: Fix regulator enable GPIO polarity
ARM: dts: omap3-overo-base: Fix regulator enable GPIO polarity
ARM: dts: omap3-tao3530: Fix regulator enable GPIO polarity
ARM: dts: tegra20-harmony: Fix regulator enable GPIO polarity
ARM: dts: tegra20-iris-512: Fix regulator enable GPIO polarity
ARM: dts: tegra20-seaboard: Fix regulator enable GPIO polarity
ARM: dts: tegra20-ventana: Fix regulator enable GPIO polarity

 arch/arm/boot/dts/am437x-gp-evm.dts              | 1 -
 arch/arm/boot/dts/am43x-epos-evm.dts             | 1 -
 arch/arm/boot/dts/armada-388-gp.dts              | 2 --
 arch/arm/boot/dts/dove-cm-a510.dtsi              | 2 +-
 arch/arm/boot/dts/dove-sbc-a510.dts              | 4 ++--
 arch/arm/boot/dts/exynos5250-arndale.dts         | 2 +-
 arch/arm/boot/dts/imx23-evk.dts                  | 3 ++-
 arch/arm/boot/dts/imx23-stmp378x_devb.dts        | 3 ++-
 arch/arm/boot/dts/imx25-pdk.dts                  | 2 +-
 arch/arm/boot/dts/imx28-cfa10036.dts             | 2 +-
 arch/arm/boot/dts/imx28-evk.dts                  | 4 ++--
 arch/arm/boot/dts/imx28-m28cu3.dts               | 4 ++--
 arch/arm/boot/dts/imx28-m28evk.dts               | 6 +++---
 arch/arm/boot/dts/imx28-sps1.dts                 | 2 +-
 arch/arm/boot/dts/imx28-tx28.dts                 | 2 +-
 arch/arm/boot/dts/imx53-m53evk.dts               | 2 +-
 arch/arm/boot/dts/imx53-mba53.dts                | 2 +-
 arch/arm/boot/dts/imx53-tx53.dtsi                | 2 +-
 arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts          | 2 +-
 arch/arm/boot/dts/imx6qdl-tx6.dtsi               | 3 +--
 arch/arm/boot/dts/imx6sx-sdb.dtsi                | 2 +-
 arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts | 4 ++--
 arch/arm/boot/dts/kirkwood-nsa3x0-common.dtsi    | 2 +-
 arch/arm/boot/dts/omap3-beagle-xm.dts            | 2 +-
 arch/arm/boot/dts/omap3-beagle.dts               | 2 +-
 arch/arm/boot/dts/omap3-evm-common.dtsi          | 1 -
 arch/arm/boot/dts/omap3-overo-base.dtsi          | 4 ++--
 arch/arm/boot/dts/omap3-sb-t35.dtsi              | 1 -
 arch/arm/boot/dts/omap3-tao3530.dtsi             | 3 +--
 arch/arm/boot/dts/omap4-duovero.dtsi             | 2 +-
 arch/arm/boot/dts/s5pv210-aquila.dts             | 2 +-
 arch/arm/boot/dts/s5pv210-goni.dts               | 4 ++--
 arch/arm/boot/dts/tegra20-harmony.dts            | 2 +-
 arch/arm/boot/dts/tegra20-iris-512.dts           | 2 +-
 arch/arm/boot/dts/tegra20-seaboard.dts           | 2 +-
 arch/arm/boot/dts/tegra20-ventana.dts            | 2 +-
 36 files changed, 41 insertions(+), 47 deletions(-)
Signature · 3 lines
Regards,

Laurent Pinchart
#1 lore raw

PATCH 1/37 ARM: dts: am437x-gp-evm: Remove unneeded regulator property

The enable-active-high regulator property only makes sense when an enable GPIO is specified. Remove it.

---
 arch/arm/boot/dts/am437x-gp-evm.dts | 1 -
 1 file changed, 1 deletion(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/am437x-gp-evm.dts +0 -1
index 84aa30c3235a..a3cd413aebf0 100644
@@ -29,7 +29,6 @@
 		regulator-name = "vmmcsd_fixed";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		enable-active-high;
 	};
 
 	vtt_fixed: fixedregulator-vtt {
Signature · 1 line
2.4.9
#2 lore raw

PATCH 2/37 ARM: dts: am43xx-epos-evm: Remove unneeded regulator property

In reply to Laurent Pinchart

The enable-active-high regulator property only makes sense when an enable GPIO is specified. Remove it.

---
 arch/arm/boot/dts/am43x-epos-evm.dts | 1 -
 1 file changed, 1 deletion(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/am43x-epos-evm.dts +0 -1
index 795d68af6df9..705048bf17c5 100644
@@ -28,7 +28,6 @@
 		regulator-name = "vmmcsd_fixed";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		enable-active-high;
 	};
 
 	lcd0: display {
Signature · 1 line
2.4.9
#3 lore raw

PATCH 3/37 ARM: mvebu: Armada 388 GP: Remove unneeded regulator property

In reply to Laurent Pinchart

The enable-active-high regulator property only makes sense when an enable GPIO is specified. Remove it.

---
 arch/arm/boot/dts/armada-388-gp.dts | 2 --
 1 file changed, 2 deletions(-)
Cc Jason Cooper <[email protected]>
Cc Andrew Lunn <[email protected]>
Cc Gregory Clement <[email protected]>
Cc Sebastian Hesselbarth <[email protected]>
arch/arm/boot/dts/armada-388-gp.dts +0 -2
index fd4f6fd8a2e8..ebf6e8c2c522 100644
@@ -301,9 +301,7 @@
 	reg_sata0: pwr-sata0 {
 		compatible = "regulator-fixed";
 		regulator-name = "pwr_en_sata0";
-		enable-active-high;
 		regulator-always-on;
-
 	};
 
 	reg_5v_sata0: v5-sata0 {
Signature · 1 line
2.4.9
#4 lore raw

PATCH 3/37 ARM: mvebu: Armada 388 GP: Remove unneeded regulator property

Laurent,

Quoted 7 lines · Laurent Pinchart

The enable-active-high regulator property only makes sense when an enable GPIO is specified. Remove it.

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

Thanks, but this patch is no longer needed, since commit b5999dcbdb96570c130772ac2055a4d34ec8b3d7 actually added the reference to the gpio controlling the regulator.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#5 lore raw

PATCH 4/37 ARM: imx6sx-sdb: Fix typo in regulator enable GPIO property

In reply to Laurent Pinchart

The property name should be "gpio", not "gpios". Fix it.

---
 arch/arm/boot/dts/imx6sx-sdb.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx6sx-sdb.dtsi +1 -1
index ac88c3467078..94ac4005d9cd 100644
@@ -114,7 +114,7 @@
 			regulator-name = "peri_3v3";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpios = <&gpio4 16 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio4 16 GPIO_ACTIVE_HIGH>;
 			enable-active-high;
 			regulator-always-on;
 		};
Signature · 1 line
2.4.9
#6 lore raw

PATCH 5/37 ARM: dts: s5pv210-aquila: Fix typo in regulator enable GPIO property

In reply to Laurent Pinchart

The property name should be "gpio", not "gpios". Fix it.

---
 arch/arm/boot/dts/s5pv210-aquila.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-samsung-soc at vger.kernel.org
Cc Kukjin Kim <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
arch/arm/boot/dts/s5pv210-aquila.dts +1 -1
index f00cea7aca2f..aa64faa72970 100644
@@ -46,7 +46,7 @@
 			regulator-name = "V_TF_2.8V";
 			regulator-min-microvolt = <2800000>;
 			regulator-max-microvolt = <2800000>;
-			gpios = <&mp05 4 0>;
+			gpio = <&mp05 4 0>;
 			enable-active-high;
 		};
 
Signature · 1 line
2.4.9
#7 lore raw

PATCH 5/37 ARM: dts: s5pv210-aquila: Fix typo in regulator enable GPIO property

Quoted 10 lines · Laurent Pinchart

The property name should be "gpio", not "gpios". Fix it.

---
 arch/arm/boot/dts/s5pv210-aquila.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-samsung-soc at vger.kernel.org
Cc Kukjin Kim <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

#8 lore raw

PATCH 6/37 ARM: dts: s5pv210-goni: Fix typo in regulator enable GPIO property

In reply to Laurent Pinchart

The property name should be "gpio", not "gpios". Fix it.

---
 arch/arm/boot/dts/s5pv210-goni.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc linux-samsung-soc at vger.kernel.org
Cc Kukjin Kim <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
arch/arm/boot/dts/s5pv210-goni.dts +2 -2
index a3d4643b202e..3b76eeeb8410 100644
@@ -47,7 +47,7 @@
 			regulator-min-microvolt = <2800000>;
 			regulator-max-microvolt = <2800000>;
 			reg = <0>;
-			gpios = <&mp05 4 0>;
+			gpio = <&mp05 4 0>;
 			enable-active-high;
 		};
 
@@ -73,7 +73,7 @@
 			regulator-min-microvolt = <2800000>;
 			regulator-max-microvolt = <2800000>;
 			reg = <3>;
-			gpios = <&gpj1 3 0>;
+			gpio = <&gpj1 3 0>;
 			enable-active-high;
 		};
 	};
Signature · 1 line
2.4.9
#9 lore raw

PATCH 6/37 ARM: dts: s5pv210-goni: Fix typo in regulator enable GPIO property

Quoted 10 lines · Laurent Pinchart

The property name should be "gpio", not "gpios". Fix it.

---
 arch/arm/boot/dts/s5pv210-goni.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc linux-samsung-soc at vger.kernel.org
Cc Kukjin Kim <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

#10 lore raw

PATCH 7/37 ARM: dts: omap3-evm: Remove invalid enable-active-low regulator property

In reply to Laurent Pinchart

The property isn't part of the regulator-fixed DT bindings and isn't used by the driver. Remove it.

---
 arch/arm/boot/dts/omap3-evm-common.dtsi | 1 -
 1 file changed, 1 deletion(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/omap3-evm-common.dtsi +0 -1
index b2589f96d5f7..c55f256d014b 100644
@@ -76,7 +76,6 @@
 
 &lcd_3v3 {
 	gpio = <&gpio5 25 GPIO_ACTIVE_LOW>;	/* gpio153 */
-	enable-active-low;
 };
 
 &lcd0 {
Signature · 1 line
2.4.9
#11 lore raw

PATCH 8/37 ARM: dts: omap3-sb-t35: Remove invalid enable-active-low regulator property

In reply to Laurent Pinchart

The property isn't part of the regulator-fixed DT bindings and isn't used by the driver. Remove it.

---
 arch/arm/boot/dts/omap3-sb-t35.dtsi | 1 -
 1 file changed, 1 deletion(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/omap3-sb-t35.dtsi +0 -1
index 827f614261f6..b77e25d20e07 100644
@@ -50,7 +50,6 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&sb_t35_audio_amp>;
 		gpio = <&gpio2 29 GPIO_ACTIVE_LOW>;   /* gpio_61 */
-		enable-active-low;
 		regulator-always-on;
 	};
 };
Signature · 1 line
2.4.9
#12 lore raw

PATCH 9/37 ARM: dts: omap3-tao3530: Remove invalid enable-active-low regulator property

In reply to Laurent Pinchart

The property isn't part of the regulator-fixed DT bindings and isn't used by the driver. Remove it.

---
 arch/arm/boot/dts/omap3-tao3530.dtsi | 1 -
 1 file changed, 1 deletion(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/omap3-tao3530.dtsi +0 -1
index 7bd8d9a4f67f..a520b4fdcf20 100644
@@ -63,7 +63,6 @@
 		regulator-min-microvolt = <3150000>;
 		regulator-max-microvolt = <3150000>;
 		gpio = <&gpio5 29 GPIO_ACTIVE_LOW>;		/* gpio_157 */
-		enable-active-low;
 		startup-delay-us = <10000>;
 	};
 };
Signature · 1 line
2.4.9
#13 lore raw

PATCH 10/37 ARM: dts: imx6qdl-tx6: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

While at it remove the enable-active-low property as it isn't part of the regulator-fixed DT bindings and isn't used by the driver.

---
 arch/arm/boot/dts/imx6qdl-tx6.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx6qdl-tx6.dtsi +1 -2
index da08de324e9e..8d472ae3e8ae 100644
@@ -108,8 +108,7 @@
 			regulator-max-microvolt = <3300000>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_flexcan_xcvr>;
-			gpio = <&gpio4 21 GPIO_ACTIVE_HIGH>;
-			enable-active-low;
+			gpio = <&gpio4 21 GPIO_ACTIVE_LOW>;
 		};
 
 		reg_lcd0_pwr: regulator at 4 {
Signature · 1 line
2.4.9
#14 lore raw

PATCH 11/37 ARM: dts: dove-cm-a510: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/dove-cm-a510.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Jason Cooper <[email protected]>
Cc Andrew Lunn <[email protected]>
Cc Gregory Clement <[email protected]>
Cc Sebastian Hesselbarth <[email protected]>
arch/arm/boot/dts/dove-cm-a510.dtsi +1 -1
index 59b4056b478f..d5d84a61437f 100644
@@ -118,7 +118,7 @@
 			regulator-name = "WiFi Power";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio2 7 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio2 7 GPIO_ACTIVE_LOW>;
 		};
 	};
 };
Signature · 1 line
2.4.9
#15 lore raw

PATCH 12/37 ARM: dts: dove-sbc-a510: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/dove-sbc-a510.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc Jason Cooper <[email protected]>
Cc Andrew Lunn <[email protected]>
Cc Gregory Clement <[email protected]>
Cc Sebastian Hesselbarth <[email protected]>
arch/arm/boot/dts/dove-sbc-a510.dts +2 -2
index 288e707dea99..3bc5c96483be 100644
@@ -82,7 +82,7 @@
 			regulator-name = "USB Power";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio_ext 0 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio_ext 0 GPIO_ACTIVE_LOW>;
 		};
 
 		mmc_power: regulator at 3 {
@@ -90,7 +90,7 @@
 			regulator-name = "MMC Power";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio_ext 13 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio_ext 13 GPIO_ACTIVE_LOW>;
 		};
 	};
 };
Signature · 1 line
2.4.9
#16 lore raw

PATCH 12/37 ARM: dts: dove-sbc-a510: Fix regulator enable GPIO polarity

Quoted 20 lines · Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/dove-sbc-a510.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc Jason Cooper <[email protected]>
Cc Andrew Lunn <[email protected]>
Cc Gregory Clement <[email protected]>
Cc Sebastian Hesselbarth <[email protected]>
arch/arm/boot/dts/dove-sbc-a510.dts +0 -0
index 288e707dea99..3bc5c96483be 100644

arch/arm/boot/dts/dove-sbc-a510.dts line 85

Quoted 5 lines
arch/arm/boot/dts/dove-sbc-a510.dts +1 -1
@@ -82,7 +82,7 @@
 			regulator-name = "USB Power";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio_ext 0 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio_ext 0 GPIO_ACTIVE_LOW>;

I'd feel a lot better about this if the commit log made a strong reference back to some documentation or other hardware description. Just because Linux doesn't use it or uses it wrong doesn't necessarily mean the DT is wrong.

thx,

Jason.

Quoted 15 lines
arch/arm/boot/dts/dove-sbc-a510.dts +1 -1
 		};
 
 		mmc_power: regulator at 3 {
@@ -90,7 +90,7 @@
 			regulator-name = "MMC Power";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio_ext 13 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio_ext 13 GPIO_ACTIVE_LOW>;
 		};
 	};
 };
Signature · 1 line
2.4.9
#17 lore raw

PATCH 12/37 ARM: dts: dove-sbc-a510: Fix regulator enable GPIO polarity

Hi Jason,

Quoted 17 lines · On Monday 12 October 2015 22:50:09 Jason Cooper wrote:
Quoted 16 lines · Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---

 arch/arm/boot/dts/dove-sbc-a510.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc Jason Cooper <[email protected]>
Cc Andrew Lunn <[email protected]>
Cc Gregory Clement <[email protected]>
Cc Sebastian Hesselbarth <[email protected]>
Quoted 16 lines

arch/arm/boot/dts/dove-sbc-a510.dts line 85

Quoted 11 lines
(quoted diff) +0 -0

b/arch/arm/boot/dts/dove-sbc-a510.dts index 288e707dea99..3bc5c96483be 100644

arch/arm/boot/dts/dove-sbc-a510.dts +1 -1
@@ -82,7 +82,7 @@
 			regulator-name = "USB Power";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio_ext 0 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio_ext 0 GPIO_ACTIVE_LOW>;

I'd feel a lot better about this if the commit log made a strong reference back to some documentation or other hardware description. Just because Linux doesn't use it or uses it wrong doesn't necessarily mean the DT is wrong.

It looks like I might be wrong, see Stephen's reply to patch 37/37. Let's discuss the issue there.

Quoted 12 lines
Quoted 12 lines
arch/arm/boot/dts/dove-sbc-a510.dts +1 -1
 		};
 
 		mmc_power: regulator at 3 {
@@ -90,7 +90,7 @@
 			regulator-name = "MMC Power";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio_ext 13 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio_ext 13 GPIO_ACTIVE_LOW>;
 		};
 	};
 };
Signature · 3 lines
Regards,

Laurent Pinchart
#18 lore raw

PATCH 12/37 ARM: dts: dove-sbc-a510: Fix regulator enable GPIO polarity

Hey Laurent,

Quoted 35 lines · Laurent Pinchart
Quoted 17 lines · On Monday 12 October 2015 22:50:09 Jason Cooper wrote:
Quoted 16 lines · Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---

 arch/arm/boot/dts/dove-sbc-a510.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc Jason Cooper <[email protected]>
Cc Andrew Lunn <[email protected]>
Cc Gregory Clement <[email protected]>
Cc Sebastian Hesselbarth <[email protected]>
Quoted 16 lines

arch/arm/boot/dts/dove-sbc-a510.dts line 85

Quoted 11 lines
(quoted diff) +0 -0

b/arch/arm/boot/dts/dove-sbc-a510.dts index 288e707dea99..3bc5c96483be 100644

arch/arm/boot/dts/dove-sbc-a510.dts +1 -1
@@ -82,7 +82,7 @@
 			regulator-name = "USB Power";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio_ext 0 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio_ext 0 GPIO_ACTIVE_LOW>;

I'd feel a lot better about this if the commit log made a strong reference back to some documentation or other hardware description. Just because Linux doesn't use it or uses it wrong doesn't necessarily mean the DT is wrong.

Quoted 2 lines

It looks like I might be wrong, see Stephen's reply to patch 37/37. Let's discuss the issue there.

No problem, as long as it's being addressed, I'll keep an eye out for a v2. I've some email remailer sketchyness resulting in a lack of subscription to the MLs atm. And I'm not on the 37/37 thread.

thx,

Jason.

#19 lore raw

PATCH 13/37 ARM: dts: exynos5250-arndale: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active high, but is flagged as active low in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/exynos5250-arndale.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/exynos5250-arndale.dts +1 -1
index 7e728a1b5559..e8703f0568c9 100644
@@ -90,7 +90,7 @@
 			regulator-name = "VDD_33ON_2.8V";
 			regulator-min-microvolt = <2800000>;
 			regulator-max-microvolt = <2800000>;
-			gpio = <&gpx1 1 GPIO_ACTIVE_LOW>;
+			gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>;
 			enable-active-high;
 		};
 
Signature · 1 line
2.4.9
#20 lore raw

PATCH 13/37 ARM: dts: exynos5250-arndale: Fix regulator enable GPIO polarity

Quoted 5 lines · Laurent Pinchart

The enable GPIO is active high, but is flagged as active low in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

Quoted 7 lines
---
 arch/arm/boot/dts/exynos5250-arndale.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>

This one should be sent to Samsung folks instead.

Shawn

#21 lore raw

PATCH 14/37 ARM: dts: imx23-evk: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx23-evk.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx23-evk.dts +2 -1
index 57e29977ba06..b6709ff69ebe 100644
@@ -10,6 +10,7 @@
  */
 
 /dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
 #include "imx23.dtsi"
 
 / {
@@ -136,7 +137,7 @@
 			regulator-name = "vddio-sd0";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio1 29 0>;
+			gpio = <&gpio1 29 GPIO_ACTIVE_LOW>;
 		};
 
 		reg_lcd_3v3: regulator at 1 {
Signature · 1 line
2.4.9
#22 lore raw

PATCH 15/37 ARM: dts: imx23-stmp378x_devb: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx23-stmp378x_devb.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx23-stmp378x_devb.dts +2 -1
index 455169e99d49..928f5556ae7b 100644
@@ -10,6 +10,7 @@
  */
 
 /dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
 #include "imx23.dtsi"
 
 / {
@@ -75,7 +76,7 @@
 			regulator-name = "vddio-sd0";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio1 29 0>;
+			gpio = <&gpio1 29 GPIO_ACTIVE_LOW>;
 		};
 	};
 };
Signature · 1 line
2.4.9
#23 lore raw

PATCH 16/37 ARM: dts: imx25-pdk: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx25-pdk.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx25-pdk.dts +1 -1
index 9351296356dc..71d358f6c707 100644
@@ -59,7 +59,7 @@
 			regulator-name = "can-3v3";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio4 6 0>;
+			gpio = <&gpio4 6 GPIO_ACTIVE_LOW>;
 		};
 	};
 
Signature · 1 line
2.4.9
#24 lore raw

PATCH 17/37 ARM: dts: imx28-cfa10036: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx28-cfa10036.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx28-cfa10036.dts +1 -1
index 570aa339a05e..b053057d50fa 100644
@@ -139,6 +139,6 @@
 		regulator-name = "vddio-sd0";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		gpio = <&gpio3 28 0>;
+		gpio = <&gpio3 28 GPIO_ACTIVE_LOW>;
 	};
 };
Signature · 1 line
2.4.9
#25 lore raw

PATCH 18/37 ARM: dts: imx28-evk: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx28-evk.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx28-evk.dts +2 -2
index 279249b8c3f3..17598d3bb408 100644
@@ -295,7 +295,7 @@
 			regulator-name = "vddio-sd0";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio3 28 0>;
+			gpio = <&gpio3 28 GPIO_ACTIVE_LOW>;
 		};
 
 		reg_fec_3v3: regulator at 2 {
@@ -304,7 +304,7 @@
 			regulator-name = "fec-3v3";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio2 15 0>;
+			gpio = <&gpio2 15 GPIO_ACTIVE_LOW>;
 		};
 
 		reg_usb0_vbus: regulator at 3 {
Signature · 1 line
2.4.9
#26 lore raw

PATCH 19/37 ARM: dts: imx28-m28cu3: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx28-m28cu3.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx28-m28cu3.dts +2 -2
index 2df63bee6f4e..026a17e05dc9 100644
@@ -246,7 +246,7 @@
 			regulator-name = "vddio-sd0";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio3 29 0>;
+			gpio = <&gpio3 29 GPIO_ACTIVE_LOW>;
 		};
 
 		reg_vddio_sd1: regulator at 2 {
@@ -255,7 +255,7 @@
 			regulator-name = "vddio-sd1";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio2 19 0>;
+			gpio = <&gpio2 19 GPIO_ACTIVE_LOW>;
 		};
 
 		reg_usb1_vbus: regulator at 3 {
Signature · 1 line
2.4.9
#27 lore raw

PATCH 20/37 ARM: dts: imx28-m28evk: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx28-m28evk.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx28-m28evk.dts +3 -3
index e35cc6ba3ca6..c34cc49cfe42 100644
@@ -243,7 +243,7 @@
 			regulator-name = "vddio-sd0";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio3 28 0>;
+			gpio = <&gpio3 28 GPIO_ACTIVE_LOW>;
 		};
 
 		reg_usb0_vbus: regulator at 2 {
@@ -252,7 +252,7 @@
 			regulator-name = "usb0_vbus";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio3 12 0>;
+			gpio = <&gpio3 12 GPIO_ACTIVE_LOW>;
 		};
 
 		reg_usb1_vbus: regulator at 3 {
@@ -261,7 +261,7 @@
 			regulator-name = "usb1_vbus";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio3 13 0>;
+			gpio = <&gpio3 13 GPIO_ACTIVE_LOW>;
 		};
 	};
 
Signature · 1 line
2.4.9
#28 lore raw

PATCH 21/37 ARM: dts: imx28-sps1: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx28-sps1.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx28-sps1.dts +1 -1
index 0ce3cb8e7914..d68886646941 100644
@@ -136,7 +136,7 @@
 			regulator-name = "usb0_vbus";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio3 9 0>;
+			gpio = <&gpio3 9 GPIO_ACTIVE_LOW>;
 		};
 	};
 
Signature · 1 line
2.4.9
#29 lore raw

PATCH 22/37 ARM: dts: imx28-tx28: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx28-tx28.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx28-tx28.dts +1 -1
index a5b27c85a91c..480c5ecaea2d 100644
@@ -93,7 +93,7 @@
 			regulator-name = "CAN XCVR";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
-			gpio = <&gpio1 0 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio1 0 GPIO_ACTIVE_LOW>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&tx28_flexcan_xcvr_pins>;
 		};
Signature · 1 line
2.4.9
#30 lore raw

PATCH 23/37 ARM: dts: imx53-m53evk: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx53-m53evk.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx53-m53evk.dts +1 -1
index 53f40885c530..e2145338580f 100644
@@ -82,7 +82,7 @@
 			regulator-name = "vbus";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio1 2 0>;
+			gpio = <&gpio1 2 GPIO_ACTIVE_LOW>;
 		};
 	};
 
Signature · 1 line
2.4.9
#31 lore raw

PATCH 24/37 ARM: dts: imx53-mba53: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx53-mba53.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx53-mba53.dts +1 -1
index 2e44d2aba14e..8eb4c58b2f24 100644
@@ -53,7 +53,7 @@
 			compatible = "regulator-fixed";
 			reg = <0>;
 			regulator-name = "lcd-supply";
-			gpio = <&gpio2 5 0>;
+			gpio = <&gpio2 5 GPIO_ACTIVE_LOW>;
 			startup-delay-us = <5000>;
 		};
 
Signature · 1 line
2.4.9
#32 lore raw

PATCH 25/37 ARM: dts: imx53-tx53: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx53-tx53.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx53-tx53.dtsi +1 -1
index d3e50b22064f..62a324961ec8 100644
@@ -95,7 +95,7 @@
 			regulator-max-microvolt = <3300000>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_can_xcvr>;
-			gpio = <&gpio4 21 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio4 21 GPIO_ACTIVE_LOW>;
 		};
 
 		reg_usbh1_vbus: regulator at 3 {
Signature · 1 line
2.4.9
#33 lore raw

PATCH 26/37 ARM: dts: imx6q-dmo-edmqmx6: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts +1 -1
index 4fa254347798..3272d8048ace 100644
@@ -53,7 +53,7 @@
 			regulator-name = "usb_otg_switch";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio7 12 0>;
+			gpio = <&gpio7 12 GPIO_ACTIVE_LOW>;
 			regulator-boot-on;
 			regulator-always-on;
 		};
Signature · 1 line
2.4.9
#34 lore raw

PATCH 27/37 ARM: dts: kirkwood-blackarmor-nas220: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIOs are active high, but are flagged as active low in the gpio properties. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc Jason Cooper <[email protected]>
Cc Andrew Lunn <[email protected]>
Cc Gregory Clement <[email protected]>
Cc Sebastian Hesselbarth <[email protected]>
arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts +2 -2
index fa02a9aff05e..8d155295d528 100644
@@ -75,7 +75,7 @@
 			enable-active-high;
 			regulator-always-on;
 			regulator-boot-on;
-			gpio = <&gpio0 24 GPIO_ACTIVE_LOW>;
+			gpio = <&gpio0 24 GPIO_ACTIVE_HIGH>;
 		};
 
 		sata1_power: regulator at 2 {
@@ -87,7 +87,7 @@
 			enable-active-high;
 			regulator-always-on;
 			regulator-boot-on;
-			gpio = <&gpio0 28 GPIO_ACTIVE_LOW>;
+			gpio = <&gpio0 28 GPIO_ACTIVE_HIGH>;
 		};
 	};
 };
Signature · 1 line
2.4.9
#35 lore raw

PATCH 28/37 ARM: dts: omap4-duovero: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active high, but is flagged as active low in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/omap4-duovero.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/omap4-duovero.dtsi +1 -1
index f2a94fa62552..23c1a74f91ea 100644
@@ -56,7 +56,7 @@
 		regulator-name = "w2cbw0015";
 		regulator-min-microvolt = <3000000>;
 		regulator-max-microvolt = <3000000>;
-		gpio = <&gpio2 11 GPIO_ACTIVE_LOW>;		/* gpio_43 */
+		gpio = <&gpio2 11 GPIO_ACTIVE_HIGH>;		/* gpio_43 */
 		startup-delay-us = <70000>;
 		enable-active-high;
 		regulator-boot-on;
Signature · 1 line
2.4.9
#36 lore raw

PATCH 29/37 ARM: dts: kirkwood-nsa3x0-common: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/kirkwood-nsa3x0-common.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Jason Cooper <[email protected]>
Cc Andrew Lunn <[email protected]>
Cc Gregory Clement <[email protected]>
Cc Sebastian Hesselbarth <[email protected]>
arch/arm/boot/dts/kirkwood-nsa3x0-common.dtsi +1 -1
index 2075a2e828f1..c4b17f396e1e 100644
@@ -110,7 +110,7 @@
 			regulator-max-microvolt = <5000000>;
 			regulator-always-on;
 			regulator-boot-on;
-			gpio = <&gpio0 21 GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio0 21 GPIO_ACTIVE_LOW>;
 		};
 	};
 };
Signature · 1 line
2.4.9
#37 lore raw

PATCH 30/37 ARM: dts: omap3-beagle-xm: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/omap3-beagle-xm.dts +1 -1
index 7c4dca122a91..f44f17673061 100644
@@ -80,7 +80,7 @@
 		regulator-name = "hsusb2_vbus";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		gpio = <&twl_gpio 18 0>;	/* GPIO LEDA */
+		gpio = <&twl_gpio 18 GPIO_ACTIVE_LOW>;	/* GPIO LEDA */
 		startup-delay-us = <70000>;
 	};
 
Signature · 1 line
2.4.9
#38 lore raw

PATCH 31/37 ARM: dts: omap3-beagle: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/omap3-beagle.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/omap3-beagle.dts +1 -1
index a5474113cd50..0849035643fc 100644
@@ -55,7 +55,7 @@
 		regulator-name = "hsusb2_vbus";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		gpio = <&twl_gpio 18 0>;	/* GPIO LEDA */
+		gpio = <&twl_gpio 18 GPIO_ACTIVE_LOW>;	/* GPIO LEDA */
 		startup-delay-us = <70000>;
 	};
 
Signature · 1 line
2.4.9
#39 lore raw

PATCH 32/37 ARM: dts: omap3-overo-base: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/omap3-overo-base.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/omap3-overo-base.dtsi +2 -2
index 18e1649681c1..1fa59d0e1e8a 100644
@@ -65,7 +65,7 @@
 		regulator-name = "regulator-w3cbw003c-wifi-nreset";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		gpio = <&gpio1 16 GPIO_ACTIVE_HIGH>;		/* gpio_16: WiFi nReset */
+		gpio = <&gpio1 16 GPIO_ACTIVE_LOW>;		/* gpio_16: WiFi nReset */
 		startup-delay-us = <10000>;
 	};
 
@@ -75,7 +75,7 @@
 		regulator-name = "regulator-w3cbw003c-bt-nreset";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		gpio = <&gpio6 4 GPIO_ACTIVE_HIGH>;		/* gpio_164: BT nReset */
+		gpio = <&gpio6 4 GPIO_ACTIVE_LOW>;		/* gpio_164: BT nReset */
 		startup-delay-us = <10000>;
 	};
 };
Signature · 1 line
2.4.9
#40 lore raw

PATCH 33/37 ARM: dts: omap3-tao3530: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/omap3-tao3530.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-omap at vger.kernel.org
Cc Benoit Cousson <[email protected]>
Cc Tony Lindgren <[email protected]>
arch/arm/boot/dts/omap3-tao3530.dtsi +1 -1
index a520b4fdcf20..68f75be5d99a 100644
@@ -37,7 +37,7 @@
 		regulator-name = "hsusb2_vbus";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		gpio = <&twl_gpio 18 0>;	/* GPIO LEDA */
+		gpio = <&twl_gpio 18 GPIO_ACTIVE_LOW>;	/* GPIO LEDA */
 		startup-delay-us = <70000>;
 	};
 
Signature · 1 line
2.4.9
#41 lore raw

PATCH 34/37 ARM: dts: tegra20-harmony: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/tegra20-harmony.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-tegra at vger.kernel.org
Cc Stephen Warren <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Alexandre Courbot <[email protected]>
arch/arm/boot/dts/tegra20-harmony.dts +1 -1
index b926a07b9443..d18801fe6730 100644
@@ -688,7 +688,7 @@
 			regulator-name = "vdd_1v5";
 			regulator-min-microvolt = <1500000>;
 			regulator-max-microvolt = <1500000>;
-			gpio = <&pmic 0 GPIO_ACTIVE_HIGH>;
+			gpio = <&pmic 0 GPIO_ACTIVE_LOW>;
 		};
 
 		regulator at 2 {
Signature · 1 line
2.4.9
#42 lore raw

PATCH 35/37 ARM: dts: tegra20-iris-512: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/tegra20-iris-512.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-tegra at vger.kernel.org
Cc Stephen Warren <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Alexandre Courbot <[email protected]>
arch/arm/boot/dts/tegra20-iris-512.dts +1 -1
index 1dd7d7bfdfcc..9b04a367575c 100644
@@ -85,7 +85,7 @@
 			regulator-max-microvolt = <5000000>;
 			regulator-boot-on;
 			regulator-always-on;
-			gpio = <&gpio TEGRA_GPIO(W, 2) GPIO_ACTIVE_HIGH>;
+			gpio = <&gpio TEGRA_GPIO(W, 2) GPIO_ACTIVE_LOW>;
 		};
 
 		vcc_sd_reg: regulator at 1 {
Signature · 1 line
2.4.9
#43 lore raw

PATCH 36/37 ARM: dts: tegra20-seaboard: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/tegra20-seaboard.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-tegra at vger.kernel.org
Cc Stephen Warren <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Alexandre Courbot <[email protected]>
arch/arm/boot/dts/tegra20-seaboard.dts +1 -1
index aea8994b35f2..1d790cd59c21 100644
@@ -850,7 +850,7 @@
 			regulator-name = "vdd_1v5";
 			regulator-min-microvolt = <1500000>;
 			regulator-max-microvolt = <1500000>;
-			gpio = <&pmic 0 GPIO_ACTIVE_HIGH>;
+			gpio = <&pmic 0 GPIO_ACTIVE_LOW>;
 		};
 
 		regulator at 2 {
Signature · 1 line
2.4.9
#44 lore raw

PATCH 37/37 ARM: dts: tegra20-ventana: Fix regulator enable GPIO polarity

In reply to Laurent Pinchart

The enable GPIO is active low, but is flagged as active high in the gpio property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

---
 arch/arm/boot/dts/tegra20-ventana.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cc linux-tegra at vger.kernel.org
Cc Stephen Warren <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Alexandre Courbot <[email protected]>
arch/arm/boot/dts/tegra20-ventana.dts +1 -1
index 04c58e9ca490..ba1fc1487c69 100644
@@ -635,7 +635,7 @@
 			regulator-name = "vdd_1v5";
 			regulator-min-microvolt = <1500000>;
 			regulator-max-microvolt = <1500000>;
-			gpio = <&pmic 0 GPIO_ACTIVE_HIGH>;
+			gpio = <&pmic 0 GPIO_ACTIVE_LOW>;
 		};
 
 		regulator at 2 {
Signature · 1 line
2.4.9
#45 lore raw

PATCH 37/37 ARM: dts: tegra20-ventana: Fix regulator enable GPIO polarity

Quoted 1 line · Laurent Pinchart

The enable GPIO is active low,

It'd be good to mention a justification for that statement in the patches, since the cover letter isn't going to be checked in.

Quoted 4 lines

but is flagged as active high in the gpio
property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

IIRC the history here was that for some bizarre reason not all GPIO bindings contained an active-high/low flag and there was resistance to extending them in a backwards compatible way. So the regulator binding needed the separate property to represent this. For bindings that did have the flag, we had to set the GPIO flag to active-high, so that if anything started honoring the GPIO flags (e.g. I thikn the gpiod API does today, but the legacy GPIO API doesn't), we wouldn't apply both "active low indicators", and end up driving an active-high signal, and breaking things.

So while this change is logically correct when read in isolation (and for Harmony, Seaboard, and Ventana I verified that these regulators do use an active-low GPIO), I worry that making it makes mistakes likely later. How would we mitigate that?

#46 lore raw

PATCH 37/37 ARM: dts: tegra20-ventana: Fix regulator enable GPIO polarity

Hi Stephen,

Quoted 21 lines · On Monday 12 October 2015 15:34:43 Stephen Warren wrote:
Quoted 1 line · Laurent Pinchart

The enable GPIO is active low,

It'd be good to mention a justification for that statement in the patches, since the cover letter isn't going to be checked in.

Quoted 4 lines

but is flagged as active high in the gpio
property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

IIRC the history here was that for some bizarre reason not all GPIO bindings contained an active-high/low flag and there was resistance to extending them in a backwards compatible way. So the regulator binding needed the separate property to represent this. For bindings that did have the flag, we had to set the GPIO flag to active-high, so that if anything started honoring the GPIO flags (e.g. I thikn the gpiod API does today, but the legacy GPIO API doesn't), we wouldn't apply both "active low indicators", and end up driving an active-high signal, and breaking things.

Quoted 4 lines

So while this change is logically correct when read in isolation (and for Harmony, Seaboard, and Ventana I verified that these regulators do use an active-low GPIO), I worry that making it makes mistakes likely later. How would we mitigate that?

That's a very good point. Is the resistance to move to the standard GPIO active low/high flags still present, or is it now only history ? In other words, could we aim for using GPIO flags as the primary method to specify polarities, and fall back to the custom properties for backward compatibility (and possibly for GPIO controllers that don't support the flags) ?

Signature · 3 lines
Regards,

Laurent Pinchart
#47 lore raw

PATCH 37/37 ARM: dts: tegra20-ventana: Fix regulator enable GPIO polarity

Quoted 24 lines · Laurent Pinchart

Hi Stephen,

Quoted 21 lines · On Monday 12 October 2015 15:34:43 Stephen Warren wrote:
Quoted 1 line · Laurent Pinchart

The enable GPIO is active low,

It'd be good to mention a justification for that statement in the patches, since the cover letter isn't going to be checked in.

Quoted 4 lines

but is flagged as active high in the gpio
property. As the gpio property flags are currently unused by the driver this doesn't cause any issue for now, but will break later if the driver starts making use of the flags. Fix it.

IIRC the history here was that for some bizarre reason not all GPIO bindings contained an active-high/low flag and there was resistance to extending them in a backwards compatible way. So the regulator binding needed the separate property to represent this. For bindings that did have the flag, we had to set the GPIO flag to active-high, so that if anything started honoring the GPIO flags (e.g. I thikn the gpiod API does today, but the legacy GPIO API doesn't), we wouldn't apply both "active low indicators", and end up driving an active-high signal, and breaking things.

Quoted 7 lines
Quoted 4 lines

So while this change is logically correct when read in isolation (and for Harmony, Seaboard, and Ventana I verified that these regulators do use an active-low GPIO), I worry that making it makes mistakes likely later. How would we mitigate that?

That's a very good point. Is the resistance to move to the standard GPIO active low/high flags still present, or is it now only history ?

This was a few years back, so I don't remember the details; it might have been as simple as "some bindings don't already have GPIO flags, and I'd rather get GPIO regulators implemented first before thinking about fixing that" or it could have been "some bindings don't already have GPIO flags, and there's ${some reason} why it's not possible to solve that in a backwards-compatible fashion" (recalling that DT bindings must evolve in a backwards-compatible fashion since they're an ABI). Unfortunately, you'd have to read through the mailing list posts related to the patches that defined the GPIO regulator bindings or added the nodes to DT.

Quoted 4 lines

In other
words, could we aim for using GPIO flags as the primary method to specify polarities, and fall back to the custom properties for backward compatibility (and possibly for GPIO controllers that don't support the flags) ?

I don't think we can switch to using GPIO flags, without changing the compatible values for the relevant DT nodes.

For one, we'd need some way of actively marking the nodes to say whether they are written to expect that the GPIO flags or the other properties be used. It's not possible in all cases to determine this automatically. For example, if enable-active-high it's fairly clear we should honor this flag, yet if it's missing does that mean the GPIO is active-low or simply that the node was written to expect that the GPIO flags be used instead?

Also, old DTs must work with new kernels (and preferably also, new DTs must work with old kernels). If the GPIO flags are wrong in current DTs, then we can't use them. Of course, there's an argument that the backwards-compatibility constraint doesn't apply to buggy DTs, just to correctly written DTs. However, if we deliberately chose to make all regulator GPIO flags ACTIVE_HIGH, then the current DTs aren't buggy.

#48 lore raw
In reply to Laurent Pinchart
Quoted 73 lines · Laurent Pinchart

Hello,

While working on regulators, GPIOs and DT I noticed that many of our DT source files incorrectly describe fixed regulators. The common error patterns are

- Usage of the undefined (and never parsed) enable-active-low property
- Usage of the enable-active-high property without specifying an enable GPIO
- Typos in the enabl GPIO property name (gpios instead of gpio)
- Mismatch between the enable-active-high property (or the lack thereof) and the enable GPIO flags

This patch series fixes those issues in all the DT sources after locating the errors using the following script.

------------------------------------------------------------------------------
#!/bin/sh

echo $1
cat $1 | awk '

BEGIN {
	open_drain = 0;
	active_high = 0;
	gpio = 0;
	flags = 0;
}
match($0, /([a-zA-Z0-9 at _-]*) {/, ary) {
	name = ary[1];
}
/compatible.*"regulator-fixed"/ {
	found = 1;
}
/enable-active-high/ {
	active_high = 1;
}
/gpio-open-drain/ {
	open_drain = 1;
}
match($0, /gpio += <.* ([^ ]*)>/, ary) {
	gpio = 1;
	flags = ary[1];
	if (flags == 0)
		flags = "GPIO_ACTIVE_HIGH";
}
/}/ {
	if (found) {
		if (gpio) {
			print "\t" name ": active high " active_high " " flags " open drain " open_drain;
			if ((active_high && flags == "GPIO_ACTIVE_LOW") ||
			    (!active_high && flags == "GPIO_ACTIVE_HIGH"))
				print "WARNING: enable-active-high and flags do not match"
		} else {
			if (active_high)
				print "WARNING: active high without GPIO"
			if (open_drain)
				print "WARNING: open drain without GPIO"
		}
	}

	gpio = 0;
	found = 0;
	active_high = 0;
	open_drain = 0;
	flags = 0;
}

'
------------------------------------------------------------------------------

Quoted 7 lines

All patches except for the ones touching omap3-beagle-xm and omap3-overo-base are untested as I lack test hardware.

As there's no dependency between the patches touching different source files the appropriate maintainers could take their share of the patches in their tree. Alternatively I could send a single pull request after collecting all acks but that might be more complex.

Nice clean-up. For omaps, there's an earlier patch posted by Javier Martinez Canillas <[email protected]> as "[PATCH] ARM: dts: Use defined GPIO constants in flags cell for OMAP2+ boards". Can you guys do some cross checking and let me know which combination I should appluy for omaps?

Regards,

Tony

#49 lore raw

Hello Tony,

Quoted 82 lines · Tony Lindgren
Quoted 73 lines · Laurent Pinchart

Hello,

While working on regulators, GPIOs and DT I noticed that many of our DT source files incorrectly describe fixed regulators. The common error patterns are

- Usage of the undefined (and never parsed) enable-active-low property
- Usage of the enable-active-high property without specifying an enable GPIO
- Typos in the enabl GPIO property name (gpios instead of gpio)
- Mismatch between the enable-active-high property (or the lack thereof) and the enable GPIO flags

This patch series fixes those issues in all the DT sources after locating the errors using the following script.

------------------------------------------------------------------------------
#!/bin/sh

echo $1
cat $1 | awk '

BEGIN {
	open_drain = 0;
	active_high = 0;
	gpio = 0;
	flags = 0;
}
match($0, /([a-zA-Z0-9 at _-]*) {/, ary) {
	name = ary[1];
}
/compatible.*"regulator-fixed"/ {
	found = 1;
}
/enable-active-high/ {
	active_high = 1;
}
/gpio-open-drain/ {
	open_drain = 1;
}
match($0, /gpio += <.* ([^ ]*)>/, ary) {
	gpio = 1;
	flags = ary[1];
	if (flags == 0)
		flags = "GPIO_ACTIVE_HIGH";
}
/}/ {
	if (found) {
		if (gpio) {
			print "\t" name ": active high " active_high " " flags " open drain " open_drain;
			if ((active_high && flags == "GPIO_ACTIVE_LOW") ||
			    (!active_high && flags == "GPIO_ACTIVE_HIGH"))
				print "WARNING: enable-active-high and flags do not match"
		} else {
			if (active_high)
				print "WARNING: active high without GPIO"
			if (open_drain)
				print "WARNING: open drain without GPIO"
		}
	}

	gpio = 0;
	found = 0;
	active_high = 0;
	open_drain = 0;
	flags = 0;
}

'
------------------------------------------------------------------------------

Quoted 7 lines

All patches except for the ones touching omap3-beagle-xm and omap3-overo-base are untested as I lack test hardware.

As there's no dependency between the patches touching different source files the appropriate maintainers could take their share of the patches in their tree. Alternatively I could send a single pull request after collecting all acks but that might be more complex.

Quoted 5 lines

Nice clean-up. For omaps, there's an earlier patch posted by Javier Martinez Canillas <[email protected]> as "[PATCH] ARM: dts: Use defined GPIO constants in flags cell for OMAP2+ boards". Can you guys do some cross checking and let me know which combination I should appluy for omaps?

Since Laurent's changes for OMAP are part of a bigger series and my patch was only for OMAP, probably makes sense for you to pick his patches and I can re-spin mine on top of that.

BTW, I posted as a single patch since the changes were trivial but maybe that made handling these conflicts harder and I should split the changes instead, since I'll resend anyways.

What do you prefer? a patch per SoC family (i.e: OMAP{2,3,4,5}) or patch per board DTS?

Quoted 8 lines

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Best regards,

Signature · 3 lines
Javier Martinez Canillas
Open Source Group
Samsung Research America
#50 lore raw

Hi Javier,

Quoted 53 lines · On Tuesday 13 October 2015 00:19:20 Javier Martinez Canillas wrote:
Quoted 52 lines · Tony Lindgren
Quoted 51 lines · Laurent Pinchart

Hello,

While working on regulators, GPIOs and DT I noticed that many of our DT source files incorrectly describe fixed regulators. The common error patterns are

- Usage of the undefined (and never parsed) enable-active-low property
- Usage of the enable-active-high property without specifying an enable GPIO
- Typos in the enabl GPIO property name (gpios instead of gpio)
- Mismatch between the enable-active-high property (or the lack thereof) and the enable GPIO flags

This patch series fixes those issues in all the DT sources after locating the errors using the following script.

-------------------------------------------------------------------------
!/bin/sh

echo $1
cat $1 | awk '

BEGIN {
	open_drain = 0;
	active_high = 0;
	gpio = 0;
	flags = 0;
}
match($0, /([a-zA-Z0-9@_-]*) {/, ary) {
	name = ary[1];
}
/compatible.*"regulator-fixed"/ {
	found = 1;
}
/enable-active-high/ {
	active_high = 1;
}
/gpio-open-drain/ {
	open_drain = 1;
}
match($0, /gpio += <.* ([^ ]*)>/, ary) {
	gpio = 1;
	flags = ary[1];
	if (flags == 0)
		flags = "GPIO_ACTIVE_HIGH";
}
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
/}/ {
	if (found) {
		if (gpio) {
			print "\t" name ": active high " active_high " " flags " open 

drain "

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
			open_drain;
			if ((active_high && flags == "GPIO_ACTIVE_LOW") ||
			    (!active_high && flags == "GPIO_ACTIVE_HIGH"))
				print "WARNING: enable-active-high and flags do not 

match"

Quoted 35 lines
Quoted 25 lines
Quoted 17 lines
		} else {
			if (active_high)
				print "WARNING: active high without GPIO"
			if (open_drain)
				print "WARNING: open drain without GPIO"
		}
	}
	
	gpio = 0;
	found = 0;
	active_high = 0;
	open_drain = 0;
	flags = 0;
}

'
-------------------------------------------------------------------------

Quoted 7 lines

All patches except for the ones touching omap3-beagle-xm and omap3-overo-base are untested as I lack test hardware.

As there's no dependency between the patches touching different source files the appropriate maintainers could take their share of the patches in their tree. Alternatively I could send a single pull request after collecting all acks but that might be more complex.

Quoted 5 lines

Nice clean-up. For omaps, there's an earlier patch posted by Javier Martinez Canillas <[email protected]> as "[PATCH] ARM: dts:
Use defined GPIO constants in flags cell for OMAP2+ boards". Can you guys do some cross checking and let me know which combination I should appluy for omaps?

Since Laurent's changes for OMAP are part of a bigger series and my patch was only for OMAP, probably makes sense for you to pick his patches and I can re-spin mine on top of that.

Quoted 6 lines

BTW, I posted as a single patch since the changes were trivial but maybe that made handling these conflicts harder and I should split the changes instead, since I'll resend anyways.

What do you prefer? a patch per SoC family (i.e: OMAP{2,3,4,5}) or patch per board DTS?

My series will likely miss the next merge window as more discussion is needed. I'll thus respin the patches on top of yours, please proceed without caring about this.

Signature · 3 lines
Regards,

Laurent Pinchart
#51 lore raw
Quoted 105 lines · Laurent Pinchart

Hi Javier,

Quoted 53 lines · On Tuesday 13 October 2015 00:19:20 Javier Martinez Canillas wrote:
Quoted 52 lines · Tony Lindgren
Quoted 51 lines · Laurent Pinchart

Hello,

While working on regulators, GPIOs and DT I noticed that many of our DT source files incorrectly describe fixed regulators. The common error patterns are

- Usage of the undefined (and never parsed) enable-active-low property
- Usage of the enable-active-high property without specifying an enable GPIO
- Typos in the enabl GPIO property name (gpios instead of gpio)
- Mismatch between the enable-active-high property (or the lack thereof) and the enable GPIO flags

This patch series fixes those issues in all the DT sources after locating the errors using the following script.

-------------------------------------------------------------------------
!/bin/sh

echo $1
cat $1 | awk '

BEGIN {
	open_drain = 0;
	active_high = 0;
	gpio = 0;
	flags = 0;
}
match($0, /([a-zA-Z0-9 at _-]*) {/, ary) {
	name = ary[1];
}
/compatible.*"regulator-fixed"/ {
	found = 1;
}
/enable-active-high/ {
	active_high = 1;
}
/gpio-open-drain/ {
	open_drain = 1;
}
match($0, /gpio += <.* ([^ ]*)>/, ary) {
	gpio = 1;
	flags = ary[1];
	if (flags == 0)
		flags = "GPIO_ACTIVE_HIGH";
}
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
/}/ {
	if (found) {
		if (gpio) {
			print "\t" name ": active high " active_high " " flags " open 

drain "

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
			open_drain;
			if ((active_high && flags == "GPIO_ACTIVE_LOW") ||
			    (!active_high && flags == "GPIO_ACTIVE_HIGH"))
				print "WARNING: enable-active-high and flags do not 

match"

Quoted 39 lines
Quoted 25 lines
Quoted 17 lines
		} else {
			if (active_high)
				print "WARNING: active high without GPIO"
			if (open_drain)
				print "WARNING: open drain without GPIO"
		}
	}
	
	gpio = 0;
	found = 0;
	active_high = 0;
	open_drain = 0;
	flags = 0;
}

'
-------------------------------------------------------------------------

Quoted 7 lines

All patches except for the ones touching omap3-beagle-xm and omap3-overo-base are untested as I lack test hardware.

As there's no dependency between the patches touching different source files the appropriate maintainers could take their share of the patches in their tree. Alternatively I could send a single pull request after collecting all acks but that might be more complex.

Quoted 5 lines

Nice clean-up. For omaps, there's an earlier patch posted by Javier Martinez Canillas <[email protected]> as "[PATCH] ARM: dts:
Use defined GPIO constants in flags cell for OMAP2+ boards". Can you guys do some cross checking and let me know which combination I should appluy for omaps?

Since Laurent's changes for OMAP are part of a bigger series and my patch was only for OMAP, probably makes sense for you to pick his patches and I can re-spin mine on top of that.

BTW, I posted as a single patch since the changes were trivial but maybe that made handling these conflicts harder and I should split the changes instead, since I'll resend anyways.

Quoted 6 lines
Quoted 2 lines

What do you prefer? a patch per SoC family (i.e: OMAP{2,3,4,5}) or patch per board DTS?

My series will likely miss the next merge window as more discussion is needed. I'll thus respin the patches on top of yours, please proceed without caring about this.

OK applying Javier's patch into omap-for-v4.4/dt then.

Regards,

Tony

maintainer Reviewed-by on the v1 series
#52 lore raw
In reply to Laurent Pinchart
Quoted 5 lines · Laurent Pinchart

Hello,

While working on regulators, GPIOs and DT I noticed that many of our DT source files incorrectly describe fixed regulators. The common error patterns are

Quoted 8 lines

- Usage of the undefined (and never parsed) enable-active-low property
- Usage of the enable-active-high property without specifying an enable GPIO
- Typos in the enabl GPIO property name (gpios instead of gpio)
- Mismatch between the enable-active-high property (or the lack thereof) and the enable GPIO flags

This patch series fixes those issues in all the DT sources after locating the errors using the following script.

Nice. For the i.MX boards:

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Applied
#53 lore raw
In reply to Laurent Pinchart
Quoted 1 line · Laurent Pinchart

Laurent Pinchart (37):

...

Quoted 1 line

ARM: imx6sx-sdb: Fix typo in regulator enable GPIO property

...

Quoted 1 line

ARM: dts: imx6qdl-tx6: Fix regulator enable GPIO polarity

...

Quoted 5 lines

ARM: dts: imx23-evk: Fix regulator enable GPIO polarity
ARM: dts: imx23-stmp378x_devb: Fix regulator enable GPIO polarity
ARM: dts: imx25-pdk: Fix regulator enable GPIO polarity
ARM: dts: imx28-cfa10036: Fix regulator enable GPIO polarity
ARM: dts: imx28-evk: Fix regulator enable GPIO polarity

Quoted 8 lines

ARM: dts: imx28-m28cu3: Fix regulator enable GPIO polarity
ARM: dts: imx28-m28evk: Fix regulator enable GPIO polarity
ARM: dts: imx28-sps1: Fix regulator enable GPIO polarity
ARM: dts: imx28-tx28: Fix regulator enable GPIO polarity
ARM: dts: imx53-m53evk: Fix regulator enable GPIO polarity
ARM: dts: imx53-mba53: Fix regulator enable GPIO polarity
ARM: dts: imx53-tx53: Fix regulator enable GPIO polarity
ARM: dts: imx6q-dmo-edmqmx6: Fix regulator enable GPIO polarity

Applied these 15 patches, thanks.

#54 lore raw

Hi Shawn,

Quoted 26 lines · On Tuesday 13 October 2015 22:09:46 Shawn Guo wrote:
Quoted 1 line · Laurent Pinchart

Laurent Pinchart (37):

...

Quoted 1 line

ARM: imx6sx-sdb: Fix typo in regulator enable GPIO property

...

Quoted 1 line

ARM: dts: imx6qdl-tx6: Fix regulator enable GPIO polarity

...

Quoted 5 lines

ARM: dts: imx23-evk: Fix regulator enable GPIO polarity
ARM: dts: imx23-stmp378x_devb: Fix regulator enable GPIO polarity
ARM: dts: imx25-pdk: Fix regulator enable GPIO polarity
ARM: dts: imx28-cfa10036: Fix regulator enable GPIO polarity
ARM: dts: imx28-evk: Fix regulator enable GPIO polarity

Quoted 8 lines

ARM: dts: imx28-m28cu3: Fix regulator enable GPIO polarity
ARM: dts: imx28-m28evk: Fix regulator enable GPIO polarity
ARM: dts: imx28-sps1: Fix regulator enable GPIO polarity
ARM: dts: imx28-tx28: Fix regulator enable GPIO polarity
ARM: dts: imx53-m53evk: Fix regulator enable GPIO polarity
ARM: dts: imx53-mba53: Fix regulator enable GPIO polarity
ARM: dts: imx53-tx53: Fix regulator enable GPIO polarity
ARM: dts: imx6q-dmo-edmqmx6: Fix regulator enable GPIO polarity

Quoted 1 line

Applied these 15 patches, thanks.

There's ongoing discussions regarding whether this is the right thing to do. Please see http://www.spinics.net/lists/arm-kernel/msg451724.html. It's thus a bit early to apply the patches at this point I'm afraid.

Signature · 3 lines
Regards,

Laurent Pinchart
#55 lore raw
Quoted 29 lines · Laurent Pinchart

Hi Shawn,

Quoted 26 lines · On Tuesday 13 October 2015 22:09:46 Shawn Guo wrote:
Quoted 1 line · Laurent Pinchart

Laurent Pinchart (37):

...

Quoted 1 line

ARM: imx6sx-sdb: Fix typo in regulator enable GPIO property

...

Quoted 1 line

ARM: dts: imx6qdl-tx6: Fix regulator enable GPIO polarity

...

Quoted 13 lines

ARM: dts: imx23-evk: Fix regulator enable GPIO polarity
ARM: dts: imx23-stmp378x_devb: Fix regulator enable GPIO polarity
ARM: dts: imx25-pdk: Fix regulator enable GPIO polarity
ARM: dts: imx28-cfa10036: Fix regulator enable GPIO polarity
ARM: dts: imx28-evk: Fix regulator enable GPIO polarity
ARM: dts: imx28-m28cu3: Fix regulator enable GPIO polarity
ARM: dts: imx28-m28evk: Fix regulator enable GPIO polarity
ARM: dts: imx28-sps1: Fix regulator enable GPIO polarity
ARM: dts: imx28-tx28: Fix regulator enable GPIO polarity
ARM: dts: imx53-m53evk: Fix regulator enable GPIO polarity
ARM: dts: imx53-mba53: Fix regulator enable GPIO polarity
ARM: dts: imx53-tx53: Fix regulator enable GPIO polarity
ARM: dts: imx6q-dmo-edmqmx6: Fix regulator enable GPIO polarity

Quoted 5 lines
Quoted 1 line

Applied these 15 patches, thanks.

There's ongoing discussions regarding whether this is the right thing to do. Please see http://www.spinics.net/lists/arm-kernel/msg451724.html. It's thus a bit early to apply the patches at this point I'm afraid.

Okay, dropped them except the first one which fixes a typo for imx6sx-sdb.

Shawn