gpio: Add APM X-Gene SoC platform GPIO driver
Subsystems (2)
v5 cover gpio: Add APM X-Gene SoC platform GPIO driver
This patch add the GPIO controller in the APM X-Gene platform. The GPIO controller pins are muxed with the generic flash controller pins on the system.
V5 Change:
1. Remove gpio->dev
2. Remove not needed headers.
3. Change to use latest driver.h for gpio.
4. Set chip base to -1
5. Put back label as it is needed sys interface.
V4 Change:
1. Add remove function for module
2. Add set function to dir out function.
3. remove sets to np and label in probe.
V3 Change:
1. Change code to use single entry to describe gpio node, hardcode to use bank stride to target gpios.
2. Fix up dts and doc accordingly
V2 Change:
1. Address concerns from maintainer, split to multiple device node
2. Add pm code to restore set register.
3. fix up dts and documentation accordingly.
I did not incorporate pin mux functionality at this time, move this to bootloader for now. Will submit pinctrl driver seperately.
Feng Kan (3):
gpio: Add APM X-Gene SoC GPIO controller support
Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding arm64:dts: Add APM X-Gene SoC GPIO controller DTS entries
.../devicetree/bindings/gpio/gpio-xgene.txt | 37 ++++ arch/arm64/boot/dts/apm-storm.dtsi | 21 ++ drivers/gpio/Kconfig | 9 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-xgene.c | 231 +++++++++++++++++++++ 5 files changed, 299 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xgene.txt create mode 100644 drivers/gpio/gpio-xgene.c
Signature · 1 line
1.9.1
PATCH v5 1/3 gpio: Add APM X-Gene SoC GPIO controller support
Add APM X-Gene SoC gpio controller driver.
--- drivers/gpio/Kconfig | 9 ++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-xgene.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 260 insertions(+) create mode 100644 drivers/gpio/gpio-xgene.c
drivers/gpio/Kconfig +9 -0
@@ -334,6 +334,15 @@ config GPIO_TZ1090_PDC help Say yes here to support Toumaz Xenif TZ1090 PDC GPIOs. +config GPIO_XGENE + bool "APM X-Gene GPIO controller support" + depends on ARM64 && OF_GPIO + help + This driver is to support the GPIO block within the APM X-Gene SoC + platform's generic flash controller. The GPIO pins are muxed with + the generic flash controller's address and data pins. Say yes + here to enable the GFC GPIO functionality. + config GPIO_XILINX bool "Xilinx GPIO support" depends on PPC_OF || MICROBLAZE || ARCH_ZYNQ
drivers/gpio/Makefile +1 -0
@@ -98,6 +98,7 @@ obj-$(CONFIG_GPIO_VX855) += gpio-vx855.o obj-$(CONFIG_GPIO_WM831X) += gpio-wm831x.o obj-$(CONFIG_GPIO_WM8350) += gpio-wm8350.o obj-$(CONFIG_GPIO_WM8994) += gpio-wm8994.o +obj-$(CONFIG_GPIO_XGENE) += gpio-xgene.o obj-$(CONFIG_GPIO_XILINX) += gpio-xilinx.o obj-$(CONFIG_GPIO_XTENSA) += gpio-xtensa.o obj-$(CONFIG_GPIO_ZEVIO) += gpio-zevio.o
drivers/gpio/gpio-xgene.c new file +250 -0
250 lines not on this page · raw message
Signature · 1 line
1.9.1
PATCH v5 1/3 gpio: Add APM X-Gene SoC GPIO controller support
Quoted 3 lines · Feng Kan
Add APM X-Gene SoC gpio controller driver.
Signed-off-by Feng Kan <[email protected]>
Patch applied with Alexandre's review tag, good job!
Yours,
Linus Walleij
PATCH v5 1/3 gpio: Add APM X-Gene SoC GPIO controller support
Quoted 7 lines · Linus Walleij
Quoted 3 lines · Feng Kan
Add APM X-Gene SoC gpio controller driver.
Signed-off-by Feng Kan <[email protected]>Patch applied with Alexandre's review tag, good job!
Ah wait, Mark still has comments on the DT bindings so holding back the set until that has been resolved.
Yours,
Linus Walleij
PATCH v5 2/3 Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding
In reply to Feng KanDocumentation for APM X-Gene SoC GPIO controller DTS binding.
--- .../devicetree/bindings/gpio/gpio-xgene.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xgene.txt
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +20 -0
@@ -0,0 +1,20 @@ +APM X-Gene SoC GPIO controller bindings + +This is a gpio controller that is part of the flash controller. +This gpio controller controls a total of 48 gpios. + +Required properties: +- compatible: "apm,xgene-gpio" for X-Gene GPIO controller +- reg: Physical base address and size of the controller's registers +- #gpio-cells: Should be two. + - first cell is the pin number + - second cell is used to specify optional parameters (unused) +- gpio-controller: Marks the device node as a GPIO controller. + +Example: + gpio0: gpio0 at 1701c000 { + compatible = "apm,xgene-gpio"; + reg = <0x0 0x1701c000 0x0 0x40>; + gpio-controller; + #gpio-cells = <2>; + };
Signature · 1 line
1.9.1
PATCH v5 2/3 Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding
Quoted 15 lines · Feng Kan
Documentation for APM X-Gene SoC GPIO controller DTS binding.
Signed-off-by Feng Kan <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>--- .../devicetree/bindings/gpio/gpio-xgene.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xgene.txtDocumentation/devicetree/bindings/gpio/gpio-xgene.txt new file +0
-0
Documentation/devicetree/bindings/gpio/gpio-xgene.txt line 8
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +8
-0@@ -0,0 +1,20 @@ +APM X-Gene SoC GPIO controller bindings + +This is a gpio controller that is part of the flash controller. +This gpio controller controls a total of 48 gpios. + +Required properties: +- compatible: "apm,xgene-gpio" for X-Gene GPIO controller +- reg: Physical base address and size of the controller's registers
There is just the one bank?
Documentation/devicetree/bindings/gpio/gpio-xgene.txt line 11
Quoted 3 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +3
-0+- #gpio-cells: Should be two. + - first cell is the pin number + - second cell is used to specify optional parameters (unused)
Why is there an unused cell?
Why not just make this a single cell if the binding defines no valid parameters?
Documentation/devicetree/bindings/gpio/gpio-xgene.txt line 12
Quoted 1 line
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +1
-0+- gpio-controller: Marks the device node as a GPIO controller.
No interrupts?
Thanks,
Mark.
Quoted 16 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +8
-0+ +Example: + gpio0: gpio0 at 1701c000 { + compatible = "apm,xgene-gpio"; + reg = <0x0 0x1701c000 0x0 0x40>; + gpio-controller; + #gpio-cells = <2>; + };Signature · 6 lines
1.9.1 -- 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
PATCH v5 2/3 Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding
Quoted 10 lines · Mark Rutland
Documentation/devicetree/bindings/gpio/gpio-xgene.txt line 11
Quoted 3 lines · Feng Kan
+- #gpio-cells: Should be two.
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +2
-0+ - first cell is the pin number + - second cell is used to specify optional parameters (unused)Why is there an unused cell?
Why not just make this a single cell if the binding defines no valid parameters?
I don't get this either. The only reason would be that this cell should contain flags (such as open collector) the code for using it being inplemented later.
If the controller is too primitive to use such flags it should be onecell.
Yours,
Linus Walleij
PATCH v5 2/3 Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding
Quoted 7 lines · Linus Walleij
Quoted 6 lines · Mark Rutland
Quoted 3 lines · Feng Kan
+- #gpio-cells: Should be two.
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +2
-0+ - first cell is the pin number + - second cell is used to specify optional parameters (unused)
Quoted 8 lines
Quoted 4 lines
Why is there an unused cell?
Why not just make this a single cell if the binding defines no valid parameters?
I don't get this either. The only reason would be that this cell should contain flags (such as open collector) the code for using it being inplemented later.
Yes, open drain configuration and mux via pinctrl was something I planned for later.
There was another reason for this as well, part of the gpio code I read was confusing me. So I look through the other gpio documentations and found an example that did this as well.
int of_gpio_simple_xlate(struct gpio_chip *gc,
const struct of_phandle_args *gpiospec, u32 *flags)
{
/*
* We're discouraging gpio_cells < 2, since that way you'll have to
* write your own xlate function (that will have to retrive the GPIO
* number and the flags from a single gpio cell -- this is possible,
* but not recommended).
*/
Thanks, and please advise me on what to do next.
Quoted 5 lines
If the controller is too primitive to use such flags it should be onecell.
Yours,
Linus Walleij
PATCH v5 2/3 Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding
Quoted 8 lines · Feng Kan
Quoted 3 lines · Linus Walleij
I don't get this either. The only reason would be that this cell should contain flags (such as open collector) the code for using it being inplemented later.
Yes, open drain configuration and mux via pinctrl was something I planned for later.
OK but pinctrl is something else. There is also the GPIO OD configuration, as it modifies the behaviour of how to set GPIO lines.
(Yes I know the frameworks maybe ought to talk to each other for such things....)
So if you plan to do this, add it to the bindings even if you're not adding the code for handling it right now.
Quoted 4 lines
There was another reason for this as well, part of the gpio code I read was confusing me. So I look through the other gpio documentations and found an example that did this as well.
Quoted 9 lines
int of_gpio_simple_xlate(struct
gpio_chip*gc,
const structof_phandle_args*gpiospec, u32 *flags)
{
/*
* We're discouraginggpio_cells< 2, since that way you'll have to
* write your own xlate function (that will have to retrive the GPIO
* number and the flags from a single gpio cell -- this is possible,
* but not recommended).
*/
Hm yeah that's right.
This check was added by Anton in 2008. Anton why did we discourage onecell GPIOs?
Yours,
Linus Walleij
PATCH v5 2/3 Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding
On Mon, Jul 07, 2014 at 11:26:18PM +0200, Linus Walleij wrote:
...
Quoted 14 lines
Quoted 4 lines
There was another reason for this as well, part of the gpio code I read was confusing me. So I look through the other gpio documentations and found an example that did this as well.
Quoted 9 lines
int of_gpio_simple_xlate(struct
gpio_chip*gc,
const structof_phandle_args*gpiospec, u32 *flags)
{
/*
* We're discouraginggpio_cells< 2, since that way you'll have to
* write your own xlate function (that will have to retrive the GPIO
* number and the flags from a single gpio cell -- this is possible,
* but not recommended).
*/
Quoted 4 lines
Hm yeah that's right.
This check was added by Anton in 2008. Anton why did we discourage onecell GPIOs?
Yup, the check was there from the very beginning. Think of OF_GPIO_ACTIVE_LOW flag (it is widely used in drivers.)
The documentation in Feng's driver says "second cell is used to specify optional parameters (unused)," which is not entirely correct. With the standard xlate call it is used for active-low flag. You can implement active-low flag w/o using the second cell, but it will be ugly.
Thanks,
Anton
PATCH v5 2/3 Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding
In reply to Mark RutlandQuoted 10 lines · Mark Rutland
Quoted 9 lines · Feng Kan
Documentation for APM X-Gene SoC GPIO controller DTS binding.
Signed-off-by Feng Kan <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>--- .../devicetree/bindings/gpio/gpio-xgene.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xgene.txt
Quoted 16 lines
Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +0
-0Documentation/devicetree/bindings/gpio/gpio-xgene.txt line 8
Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +8
-0@@ -0,0 +1,20 @@ +APM X-Gene SoC GPIO controller bindings + +This is a gpio controller that is part of the flash controller. +This gpio controller controls a total of 48 gpios. + +Required properties: +- compatible: "apm,xgene-gpio" for X-Gene GPIO controller +- reg: Physical base address and size of the controller's registersThere is just the one bank?
Internally there are three banks. Due to the fact the address space is meshed, It was agreed that we turn this into one simple node.
Quoted 9 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt line 11
Quoted 4 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +3
-0+- #gpio-cells: Should be two. + - first cell is the pin number + - second cell is used to specify optional parameters (unused)Why is there an unused cell?
Why not just make this a single cell if the binding defines no valid parameters?
I will update documentation to indicate second cell is used for active low and active high setting since that is the behavior of the default of_xlate function. All it is
seems to be doing is flipping the value of the gpio. I initially did this because I
did not want to use additional attributes and followed the example documentation of
number of other gpio driver.
Quoted 4 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt line 12
Quoted 2 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +1
-0+- gpio-controller: Marks the device node as a GPIO controller.No interrupts?
Yes, no interrupts in this block.
Quoted 20 lines
Thanks,
Mark.Quoted 16 lines
Documentation/devicetree/bindings/gpio/gpio-xgene.txt new file +8
-1+ +Example: + gpio0: gpio0 at 1701c000 { + compatible = "apm,xgene-gpio"; + reg = <0x0 0x1701c000 0x0 0x40>; + gpio-controller; + #gpio-cells = <2>; + }; --1.9.1
--
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
PATCH v5 3/3 arm64:dts: Add APM X-Gene SoC GPIO controller DTS entries
In reply to Feng KanAdd gpio dts node for APM X-Gene SoC platform.
--- arch/arm64/boot/dts/apm-storm.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)
arch/arm64/boot/dts/apm-storm.dtsi +7 -0
@@ -397,5 +397,12 @@ #clock-cells = <1>; clocks = <&rtcclk 0>; }; + + gpio: gpio0 at 1701c000 { + compatible = "apm,xgene-gpio"; + reg = <0x0 0x1701c000 0x0 0x40>; + gpio-controller; + #gpio-cells = <2>; + }; }; };
Signature · 1 line
1.9.1
v5 cover gpio: Add APM X-Gene SoC platform GPIO driver
In reply to Feng KanQuoted 10 lines · Feng Kan
This patch add the GPIO controller in the APM X-Gene platform. The GPIO controller pins are muxed with the generic flash controller pins on the system.
V5 Change:
1. Removegpio->dev
2. Remove not needed headers.
3. Change to use latestdriver.hfor gpio.
4. Set chip base to -1
5. Put back label as it is needed sys interface.
Let's see what Linus has to say, but to me this iteration looks good!
The series,