drivers/gpio: Port gpio driver to layerscape platform
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 2/2 drivers/gpio: Port gpio driver to layerscape platform
Quoted 14 lines · Liu Gang
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.
Quoted 7 lines
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.
Signed-off-by Liu Gang <[email protected]>Signed-off-by Shaveta Leekha <[email protected]>
This doesn't seem good. You are starting to duplicate stuff that is already available inside the gpio-generic.c driver.
Why can't this driver just select GPIO_GENERIC in Kconfig and use the common code for handling
the endianness accessors?
bgpio_init() takes a flag BGPIOF_BIG_ENDIAN_BYTE_ORDER
to make all accesses big endian, so using the generic GPIO library would save you a lot of code.
Just look at other drivers for those selecting GPIO_GENERIC, including <linux/basic_mmio_gpio.h> and calling bgpio_init(). You might want to keep some local custom BE code around the IRQ parts.
But I would do two patches:
- One that switches MPC8xxx to using GENERIC_GPIO
- One that adds BE support using that infrastructure
It will result in a lot less code. I think.
Yours,
Linus Walleij
PATCH 1/2 ls2080a/dts: Add little endian property for GPIO IP block
In reply to a message not in the archiveQuoted 11 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.
Signed-off-by Liu Gang <[email protected]>Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +0
-0
Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt Required properties: · line 15
Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +3
-0@@ -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.
That is a very generic binding and I would like the devicetree maintainers to say something about this.
I would be OK if this is specified for *all* gpiochips in Documentation/devicetree/bindings/gpio/gpio.txt or even higher up in the desriptions.
Just for Freescale seems a bit too local.
Yours,
Linus Walleij
PATCH 1/2 ls2080a/dts: Add little endian property for GPIO IP block
Quoted 23 lines · Linus Walleij
Quoted 11 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.
Signed-off-by Liu Gang <[email protected]>Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +0
-0Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt Required properties: · line 15
Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +3
-0@@ -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.That is a very generic binding and I would like the devicetree maintainers to say something about this.
Quoted 5 lines
I would be OK if this is specified for *all* gpiochips in
Documentation/devicetree/bindings/gpio/gpio.txtor even higher up in the desriptions.Just for Freescale seems a bit too local.
There is already a generic definition at Documentation/devicetree/bindings/common-properties.txt. But it will be special for Freescale controller to say that the default is big-endian for backward compatibility.
Regards,
Leo
PATCH 1/2 ls2080a/dts: Add little endian property for GPIO IP block
Quoted 30 lines · Li Yang
Quoted 28 lines · Linus Walleij
Quoted 12 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.
Signed-off-by Liu Gang <[email protected]>Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +0
-0Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt Required properties: · line 15
Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +3
-0@@ -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.That is a very generic binding and I would like the devicetree maintainers to say something about this.
I would be OK if this is specified for *all* gpiochips in
Documentation/devicetree/bindings/gpio/gpio.txtor even higher up in the desriptions.
Quoted 6 lines
Quoted 1 line
Just for Freescale seems a bit too local.
There is already a generic definition at
Documentation/devicetree/bindings/common-properties.txt. But it will be special for Freescale controller to say that the default is big-endian for backward compatibility.
Also, common-properties.txt says "if a binding supports these properties" so this patch serves to indicate that this binding does support the little-endian property.
-Scott
PATCH 1/2 ls2080a/dts: Add little endian property for GPIO IP block
In reply to Li YangQuoted 28 lines · Li Yang
Quoted 27 lines · Linus Walleij
Quoted 11 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.
Signed-off-by Liu Gang <[email protected]>Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +0
-0Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt Required properties: · line 15
Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt +3
-0@@ -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.That is a very generic binding and I would like the devicetree maintainers to say something about this.
I would be OK if this is specified for *all* gpiochips in
Documentation/devicetree/bindings/gpio/gpio.txtor even higher up in the desriptions.
Quoted 6 lines
Quoted 1 line
Just for Freescale seems a bit too local.
There is already a generic definition at
Documentation/devicetree/bindings/common-properties.txt. But it will be special for Freescale controller to say that the default is big-endian for backward compatibility.
OK! Why not reference that just like you reference gpio.txt? It fooled me so it will fool others.
Yours,
Linus Walleij