Skip to contentSkip to conversation

ls2080a/dts: Add little endian property for GPIO IP block

PATCH Superseded Liu Gang 5 participants 7 messages Nov 4, 2015 – Nov 24, 2015

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Liu Gang

author
#0 lore raw

PATCH 1/2 ls2080a/dts: Add little endian property for GPIO IP block

The GPIO block for ls2080a platform has little endian registers, the GPIO driver needs this property to read/write registers by right interface.

Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +3 -0
index f2455c5..c836dab 100644
@@ -10,6 +10,9 @@ Required properties:
   the second cell is used to specify the gpio polarity:
       0 = active high
       1 = active low
+- little-endian : Should be set if the GPIO has little endian
+		  registers. No the property means the GPIO
+		  registers are big endian mode.
 
 Example:
 
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi +4 -0
index f3c59f9..41bb8c1 100644
@@ -277,6 +277,7 @@
 			reg = <0x0 0x2300000 0x0 0x10000>;
 			interrupts = <0 36 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -287,6 +288,7 @@
 			reg = <0x0 0x2310000 0x0 0x10000>;
 			interrupts = <0 36 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -297,6 +299,7 @@
 			reg = <0x0 0x2320000 0x0 0x10000>;
 			interrupts = <0 37 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -307,6 +310,7 @@
 			reg = <0x0 0x2330000 0x0 0x10000>;
 			interrupts = <0 37 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
Signature · 1 line
2.1.0.27.g96db324
author
#1 lore raw

PATCH 2/2 drivers/gpio: Port gpio driver to layerscape platform

Layerscape has the same ip block/controller as GPIO on powerpc platform(MPC8XXX).

So use portable i/o accessors, as in_be32/out_be32 accessors are Power architecture specific whereas ioread32/iowrite32 and ioread32be/iowrite32be are available in other architectures.

Layerscape GPIO controller's registers may be big or little endian, so the code needs to get the endian property from DTB, then make additional functions to fit right register read/write operations.

Currently the code can support ls2080a GPIO with little endian registers. And it can also work well on other layerscape platform with big endian GPIO registers.

drivers/gpio/Kconfig +3 -3
index 8949b3f..c3ca283 100644
@@ -290,12 +290,12 @@ config GPIO_MPC5200
 	depends on PPC_MPC52xx
 
 config GPIO_MPC8XXX
-	bool "MPC512x/MPC8xxx GPIO support"
+	bool "MPC512x/MPC8xxx/QorIQ GPIO support"
 	depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
-		   FSL_SOC_BOOKE || PPC_86xx
+		   FSL_SOC_BOOKE || PPC_86xx || ARCH_LAYERSCAPE
 	help
 	  Say Y here if you're going to use hardware that connects to the
-	  MPC512x/831x/834x/837x/8572/8610 GPIOs.
+	  MPC512x/831x/834x/837x/8572/8610/QorIQ GPIOs.
 
 config GPIO_MSM_V2
 	tristate "Qualcomm MSM GPIO v2"
drivers/gpio/gpio-mpc8xxx.c +64 -23

218 lines not on this page · raw message

Signature · 1 line
2.1.0.27.g96db324
#2 lore raw

PATCH 2/2 drivers/gpio: Port gpio driver to layerscape platform

Quoted 29 lines · On Wednesday 04 November 2015 14:48:24 Liu Gang wrote:

Layerscape has the same ip block/controller as GPIO on powerpc platform(MPC8XXX).

So use portable i/o accessors, as in_be32/out_be32 accessors are Power architecture specific whereas ioread32/iowrite32 and ioread32be/iowrite32be are available in other architectures.

Layerscape GPIO controller's registers may be big or little endian, so the code needs to get the endian property from DTB, then make additional functions to fit right register read/write operations.

Currently the code can support ls2080a GPIO with little endian registers. And it can also work well on other layerscape platform with big endian GPIO registers.

drivers/gpio/Kconfig +0 -0
index 8949b3f..c3ca283 100644
@@ -290,12 +290,12 @@ config GPIO_MPC5200
 	depends on PPC_MPC52xx
 

drivers/gpio/Kconfig config GPIO_MPC5200 · line 298

Quoted 10 lines
drivers/gpio/Kconfig +3 -3
 config GPIO_MPC8XXX
-	bool "MPC512x/MPC8xxx GPIO support"
+	bool "MPC512x/MPC8xxx/QorIQ GPIO support"
 	depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
-		   FSL_SOC_BOOKE || PPC_86xx
+		   FSL_SOC_BOOKE || PPC_86xx || ARCH_LAYERSCAPE
 	help
 	  Say Y here if you're going to use hardware that connects to the
-	  MPC512x/831x/834x/837x/8572/8610 GPIOs.
+	  MPC512x/831x/834x/837x/8572/8610/QorIQ GPIOs.

It would be nice to also add '|| COMPILE_TEST' here and ensure that it also builds on x86 with that set, to get better coverage from the automated build testing infrastructure.

Quoted 10 lines
drivers/gpio/Kconfig +10 -0
+static bool gpio_little_endian;
+static inline u32 gpio_in32(void __iomem *addr)
+{
+	u32 val;
+
+	if (gpio_little_endian)
+		val = ioread32(addr);
+	else
+		val = ioread32be(addr);
+

drivers/gpio/Kconfig config GPIO_MPC5200

Quoted 10 lines
drivers/gpio/Kconfig +10 -0
+	return val;
+}
+
+static inline void gpio_out32(u32 val, void __iomem *addr)
+{
+	if (gpio_little_endian)
+		iowrite32(val, addr);
+	else
+		iowrite32be(val, addr);
+}

I guess this is fixed per architecture, so you could also do this as

static inline void gpio_out32(u32 val, void __iomem *addr)

{
	if (IS_ENABLED(CONFIG_ARM))
		iowrite32(val, addr);
	else if (IS_ENABLED(CONFIG_PPC)
		iowrite32be(val, addr);
	else
		BUG();
}

and then check that the DT flag for little-endian matches the architecture specific default.

Your version is more generic of course, while the one I show here is a little more efficient and avoids the global variable. Your choice.

Arnd

#3 lore raw

PATCH 2/2 drivers/gpio: Port gpio driver to layerscape platform

Quoted 26 lines · Arnd Bergmann
Quoted 11 lines · On Wednesday 04 November 2015 14:48:24 Liu Gang wrote:
drivers/gpio/gpio-mpc8xxx.c +10 -0
+static bool gpio_little_endian;
+static inline u32 gpio_in32(void __iomem *addr)
+{
+   u32 val;
+
+   if (gpio_little_endian)
+           val = ioread32(addr);
+   else
+           val = ioread32be(addr);
+

drivers/gpio/gpio-mpc8xxx.c struct mpc8xxx_gpio_chip { · line 67

Quoted 10 lines
drivers/gpio/gpio-mpc8xxx.c +10 -0
+   return val;
+}
+
+static inline void gpio_out32(u32 val, void __iomem *addr)
+{
+   if (gpio_little_endian)
+           iowrite32(val, addr);
+   else
+           iowrite32be(val, addr);
+}

I guess this is fixed per architecture, so you could also do this as

static inline void gpio_out32(u32 val, void __iomem *addr)

Quoted 8 lines
{
      if (IS_ENABLED(CONFIG_ARM))
              iowrite32(val, addr);
      else if (IS_ENABLED(CONFIG_PPC)
              iowrite32be(val, addr);
      else
              BUG();
}

Unfortunately that guess is wrong. Some of our ARM chips have big-endian I/O and some have little-endian I/O.

-Scott

#4 lore raw

PATCH 2/2 drivers/gpio: Port gpio driver to layerscape platform

Quoted 12 lines · On Wednesday 04 November 2015 03:17:38 Scott Wood wrote:
Quoted 11 lines

I guess this is fixed per architecture, so you could also do this as

static inline void gpio_out32(u32 val, void __iomem *addr)

{
      if (IS_ENABLED(CONFIG_ARM))
              iowrite32(val, addr);
      else if (IS_ENABLED(CONFIG_PPC))
              iowrite32be(val, addr);
      else
              BUG();
}
Quoted 2 lines

Unfortunately that guess is wrong. Some of our ARM chips have big-endian I/O and some have little-endian I/O.

Ok, I see. In that case, using run-time detection everywhere is probably best.

Arnd

maintainer
#5 lore raw

PATCH 1/2 ls2080a/dts: Add little endian property for GPIO IP block

In reply to Liu Gang
Quoted 5 lines · Liu Gang

The GPIO block for ls2080a platform has little endian registers, the GPIO driver needs this property to read/write registers by right interface.

Quoted 57 lines
Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +3 -0
index f2455c5..c836dab 100644
@@ -10,6 +10,9 @@ Required properties:
   the second cell is used to specify the gpio polarity:
       0 = active high
       1 = active low
+- little-endian : Should be set if the GPIO has little endian
+		  registers. No the property means the GPIO
+		  registers are big endian mode.
 
 Example:
 
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi +4 -0
index f3c59f9..41bb8c1 100644
@@ -277,6 +277,7 @@
 			reg = <0x0 0x2300000 0x0 0x10000>;
 			interrupts = <0 36 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -287,6 +288,7 @@
 			reg = <0x0 0x2310000 0x0 0x10000>;
 			interrupts = <0 36 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -297,6 +299,7 @@
 			reg = <0x0 0x2320000 0x0 0x10000>;
 			interrupts = <0 37 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -307,6 +310,7 @@
 			reg = <0x0 0x2330000 0x0 0x10000>;
 			interrupts = <0 37 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
Signature · 6 lines
2.1.0.27.g96db324

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#6 lore raw

PATCH 1/2 ls2080a/dts: Add little endian property for GPIO IP block

In reply to Liu Gang
Quoted 5 lines · Liu Gang

The GPIO block for ls2080a platform has little endian registers, the GPIO driver needs this property to read/write registers by right interface.

Please do not mix binding doc together with dts changes in one patch.

Shawn

Quoted 60 lines
Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +3 -0
index f2455c5..c836dab 100644
@@ -10,6 +10,9 @@ Required properties:
   the second cell is used to specify the gpio polarity:
       0 = active high
       1 = active low
+- little-endian : Should be set if the GPIO has little endian
+		  registers. No the property means the GPIO
+		  registers are big endian mode.
 
 Example:
 
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi +4 -0
index f3c59f9..41bb8c1 100644
@@ -277,6 +277,7 @@
 			reg = <0x0 0x2300000 0x0 0x10000>;
 			interrupts = <0 36 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -287,6 +288,7 @@
 			reg = <0x0 0x2310000 0x0 0x10000>;
 			interrupts = <0 36 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -297,6 +299,7 @@
 			reg = <0x0 0x2320000 0x0 0x10000>;
 			interrupts = <0 37 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -307,6 +310,7 @@
 			reg = <0x0 0x2330000 0x0 0x10000>;
 			interrupts = <0 37 0x4>; /* Level high type */
 			gpio-controller;
+			little-endian;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
Signature · 7 lines
2.1.0.27.g96db324


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