Skip to contentSkip to conversation

gpio-omap: ensure a stable gpio numbering

PATCH New Uwe Kleine-König 4 participants 6 messages Jun 14, 2016 – Jun 23, 2016

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

v2 cover gpio-omap: ensure a stable gpio numbering

Hello,

this series' first patch is v2 of the patch I sent earlier. This now also compiles without CONFIG_OF_GPIO.

The second patch is new and adds the respective aliases to all device trees that make use of the gpio-omap driver.

Best regards
Uwe

Uwe Kleine-K?nig (2):
gpio: omap: make gpio numbering deterministical by using of aliases
ARM: dts: add aliases for ti,omap[234]-gpio devices

 arch/arm/boot/dts/am33xx.dtsi   |  4 ++++
 arch/arm/boot/dts/am4372.dtsi   |  6 ++++++
 arch/arm/boot/dts/dm814x.dtsi   |  2 ++
 arch/arm/boot/dts/dm816x.dtsi   |  2 ++
 arch/arm/boot/dts/dra7.dtsi     |  8 ++++++++
 arch/arm/boot/dts/omap2420.dtsi |  7 +++++++
 arch/arm/boot/dts/omap2430.dtsi |  8 ++++++++
 arch/arm/boot/dts/omap3.dtsi    |  6 ++++++
 arch/arm/boot/dts/omap4.dtsi    |  6 ++++++
 arch/arm/boot/dts/omap5.dtsi    |  8 ++++++++
 drivers/gpio/gpio-omap.c        | 19 ++++++++++++++++++-
 11 files changed, 75 insertions(+), 1 deletion(-)
Signature · 1 line
2.8.1
#1 lore raw

PATCH v2 1/2 gpio: omap: make gpio numbering deterministical by using of aliases

Traditionally the n-th gpio device probed by the omap gpio driver got the gpio number range [n*32 .. n*32+31].
When order of the devices probed by the driver changes (which can happen already now when some devices have a pinctrl and so the first probe attempt returns -ENODEV) the numbering changes.

To ensure a deterministical numbering use of_alias_get_id to determine the base for a given device. If no respective alias exists fall back to the traditional numbering.

For the unusual case where only a part of the gpio devices have a matching alias some of them might fail to probe. But if none of them has an alias or all, there is no conflict which should be good enough to maintain backward compatibility.

---
 drivers/gpio/gpio-omap.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)
drivers/gpio/gpio-omap.c +18 -1
index b98ede78c9d8..4d91792e7266 100644
@@ -1053,9 +1053,26 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
 			bank->chip.parent = &omap_mpuio_device.dev;
 		bank->chip.base = OMAP_MPUIO(0);
 	} else {
+		int gpio_alias_id __maybe_unused;
+
 		bank->chip.label = "gpio";
-		bank->chip.base = gpio;
+
+#if defined(CONFIG_OF_GPIO)
+		/*
+		 * Traditionally the base is given out in first-come-first-serve
+		 * order.  This might shuffle the numbering of gpios if the
+		 * probe order changes.  So make the base deterministical if the
+		 * device tree specifies alias ids.
+		 */
+		gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio");
+		if (gpio_alias_id >= 0)
+			bank->chip.base = bank->width * gpio_alias_id;
+		else
+#endif
+			bank->chip.base = gpio;
 	}
+
+
 	bank->chip.ngpio = bank->width;
 
 	ret = gpiochip_add_data(&bank->chip, bank);
Signature · 1 line
2.8.1
#2 lore raw

PATCH v2 1/2 gpio: omap: make gpio numbering deterministical by using of aliases

Quoted 6 lines · Uwe Kleine-K?nig

Traditionally the n-th gpio device probed by the omap gpio driver got the gpio number range [n*32 .. n*32+31].
When order of the devices probed by the driver changes (which can happen already now when some devices have a pinctrl and so the first probe attempt returns -ENODEV) the numbering changes.

Quoted 8 lines

To ensure a deterministical numbering use of_alias_get_id to determine the base for a given device. If no respective alias exists fall back to the traditional numbering.

For the unusual case where only a part of the gpio devices have a matching alias some of them might fail to probe. But if none of them has an alias or all, there is no conflict which should be good enough to maintain backward compatibility.

I think doing this is a good idea to prevent nasty suprises when upgrading a kernel on some device:

#3 lore raw

PATCH v2 1/2 gpio: omap: make gpio numbering deterministical by using of aliases

In reply to Uwe Kleine-König
Quoted 43 lines · Uwe Kleine-K?nig

Traditionally the n-th gpio device probed by the omap gpio driver got the gpio number range [n*32 .. n*32+31].
When order of the devices probed by the driver changes (which can happen already now when some devices have a pinctrl and so the first probe attempt returns -ENODEV) the numbering changes.

To ensure a deterministical numbering use of_alias_get_id to determine the base for a given device. If no respective alias exists fall back to the traditional numbering.

For the unusual case where only a part of the gpio devices have a matching alias some of them might fail to probe. But if none of them has an alias or all, there is no conflict which should be good enough to maintain backward compatibility.

---
  drivers/gpio/gpio-omap.c | 19 ++++++++++++++++++-
  1 file changed, 18 insertions(+), 1 deletion(-)
drivers/gpio/gpio-omap.c +12 -1
index b98ede78c9d8..4d91792e7266 100644
@@ -1053,9 +1053,26 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
  			bank->chip.parent = &omap_mpuio_device.dev;
  		bank->chip.base = OMAP_MPUIO(0);
  	} else {
+		int gpio_alias_id __maybe_unused;
+
  		bank->chip.label = "gpio";
-		bank->chip.base = gpio;
+
+#if defined(CONFIG_OF_GPIO)
+		/*
+		 * Traditionally the base is given out in first-come-first-serve
+		 * order.  This might shuffle the numbering of gpios if the
+		 * probe order changes.  So make the base deterministical if the
+		 * device tree specifies alias ids.
+		 */
+		gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio");
+		if (gpio_alias_id >= 0)

drivers/gpio/gpio-omap.c static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) · line 1078

Quoted 10 lines
drivers/gpio/gpio-omap.c +6 -0
+			bank->chip.base = bank->width * gpio_alias_id;
+		else
+#endif
+			bank->chip.base = gpio;
  	}
+
+
  	bank->chip.ngpio = bank->width;
 
  	ret = gpiochip_add_data(&bank->chip, bank);
Signature · 2 lines
regards,
-grygorii
#4 lore raw

PATCH v2 1/2 gpio: omap: make gpio numbering deterministical by using of aliases

In reply to Uwe Kleine-König

Looping in DT people.

Please send subsequent patches also to devicetree at vger.kernel.org

Quoted 39 lines · Uwe Kleine-K?nig

Traditionally the n-th gpio device probed by the omap gpio driver got the gpio number range [n*32 .. n*32+31].
When order of the devices probed by the driver changes (which can happen already now when some devices have a pinctrl and so the first probe attempt returns -ENODEV) the numbering changes.

To ensure a deterministical numbering use of_alias_get_id to determine the base for a given device. If no respective alias exists fall back to the traditional numbering.

For the unusual case where only a part of the gpio devices have a matching alias some of them might fail to probe. But if none of them has an alias or all, there is no conflict which should be good enough to maintain backward compatibility.

---
 drivers/gpio/gpio-omap.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)
drivers/gpio/gpio-omap.c +9 -1
index b98ede78c9d8..4d91792e7266 100644
@@ -1053,9 +1053,26 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
                        bank->chip.parent = &omap_mpuio_device.dev;
                bank->chip.base = OMAP_MPUIO(0);
        } else {
+               int gpio_alias_id __maybe_unused;
+
                bank->chip.label = "gpio";
-               bank->chip.base = gpio;
+
+#if defined(CONFIG_OF_GPIO)
+               /*
+                * Traditionally the base is given out in first-come-first-serve
+                * order.  This might shuffle the numbering of gpios if the
+                * probe order changes.  So make the base deterministical if the
+                * device tree specifies alias ids.

drivers/gpio/gpio-omap.c static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) · line 1075

Quoted 10 lines
drivers/gpio/gpio-omap.c +9 -0
+                */
+               gpio_alias_id = of_alias_get_id(bank->chip.of_node, "gpio");
+               if (gpio_alias_id >= 0)
+                       bank->chip.base = bank->width * gpio_alias_id;
+               else
+#endif
+                       bank->chip.base = gpio;
        }
+
+

I need:

- A hunk of the patch to hit
Documentation/devicetree/bindings/gpio/gpio.txt
and describe the alias usage there

- an ACK from a DT bindings maintainer for this.

Yours,
Linus Walleij

#5 lore raw

PATCH v2 2/2 ARM: dts: add aliases for ti,omap[234]-gpio devices

In reply to Uwe Kleine-König

This together with the corresponding change to the gpio-omap driver ensures that the gpio numbering is independant of the probing order.

---
 arch/arm/boot/dts/am33xx.dtsi   | 4 ++++
 arch/arm/boot/dts/am4372.dtsi   | 6 ++++++
 arch/arm/boot/dts/dm814x.dtsi   | 2 ++
 arch/arm/boot/dts/dm816x.dtsi   | 2 ++
 arch/arm/boot/dts/dra7.dtsi     | 8 ++++++++
 arch/arm/boot/dts/omap2420.dtsi | 7 +++++++
 arch/arm/boot/dts/omap2430.dtsi | 8 ++++++++
 arch/arm/boot/dts/omap3.dtsi    | 6 ++++++
 arch/arm/boot/dts/omap4.dtsi    | 6 ++++++
 arch/arm/boot/dts/omap5.dtsi    | 8 ++++++++
 10 files changed, 57 insertions(+)
arch/arm/boot/dts/am33xx.dtsi +4 -0
index 52be48bbd2dd..fbfc17ab56d8 100644
@@ -35,6 +35,10 @@
 		phy1 = &usb1_phy;
 		ethernet0 = &cpsw_emac0;
 		ethernet1 = &cpsw_emac1;
+		gpio0 = &gpio0;
+		gpio1 = &gpio1;
+		gpio2 = &gpio2;
+		gpio3 = &gpio3;
 	};
 
 	cpus {
arch/arm/boot/dts/am4372.dtsi +6 -0
index 12fcde4d4d2e..0f5a915ba94b 100644
@@ -31,6 +31,12 @@
 		ethernet0 = &cpsw_emac0;
 		ethernet1 = &cpsw_emac1;
 		spi0 = &qspi;
+		gpio0 = &gpio0;
+		gpio1 = &gpio1;
+		gpio2 = &gpio2;
+		gpio3 = &gpio3;
+		gpio4 = &gpio4;
+		gpio5 = &gpio5;
 	};
 
 	cpus {
arch/arm/boot/dts/dm814x.dtsi +2 -0
index d4537dc61497..d5a49ad31d50 100644
@@ -25,6 +25,8 @@
 		usb1 = &usb1;
 		phy0 = &usb0_phy;
 		phy1 = &usb1_phy;
+		gpio0 = &gpio1;
+		gpio1 = &gpio2;
 	};
 
 	cpus {
arch/arm/boot/dts/dm816x.dtsi +2 -0
index 44e39c743b53..500d9091701e 100644
@@ -21,6 +21,8 @@
 		serial2 = &uart3;
 		ethernet0 = &eth0;
 		ethernet1 = &eth1;
+		gpio0 = &gpio1;
+		gpio1 = &gpio2;
 	};
 
 	cpus {
arch/arm/boot/dts/dra7.dtsi +8 -0
index e0074014385a..4d6e2309a742 100644
@@ -42,6 +42,14 @@
 		d_can0 = &dcan1;
 		d_can1 = &dcan2;
 		spi0 = &qspi;
+		gpio0 = &gpio1;
+		gpio1 = &gpio2;
+		gpio2 = &gpio3;
+		gpio3 = &gpio4;
+		gpio4 = &gpio5;
+		gpio5 = &gpio6;
+		gpio6 = &gpio7;
+		gpio7 = &gpio8;
 	};
 
 	timer {
arch/arm/boot/dts/omap2420.dtsi +7 -0
index fb712b9aa874..933e4415d3d2 100644
@@ -13,6 +13,13 @@
 / {
 	compatible = "ti,omap2420", "ti,omap2";
 
+	aliases {
+		gpio0 = &gpio1;
+		gpio1 = &gpio2;
+		gpio2 = &gpio3;
+		gpio3 = &gpio4;
+	};
+
 	ocp {
 		l4: l4 at 48000000 {
 			compatible = "ti,omap2-l4", "simple-bus";
arch/arm/boot/dts/omap2430.dtsi +8 -0
index 455aaea407dd..8c0e5555a550 100644
@@ -13,6 +13,14 @@
 / {
 	compatible = "ti,omap2430", "ti,omap2";
 
+	aliases {
+		gpio0 = &gpio1;
+		gpio1 = &gpio2;
+		gpio2 = &gpio3;
+		gpio3 = &gpio4;
+		gpio4 = &gpio5;
+	};
+
 	ocp {
 		l4_wkup: l4_wkup at 49000000 {
 			compatible = "ti,omap2-l4-wkup", "simple-bus";
arch/arm/boot/dts/omap3.dtsi +6 -0
index 9fbda38528dc..2676c34c5830 100644
@@ -25,6 +25,12 @@
 		serial0 = &uart1;
 		serial1 = &uart2;
 		serial2 = &uart3;
+		gpio0 = &gpio1;
+		gpio1 = &gpio2;
+		gpio2 = &gpio3;
+		gpio3 = &gpio4;
+		gpio4 = &gpio5;
+		gpio5 = &gpio6;
 	};
 
 	cpus {
arch/arm/boot/dts/omap4.dtsi +6 -0
index 3fdc51cd0fad..367a2fb04829 100644
@@ -25,6 +25,12 @@
 		serial1 = &uart2;
 		serial2 = &uart3;
 		serial3 = &uart4;
+		gpio0 = &gpio1;
+		gpio1 = &gpio2;
+		gpio2 = &gpio3;
+		gpio3 = &gpio4;
+		gpio4 = &gpio5;
+		gpio5 = &gpio6;
 	};
 
 	cpus {
arch/arm/boot/dts/omap5.dtsi +8 -0
index 84c10195e79b..017edd442390 100644
@@ -32,6 +32,14 @@
 		serial3 = &uart4;
 		serial4 = &uart5;
 		serial5 = &uart6;
+		gpio0 = &gpio1;
+		gpio1 = &gpio2;
+		gpio2 = &gpio3;
+		gpio3 = &gpio4;
+		gpio4 = &gpio5;
+		gpio5 = &gpio6;
+		gpio6 = &gpio7;
+		gpio7 = &gpio8;
 	};
 
 	cpus {
Signature · 1 line
2.8.1