[PATCH V5 0/3] gpio: Add APM X-Gene SoC platform GPIO driver

[PATCH V5 0/3] gpio: Add APM X-Gene SoC platform GPIO driver

[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. 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

-- 
1.9.1

[PATCH V5 1/3] gpio: Add APM X-Gene SoC GPIO controller support

[Feng Kan]

Add APM X-Gene SoC gpio controller driver.

Signed-off-by: Feng Kan <fkan@apm.com>
---
 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

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 4a1b511..833996a02 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -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
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d10f6a9..1bf5f82 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -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
diff --git a/drivers/gpio/gpio-xgene.c b/drivers/gpio/gpio-xgene.c
new file mode 100644
index 0000000..e25ba14
--- /dev/null
+++ b/drivers/gpio/gpio-xgene.c
@@ -0,0 +1,250 @@
+/*
+ * AppliedMicro X-Gene SoC GPIO Driver
+ *
+ * Copyright (c) 2014, Applied Micro Circuits Corporation
+ * Author: Feng Kan <fkan@apm.com>.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/spinlock.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/driver.h>
+#include <linux/types.h>
+#include <linux/bitops.h>
+
+#define GPIO_SET_DR_OFFSET	0x0C
+#define GPIO_DATA_OFFSET	0x14
+#define GPIO_BANK_STRIDE	0x0C
+
+#define XGENE_GPIOS_PER_BANK	16
+#define XGENE_MAX_GPIO_BANKS	3
+#define XGENE_MAX_GPIOS		(XGENE_GPIOS_PER_BANK * XGENE_MAX_GPIO_BANKS)
+
+#define GPIO_BIT_OFFSET(x)	(x % XGENE_GPIOS_PER_BANK)
+#define GPIO_BANK_OFFSET(x)	((x / XGENE_GPIOS_PER_BANK) * GPIO_BANK_STRIDE)
+
+struct xgene_gpio;
+
+struct xgene_gpio {
+	struct gpio_chip	chip;
+	void __iomem		*base;
+	spinlock_t		lock;
+#ifdef CONFIG_PM
+	u32			set_dr_val[XGENE_MAX_GPIO_BANKS];
+#endif
+};
+
+static inline struct xgene_gpio *to_xgene_gpio(struct gpio_chip *chip)
+{
+	return container_of(chip, struct xgene_gpio, chip);
+}
+
+static int xgene_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct xgene_gpio *chip = to_xgene_gpio(gc);
+	unsigned long bank_offset;
+	u32 bit_offset;
+
+	bank_offset = GPIO_DATA_OFFSET + GPIO_BANK_OFFSET(offset);
+	bit_offset = GPIO_BIT_OFFSET(offset);
+	return !!(ioread32(chip->base + bank_offset) & BIT(bit_offset));
+}
+
+static void __xgene_gpio_set(struct gpio_chip *gc, unsigned int offset, int val)
+{
+	struct xgene_gpio *chip = to_xgene_gpio(gc);
+	unsigned long bank_offset;
+	u32 setval, bit_offset;
+
+	bank_offset = GPIO_SET_DR_OFFSET + GPIO_BANK_OFFSET(offset);
+	bit_offset = GPIO_BIT_OFFSET(offset) + XGENE_GPIOS_PER_BANK;
+
+	setval = ioread32(chip->base + bank_offset);
+	if (val)
+		setval |= BIT(bit_offset);
+	else
+		setval &= ~BIT(bit_offset);
+	iowrite32(setval, chip->base + bank_offset);
+}
+
+static void xgene_gpio_set(struct gpio_chip *gc, unsigned int offset, int val)
+{
+	struct xgene_gpio *chip = to_xgene_gpio(gc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+	__xgene_gpio_set(gc, offset, val);
+	spin_unlock_irqrestore(&chip->lock, flags);
+}
+
+static int xgene_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
+{
+	struct xgene_gpio *chip = to_xgene_gpio(gc);
+	unsigned long flags, bank_offset;
+	u32 dirval, bit_offset;
+
+	bank_offset = GPIO_SET_DR_OFFSET + GPIO_BANK_OFFSET(offset);
+	bit_offset = GPIO_BIT_OFFSET(offset);
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	dirval = ioread32(chip->base + bank_offset);
+	dirval |= BIT(bit_offset);
+	iowrite32(dirval, chip->base + bank_offset);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static int xgene_gpio_dir_out(struct gpio_chip *gc,
+					unsigned int offset, int val)
+{
+	struct xgene_gpio *chip = to_xgene_gpio(gc);
+	unsigned long flags, bank_offset;
+	u32 dirval, bit_offset;
+
+	bank_offset = GPIO_SET_DR_OFFSET + GPIO_BANK_OFFSET(offset);
+	bit_offset = GPIO_BIT_OFFSET(offset);
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	dirval = ioread32(chip->base + bank_offset);
+	dirval &= ~BIT(bit_offset);
+	iowrite32(dirval, chip->base + bank_offset);
+	__xgene_gpio_set(gc, offset, val);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int xgene_gpio_suspend(struct device *dev)
+{
+	struct xgene_gpio *gpio = dev_get_drvdata(dev);
+	unsigned long bank_offset;
+	unsigned int bank;
+
+	for (bank = 0; bank < XGENE_MAX_GPIO_BANKS; bank++) {
+		bank_offset = GPIO_SET_DR_OFFSET + bank * GPIO_BANK_STRIDE;
+		gpio->set_dr_val[bank] = ioread32(gpio->base + bank_offset);
+	}
+	return 0;
+}
+
+static int xgene_gpio_resume(struct device *dev)
+{
+	struct xgene_gpio *gpio = dev_get_drvdata(dev);
+	unsigned long bank_offset;
+	unsigned int bank;
+
+	for (bank = 0; bank < XGENE_MAX_GPIO_BANKS; bank++) {
+		bank_offset = GPIO_SET_DR_OFFSET + bank * GPIO_BANK_STRIDE;
+		iowrite32(gpio->set_dr_val[bank], gpio->base + bank_offset);
+	}
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(xgene_gpio_pm, xgene_gpio_suspend, xgene_gpio_resume);
+#define XGENE_GPIO_PM_OPS	(&xgene_gpio_pm)
+#else
+#define XGENE_GPIO_PM_OPS	NULL
+#endif
+
+static int xgene_gpio_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct xgene_gpio *gpio;
+	int err = 0;
+
+	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+	if (!gpio) {
+		err = -ENOMEM;
+		goto err;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	gpio->base = devm_ioremap_nocache(&pdev->dev, res->start,
+							resource_size(res));
+	if (IS_ERR(gpio->base)) {
+		err = PTR_ERR(gpio->base);
+		goto err;
+	}
+
+	gpio->chip.ngpio = XGENE_MAX_GPIOS;
+
+	gpio->chip.dev = &pdev->dev;
+	gpio->chip.direction_input = xgene_gpio_dir_in;
+	gpio->chip.direction_output = xgene_gpio_dir_out;
+	gpio->chip.get = xgene_gpio_get;
+	gpio->chip.set = xgene_gpio_set;
+	gpio->chip.label = dev_name(&pdev->dev);
+	gpio->chip.base = -1;
+
+	platform_set_drvdata(pdev, gpio);
+
+	err = gpiochip_add(&gpio->chip);
+	if (err) {
+		dev_err(&pdev->dev,
+			"failed to register gpiochip.\n");
+		goto err;
+	}
+
+	dev_info(&pdev->dev, "X-Gene GPIO driver registered.\n");
+	return 0;
+err:
+	dev_err(&pdev->dev, "X-Gene GPIO driver registration failed.\n");
+	return err;
+}
+
+static int xgene_gpio_remove(struct platform_device *pdev)
+{
+	struct xgene_gpio *gpio = platform_get_drvdata(pdev);
+	int ret = 0;
+
+	ret = gpiochip_remove(&gpio->chip);
+	if (ret)
+		dev_err(&pdev->dev, "unable to remove gpio_chip.\n");
+	return ret;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id xgene_gpio_of_match[] = {
+	{ .compatible = "apm,xgene-gpio", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, xgene_gpio_of_match);
+#endif
+
+static struct platform_driver xgene_gpio_driver = {
+	.driver = {
+		.name = "xgene-gpio",
+		.owner = THIS_MODULE,
+		.of_match_table = xgene_gpio_of_match,
+		.pm     = XGENE_GPIO_PM_OPS,
+	},
+	.probe = xgene_gpio_probe,
+	.remove = xgene_gpio_remove,
+};
+
+module_platform_driver(xgene_gpio_driver);
+
+MODULE_AUTHOR("Feng Kan <fkan@apm.com>");
+MODULE_DESCRIPTION("APM X-Gene GPIO driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1

[PATCH V5 2/3] Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding

[Feng Kan]

Documentation for APM X-Gene SoC GPIO controller DTS binding.

Signed-off-by: Feng Kan <fkan@apm.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
---
 .../devicetree/bindings/gpio/gpio-xgene.txt          | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xgene.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-xgene.txt b/Documentation/devicetree/bindings/gpio/gpio-xgene.txt
new file mode 100644
index 0000000..bd5fd85
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-xgene.txt
@@ -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>;
+	};
-- 
1.9.1

[PATCH V5 3/3] arm64:dts: Add APM X-Gene SoC GPIO controller DTS entries

[Feng Kan]

Add gpio dts node for APM X-Gene SoC platform.

Signed-off-by: Feng Kan <fkan@apm.com>
---
 arch/arm64/boot/dts/apm-storm.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index 40aa96c..a203a4e 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -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>;
+		};
 	};
 };
-- 
1.9.1

[PATCH V5 0/3] gpio: Add APM X-Gene SoC platform GPIO driver

[Alexandre Courbot]

On Fri, Jun 27, 2014 at 7:59 AM, Feng Kan <fkan@apm.com> wrote:
> 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.

Let's see what Linus has to say, but to me this iteration looks good!

The series,

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

[PATCH V5 2/3] Documentation: gpio: Add APM X-Gene SoC GPIO controller DTS binding

[Mark Rutland]

On Thu, Jun 26, 2014 at 11:59:46PM +0100, Feng Kan wrote:
> Documentation for APM X-Gene SoC GPIO controller DTS binding.
> 
> Signed-off-by: Feng Kan <fkan@apm.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  .../devicetree/bindings/gpio/gpio-xgene.txt          | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xgene.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-xgene.txt b/Documentation/devicetree/bindings/gpio/gpio-xgene.txt
> new file mode 100644
> index 0000000..bd5fd85
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-xgene.txt
> @@ -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?

> +- #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?

> +- gpio-controller: Marks the device node as a GPIO controller.

No interrupts?

Thanks,
Mark.

> +
> +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 1/3] gpio: Add APM X-Gene SoC GPIO controller support

[Linus Walleij]

On Fri, Jun 27, 2014 at 12:59 AM, Feng Kan <fkan@apm.com> wrote:

> Add APM X-Gene SoC gpio controller driver.
>
> Signed-off-by: Feng Kan <fkan@apm.com>

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

[Linus Walleij]

On Fri, Jul 4, 2014 at 11:20 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Jun 27, 2014 at 12:59 AM, Feng Kan <fkan@apm.com> wrote:
>
>> Add APM X-Gene SoC gpio controller driver.
>>
>> Signed-off-by: Feng Kan <fkan@apm.com>
>
> 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

[Linus Walleij]

On Mon, Jun 30, 2014 at 2:53 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Jun 26, 2014 at 11:59:46PM +0100, Feng Kan wrote:

>> +- #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 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

[Feng Kan]

On Fri, Jul 4, 2014 at 2:28 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Jun 30, 2014 at 2:53 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> On Thu, Jun 26, 2014 at 11:59:46PM +0100, Feng Kan wrote:
>
>>> +- #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 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.
>
> 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

[Linus Walleij]

On Mon, Jul 7, 2014 at 8:52 PM, Feng Kan <fkan@apm.com> wrote:
> On Fri, Jul 4, 2014 at 2:28 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> 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.

> 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).
>          */

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

[Anton Vorontsov]

On Mon, Jul 07, 2014 at 11:26:18PM +0200, Linus Walleij wrote:
...
> > 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).
> >          */
> 
> 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

[Feng Kan]

On Mon, Jun 30, 2014 at 5:53 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Jun 26, 2014 at 11:59:46PM +0100, Feng Kan wrote:
>> Documentation for APM X-Gene SoC GPIO controller DTS binding.
>>
>> Signed-off-by: Feng Kan <fkan@apm.com>
>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>  .../devicetree/bindings/gpio/gpio-xgene.txt          | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xgene.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-xgene.txt b/Documentation/devicetree/bindings/gpio/gpio-xgene.txt
>> new file mode 100644
>> index 0000000..bd5fd85
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-xgene.txt
>> @@ -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?

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.

>
>> +- #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.
>
>> +- gpio-controller: Marks the device node as a GPIO controller.
>
> No interrupts?

Yes, no interrupts in this block.
>
> Thanks,
> Mark.
>
>> +
>> +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
>>