[PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Jamie Iles]

The Synopsys DesignWare block is used in some ARM devices (picoxcell)
and can be configured to provide multiple banks of GPIO pins.  The first
bank (A) can also provide IRQ capabilities.

v2:	- use Rob Herring's irqdomain in generic irq chip patches
	- use reg property to indicate bank index
	- support irqs on both edges based on LinusW's u300 driver

Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
---

Rob, I've based this on your generic irq chip + irq domain support but I
don't have the original mail in my inbox to reply to with a Tested-by,
	but feel free to add a:

	Tested-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>

to "irq: convert generic-chip to use irq_domain" if you like.

 .../devicetree/bindings/gpio/snps-dwapb-gpio.txt   |   63 ++++
 drivers/gpio/Kconfig                               |   10 +
 drivers/gpio/Makefile                              |    1 +
 drivers/gpio/gpio-dwapb.c                          |  351 ++++++++++++++++++++
 4 files changed, 425 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
 create mode 100644 drivers/gpio/gpio-dwapb.c

diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
new file mode 100644
index 0000000..7ef694bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
@@ -0,0 +1,63 @@
+* Synopsys DesignWare APB GPIO controller
+
+Required properties:
+- compatible : Should be "snps,dw-apb-gpio"
+- reg : Address and length of the register set for the device
+
+The GPIO controller has a configurable number of banks, each of which are
+represented as child nodes with the following properties:
+
+Required properties:
+- compatible : "snps,dw-apb-gpio-bank"
+- gpio-controller : Marks the device node as a gpio controller.
+- #gpio-cells : Should be two.  The first cell is the pin number and
+  the second cell is used to specify optional parameters (currently
+  unused).
+- reg : The integer bank index of the bank, a single cell.
+- nr-gpio : The number of pins in the bank, a single cell.
+
+Optional properties:
+- interrupt-controller : The first bank may be configured to be an interrupt
+controller.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+interrupt.  Shall be set to 2.  The first cell defines the interrupt number,
+the second encodes the triger flags encoded as:
+
+	bits[3:0] trigger type and level flags.
+		1 = low-to-high edge triggered
+		2 = high-to-low edge triggered
+		4 = active high level-sensitive
+		8 = active low level-sensitive
+
+- interrupt-parent : The parent interrupt controller.
+- interrupts : The interrupts to the parent controller raised when GPIOs
+generate the interrupts.
+
+Example:
+
+gpio: gpio@20000 {
+	compatible = "snps,dw-apb-gpio";
+	reg = <0x20000 0x1000>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	banka: gpio-controller@0 {
+		compatible = "snps,dw-apb-gpio-bank";
+		gpio-controller;
+		#gpio-cells = <2>;
+		nr-gpio = <8>;
+		reg = <0>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&vic1>;
+		interrupts = <0 1 2 3 4 5 6 7>;
+	};
+
+	bankb: gpio-controller@1 {
+		compatible = "snps,dw-apb-gpio-bank";
+		gpio-controller;
+		#gpio-cells = <2>;
+		nr-gpio = <8>;
+		reg = <1>;
+	};
+};
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8482a23..61c16f3 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -85,6 +85,16 @@ config GPIO_GENERIC_PLATFORM
 	help
 	  Say yes here to support basic platform_device memory-mapped GPIO controllers.
 
+config GPIO_DWAPB
+	bool "Synopsys DesignWare APB GPIO driver"
+	select GPIO_GENERIC
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN
+	depends on OF_GPIO
+	help
+	  Say Y or M here to build support for the Synopsys DesignWare APB
+	  GPIO block.  This requires device tree support.
+
 config GPIO_IT8761E
 	tristate "IT8761E GPIO support"
 	help
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index dbcb0bc..22665a0 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CS5535)	+= gpio-cs5535.o
 obj-$(CONFIG_GPIO_DA9052)	+= gpio-da9052.o
 obj-$(CONFIG_ARCH_DAVINCI)	+= gpio-davinci.o
+obj-$(CONFIG_GPIO_DWAPB)	+= gpio-dwapb.o
 obj-$(CONFIG_GPIO_EP93XX)	+= gpio-ep93xx.o
 obj-$(CONFIG_GPIO_IT8761E)	+= gpio-it8761e.o
 obj-$(CONFIG_GPIO_JANZ_TTL)	+= gpio-janz-ttl.o
diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
new file mode 100644
index 0000000..dae263c
--- /dev/null
+++ b/drivers/gpio/gpio-dwapb.c
@@ -0,0 +1,351 @@
+/*
+ * Copyright (c) 2011 Jamie Iles
+ *
+ * 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.
+ *
+ * All enquiries to support-QECmZ7LgVXZWk0Htik3J/w@public.gmane.org
+ */
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/basic_mmio_gpio.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+
+#define INT_EN_REG_OFFS		0x30
+#define INT_MASK_REG_OFFS	0x34
+#define INT_TYPE_REG_OFFS	0x38
+#define INT_POLARITY_REG_OFFS	0x3c
+#define INT_STATUS_REG_OFFS	0x40
+#define EOI_REG_OFFS		0x4c
+
+struct dwapb_gpio;
+
+struct dwapb_gpio_bank {
+	struct bgpio_chip	bgc;
+	unsigned int		bank_idx;
+	bool			is_registered;
+	struct dwapb_gpio	*gpio;
+};
+
+struct dwapb_gpio {
+	struct device_node	*of_node;
+	struct	device		*dev;
+	void __iomem		*regs;
+	struct dwapb_gpio_bank	*banks;
+	unsigned int		nr_banks;
+	struct irq_chip_generic	*irq_gc;
+	unsigned long		toggle_edge;
+};
+
+static unsigned int dwapb_gpio_nr_banks(struct device_node *of_node)
+{
+	unsigned int nr_banks = 0;
+	struct device_node *np;
+
+	for_each_child_of_node(of_node, np)
+		++nr_banks;
+
+	return nr_banks;
+}
+
+static int dwapb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+	struct dwapb_gpio_bank *bank = container_of(bgc, struct
+						    dwapb_gpio_bank, bgc);
+	struct dwapb_gpio *gpio = bank->gpio;
+
+	return irq_domain_to_irq(&gpio->irq_gc->domain, offset);
+}
+
+static void dwapb_toggle_trigger(struct dwapb_gpio *gpio, unsigned int offs)
+{
+	u32 v = readl(gpio->regs + INT_TYPE_REG_OFFS);
+
+	if (gpio_get_value(gpio->banks[0].bgc.gc.base + offs))
+		v &= ~BIT(offs);
+	else
+		v |= BIT(offs);
+
+	writel(v, gpio->regs + INT_TYPE_REG_OFFS);
+}
+
+static void dwapb_irq_handler(u32 irq, struct irq_desc *desc)
+{
+	struct dwapb_gpio *gpio = irq_get_handler_data(irq);
+	u32 irq_status = readl(gpio->regs + INT_STATUS_REG_OFFS);
+
+	while (irq_status) {
+		int irqoffset = fls(irq_status) - 1;
+		int irq = irq_domain_to_irq(&gpio->irq_gc->domain, irqoffset);
+
+		generic_handle_irq(irq);
+		irq_status &= ~(1 << irqoffset);
+
+		if (gpio->toggle_edge & BIT(irqoffset))
+			dwapb_toggle_trigger(gpio, irqoffset);
+	}
+}
+
+static void dwapb_irq_enable(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct dwapb_gpio *gpio = gc->private;
+
+	u32 val = readl(gpio->regs + INT_EN_REG_OFFS);
+	val |= 1 << d->hwirq;
+	writel(val, gpio->regs + INT_EN_REG_OFFS);
+}
+
+static void dwapb_irq_disable(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct dwapb_gpio *gpio = gc->private;
+
+	u32 val = readl(gpio->regs + INT_EN_REG_OFFS);
+	val &= ~(1 << d->hwirq);
+	writel(val, gpio->regs + INT_EN_REG_OFFS);
+}
+
+static int dwapb_irq_set_type(struct irq_data *d, u32 type)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct dwapb_gpio *gpio = gc->private;
+	int bit = d->hwirq;
+	unsigned long level, polarity;
+
+	if (type & ~(IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
+		     IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
+		return -EINVAL;
+
+	level = readl(gpio->regs + INT_TYPE_REG_OFFS);
+	polarity = readl(gpio->regs + INT_POLARITY_REG_OFFS);
+
+	gpio->toggle_edge &= ~BIT(bit);
+	if (type & IRQ_TYPE_EDGE_BOTH) {
+		gpio->toggle_edge |= BIT(bit);
+		level |= (1 << bit);
+		dwapb_toggle_trigger(gpio, bit);
+	} else if (type & IRQ_TYPE_EDGE_RISING) {
+		level |= (1 << bit);
+		polarity |= (1 << bit);
+	} else if (type & IRQ_TYPE_EDGE_FALLING) {
+		level |= (1 << bit);
+		polarity &= ~(1 << bit);
+	} else if (type & IRQ_TYPE_LEVEL_HIGH) {
+		level &= ~(1 << bit);
+		polarity |= (1 << bit);
+	} else if (type & IRQ_TYPE_LEVEL_LOW) {
+		level &= ~(1 << bit);
+		polarity &= ~(1 << bit);
+	}
+
+	writel(level, gpio->regs + INT_TYPE_REG_OFFS);
+	writel(polarity, gpio->regs + INT_POLARITY_REG_OFFS);
+
+	return 0;
+}
+
+static int dwapb_create_irqchip(struct dwapb_gpio *gpio,
+				struct dwapb_gpio_bank *bank,
+				unsigned int irq_base)
+{
+	struct irq_chip_type *ct;
+
+	gpio->irq_gc = irq_alloc_generic_chip("gpio-dwapb", 1, irq_base,
+					      gpio->regs, handle_level_irq);
+	if (!gpio->irq_gc)
+		return -EIO;
+
+	gpio->irq_gc->domain.of_node = of_node_get(bank->bgc.gc.of_node);
+	gpio->irq_gc->private = gpio;
+	ct = gpio->irq_gc->chip_types;
+	ct->chip.irq_ack = irq_gc_ack_set_bit;
+	ct->chip.irq_mask = irq_gc_mask_set_bit;
+	ct->chip.irq_unmask = irq_gc_mask_clr_bit;
+	ct->chip.irq_set_type = dwapb_irq_set_type;
+	ct->chip.irq_enable = dwapb_irq_enable;
+	ct->chip.irq_disable = dwapb_irq_disable;
+	ct->regs.ack = EOI_REG_OFFS;
+	ct->regs.mask = INT_MASK_REG_OFFS;
+	irq_setup_generic_chip(gpio->irq_gc, IRQ_MSK(bank->bgc.gc.ngpio),
+			       IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0);
+
+	return 0;
+}
+
+static int dwapb_configure_irqs(struct dwapb_gpio *gpio,
+				struct dwapb_gpio_bank *bank)
+{
+	unsigned int m, irq, ngpio = bank->bgc.gc.ngpio;
+	int irq_base;
+
+	for (m = 0; m < ngpio; ++m) {
+		irq = irq_of_parse_and_map(bank->bgc.gc.of_node, m);
+		if (!irq && m == 0) {
+			dev_warn(gpio->dev, "no irq for bank %s\n",
+				 bank->bgc.gc.of_node->full_name);
+			return -ENXIO;
+		} else if (!irq) {
+			break;
+		}
+
+		irq_set_chained_handler(irq, dwapb_irq_handler);
+		irq_set_handler_data(irq, gpio);
+	}
+	bank->bgc.gc.to_irq = dwapb_gpio_to_irq;
+
+	irq_base = irq_alloc_descs(-1, 0, ngpio, NUMA_NO_NODE);
+	if (irq_base < 0)
+		return irq_base;
+
+	if (dwapb_create_irqchip(gpio, bank, irq_base))
+		goto out_free_descs;
+
+	return 0;
+
+out_free_descs:
+	irq_free_descs(irq_base, ngpio);
+
+	return -EIO;
+}
+
+static int dwapb_gpio_add_bank(struct dwapb_gpio *gpio,
+			       struct device_node *bank_np)
+{
+	struct dwapb_gpio_bank *bank;
+	u32 bank_idx, ngpio;
+	int err;
+
+	if (of_property_read_u32(bank_np, "reg", &bank_idx)) {
+		dev_err(gpio->dev, "invalid bank index for %s\n",
+			bank_np->full_name);
+		return -EINVAL;
+	}
+	bank = &gpio->banks[bank_idx];
+	bank->gpio = gpio;
+
+	if (of_property_read_u32(bank_np, "nr-gpio", &ngpio)) {
+		dev_err(gpio->dev, "failed to get number of gpios for %s\n",
+			bank_np->full_name);
+		return -EINVAL;
+	}
+
+	bank->bank_idx = bank_idx;
+	err = bgpio_init(&bank->bgc, gpio->dev, 4,
+			 gpio->regs + 0x50 + (bank_idx * 0x4),
+			 gpio->regs + 0x00 + (bank_idx * 0xc),
+			 NULL, gpio->regs + 0x04 + (bank_idx * 0xc), NULL,
+			 false);
+	if (err) {
+		dev_err(gpio->dev, "failed to init gpio chip for %s\n",
+			bank_np->full_name);
+		return err;
+	}
+
+	bank->bgc.gc.ngpio = ngpio;
+	bank->bgc.gc.of_node = bank_np;
+
+	/*
+	 * Only bank A can provide interrupts in all configurations of the IP.
+	 */
+	if (bank_idx == 0 &&
+	    of_get_property(bank_np, "interrupt-controller", NULL))
+		dwapb_configure_irqs(gpio, bank);
+
+	err = gpiochip_add(&bank->bgc.gc);
+	if (err)
+		dev_err(gpio->dev, "failed to register gpiochip for %s\n",
+			bank_np->full_name);
+	else
+		bank->is_registered = true;
+
+	return err;
+}
+
+static void dwapb_gpio_unregister(struct dwapb_gpio *gpio)
+{
+	unsigned int m;
+
+	for (m = 0; m < gpio->nr_banks; ++m)
+		if (gpio->banks[m].is_registered)
+			gpiochip_remove(&gpio->banks[m].bgc.gc);
+	of_node_put(gpio->of_node);
+}
+
+static int __devinit dwapb_gpio_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct dwapb_gpio *gpio;
+	struct device_node *np;
+	int err;
+
+	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+	if (!gpio)
+		return -ENOMEM;
+	gpio->dev = &pdev->dev;
+
+	gpio->nr_banks = dwapb_gpio_nr_banks(pdev->dev.of_node);
+	if (!gpio->nr_banks)
+		return -EINVAL;
+	gpio->banks = devm_kzalloc(&pdev->dev, gpio->nr_banks *
+				   sizeof(*gpio->banks), GFP_KERNEL);
+	if (!gpio->banks)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "failed to get iomem\n");
+		return -ENXIO;
+	}
+	gpio->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!gpio->regs)
+		return -ENOMEM;
+
+	gpio->of_node = of_node_get(pdev->dev.of_node);
+	for_each_child_of_node(pdev->dev.of_node, np) {
+		err = dwapb_gpio_add_bank(gpio, np);
+		if (err)
+			goto out_unregister;
+	}
+	platform_set_drvdata(pdev, gpio);
+
+	return 0;
+
+out_unregister:
+	dwapb_gpio_unregister(gpio);
+
+	return err;
+}
+
+static const struct of_device_id dwapb_of_match_table[] = {
+	{ .compatible = "snps,dw-apb-gpio" },
+	{ /* Sentinel */ }
+};
+
+static struct platform_driver dwapb_gpio_driver = {
+	.driver		= {
+		.name	= "gpio-dwapb",
+		.owner	= THIS_MODULE,
+		.of_match_table = dwapb_of_match_table,
+	},
+	.probe		= dwapb_gpio_probe,
+};
+
+static int __init dwapb_gpio_init(void)
+{
+	return platform_driver_register(&dwapb_gpio_driver);
+}
+postcore_initcall(dwapb_gpio_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jamie Iles");
+MODULE_DESCRIPTION("Synopsys DesignWare APB GPIO driver");
-- 
1.7.5.4

[PATCHv2 2/2] ARM: picoxcell: use new Synopsys Designware GPIO binding

[Jamie Iles]

Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
---
 arch/arm/boot/dts/picoxcell-pc3x2.dtsi |   19 ++++++++-----------
 arch/arm/boot/dts/picoxcell-pc3x3.dtsi |   28 +++++++++++-----------------
 2 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/arch/arm/boot/dts/picoxcell-pc3x2.dtsi b/arch/arm/boot/dts/picoxcell-pc3x2.dtsi
index f0a8c20..a247812 100644
--- a/arch/arm/boot/dts/picoxcell-pc3x2.dtsi
+++ b/arch/arm/boot/dts/picoxcell-pc3x2.dtsi
@@ -169,28 +169,25 @@
 				reg = <0x20000 0x1000>;
 				#address-cells = <1>;
 				#size-cells = <0>;
-				reg-io-width = <4>;
 
 				banka: gpio-controller@0 {
 					compatible = "snps,dw-apb-gpio-bank";
 					gpio-controller;
 					#gpio-cells = <2>;
-					gpio-generic,nr-gpio = <8>;
-
-					regoffset-dat = <0x50>;
-					regoffset-set = <0x00>;
-					regoffset-dirout = <0x04>;
+					nr-gpio = <8>;
+					reg = <0>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+					interrupt-parent = <&vic1>;
+					interrupts = <0 1 2 3 4 5 6 7>;
 				};
 
 				bankb: gpio-controller@1 {
 					compatible = "snps,dw-apb-gpio-bank";
 					gpio-controller;
 					#gpio-cells = <2>;
-					gpio-generic,nr-gpio = <8>;
-
-					regoffset-dat = <0x54>;
-					regoffset-set = <0x0c>;
-					regoffset-dirout = <0x10>;
+					nr-gpio = <8>;
+					reg = <1>;
 				};
 			};
 
diff --git a/arch/arm/boot/dts/picoxcell-pc3x3.dtsi b/arch/arm/boot/dts/picoxcell-pc3x3.dtsi
index daa962d..96f049f 100644
--- a/arch/arm/boot/dts/picoxcell-pc3x3.dtsi
+++ b/arch/arm/boot/dts/picoxcell-pc3x3.dtsi
@@ -252,39 +252,33 @@
 				reg = <0x20000 0x1000>;
 				#address-cells = <1>;
 				#size-cells = <0>;
-				reg-io-width = <4>;
 
 				banka: gpio-controller@0 {
 					compatible = "snps,dw-apb-gpio-bank";
 					gpio-controller;
+					reg = <0>;
 					#gpio-cells = <2>;
-					gpio-generic,nr-gpio = <8>;
-
-					regoffset-dat = <0x50>;
-					regoffset-set = <0x00>;
-					regoffset-dirout = <0x04>;
+					nr-gpio = <8>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+					interrupt-parent = <&vic1>;
+					interrupts = <0 1 2 3 4 5 6 7>;
 				};
 
 				bankb: gpio-controller@1 {
 					compatible = "snps,dw-apb-gpio-bank";
 					gpio-controller;
+					reg = <1>;
 					#gpio-cells = <2>;
-					gpio-generic,nr-gpio = <16>;
-
-					regoffset-dat = <0x54>;
-					regoffset-set = <0x0c>;
-					regoffset-dirout = <0x10>;
+					nr-gpio = <16>;
 				};
 
-				bankd: gpio-controller@2 {
+				bankd: gpio-controller@3 {
 					compatible = "snps,dw-apb-gpio-bank";
 					gpio-controller;
+					reg = <3>;
 					#gpio-cells = <2>;
-					gpio-generic,nr-gpio = <30>;
-
-					regoffset-dat = <0x5c>;
-					regoffset-set = <0x24>;
-					regoffset-dirout = <0x28>;
+					nr-gpio = <30>;
 				};
 			};
 
-- 
1.7.5.4

Re: [PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Rob Herring]

Jamie,

On 12/19/2011 07:43 PM, Jamie Iles wrote:
> The Synopsys DesignWare block is used in some ARM devices (picoxcell)
> and can be configured to provide multiple banks of GPIO pins.  The first
> bank (A) can also provide IRQ capabilities.
> 
> v2:	- use Rob Herring's irqdomain in generic irq chip patches
> 	- use reg property to indicate bank index
> 	- support irqs on both edges based on LinusW's u300 driver
> 
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Cc: Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> ---
> 
> Rob, I've based this on your generic irq chip + irq domain support but I
> don't have the original mail in my inbox to reply to with a Tested-by,
> 	but feel free to add a:
> 
> 	Tested-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> 
> to "irq: convert generic-chip to use irq_domain" if you like.
> 
>  .../devicetree/bindings/gpio/snps-dwapb-gpio.txt   |   63 ++++
>  drivers/gpio/Kconfig                               |   10 +
>  drivers/gpio/Makefile                              |    1 +
>  drivers/gpio/gpio-dwapb.c                          |  351 ++++++++++++++++++++
>  4 files changed, 425 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
>  create mode 100644 drivers/gpio/gpio-dwapb.c
> 
> diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> new file mode 100644
> index 0000000..7ef694bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> @@ -0,0 +1,63 @@
> +* Synopsys DesignWare APB GPIO controller
> +
> +Required properties:
> +- compatible : Should be "snps,dw-apb-gpio"
> +- reg : Address and length of the register set for the device
> +
> +The GPIO controller has a configurable number of banks, each of which are
> +represented as child nodes with the following properties:
> +
> +Required properties:
> +- compatible : "snps,dw-apb-gpio-bank"
> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells : Should be two.  The first cell is the pin number and
> +  the second cell is used to specify optional parameters (currently
> +  unused).
> +- reg : The integer bank index of the bank, a single cell.
> +- nr-gpio : The number of pins in the bank, a single cell.
> +
> +Optional properties:
> +- interrupt-controller : The first bank may be configured to be an interrupt
> +controller.
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +interrupt.  Shall be set to 2.  The first cell defines the interrupt number,
> +the second encodes the triger flags encoded as:
> +
> +	bits[3:0] trigger type and level flags.
> +		1 = low-to-high edge triggered
> +		2 = high-to-low edge triggered
> +		4 = active high level-sensitive
> +		8 = active low level-sensitive
> +
> +- interrupt-parent : The parent interrupt controller.
> +- interrupts : The interrupts to the parent controller raised when GPIOs
> +generate the interrupts.
> +
> +Example:
> +
> +gpio: gpio@20000 {
> +	compatible = "snps,dw-apb-gpio";
> +	reg = <0x20000 0x1000>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	banka: gpio-controller@0 {
> +		compatible = "snps,dw-apb-gpio-bank";
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		nr-gpio = <8>;
> +		reg = <0>;

Actually, what I meant was to use the bank addresses here. Sorry that
wasn't clear. Then you don't need a calculation of bank offsets.

There is the problem of creating multiple mappings since it is all
within a 4KB page. Ideally, ioremap would be smart enough to reuse
existing mappings. It does for static mappings now, but not dynamic ones
AFAIK. I'm not sure how to best deal with this in DT, but it's a fairly
common issue. I see a couple of options:

- Make reg for the bank be the absolute address (or relative to the bus
ranges) and remove the top level reg property.
- Make reg for the bank just be the offset from the gpio base and keep
the top level reg property.

The latter would be easier to create a single mapping.

Rob

> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-parent = <&vic1>;
> +		interrupts = <0 1 2 3 4 5 6 7>;
> +	};
> +
> +	bankb: gpio-controller@1 {
> +		compatible = "snps,dw-apb-gpio-bank";
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		nr-gpio = <8>;
> +		reg = <1>;
> +	};
> +};
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 8482a23..61c16f3 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -85,6 +85,16 @@ config GPIO_GENERIC_PLATFORM
>  	help
>  	  Say yes here to support basic platform_device memory-mapped GPIO controllers.
>  
> +config GPIO_DWAPB
> +	bool "Synopsys DesignWare APB GPIO driver"
> +	select GPIO_GENERIC
> +	select GENERIC_IRQ_CHIP
> +	select IRQ_DOMAIN
> +	depends on OF_GPIO
> +	help
> +	  Say Y or M here to build support for the Synopsys DesignWare APB
> +	  GPIO block.  This requires device tree support.
> +
>  config GPIO_IT8761E
>  	tristate "IT8761E GPIO support"
>  	help
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index dbcb0bc..22665a0 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
>  obj-$(CONFIG_GPIO_CS5535)	+= gpio-cs5535.o
>  obj-$(CONFIG_GPIO_DA9052)	+= gpio-da9052.o
>  obj-$(CONFIG_ARCH_DAVINCI)	+= gpio-davinci.o
> +obj-$(CONFIG_GPIO_DWAPB)	+= gpio-dwapb.o
>  obj-$(CONFIG_GPIO_EP93XX)	+= gpio-ep93xx.o
>  obj-$(CONFIG_GPIO_IT8761E)	+= gpio-it8761e.o
>  obj-$(CONFIG_GPIO_JANZ_TTL)	+= gpio-janz-ttl.o
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> new file mode 100644
> index 0000000..dae263c
> --- /dev/null
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -0,0 +1,351 @@
> +/*
> + * Copyright (c) 2011 Jamie Iles
> + *
> + * 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.
> + *
> + * All enquiries to support-QECmZ7LgVXZWk0Htik3J/w@public.gmane.org
> + */
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/basic_mmio_gpio.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +
> +#define INT_EN_REG_OFFS		0x30
> +#define INT_MASK_REG_OFFS	0x34
> +#define INT_TYPE_REG_OFFS	0x38
> +#define INT_POLARITY_REG_OFFS	0x3c
> +#define INT_STATUS_REG_OFFS	0x40
> +#define EOI_REG_OFFS		0x4c
> +
> +struct dwapb_gpio;
> +
> +struct dwapb_gpio_bank {
> +	struct bgpio_chip	bgc;
> +	unsigned int		bank_idx;
> +	bool			is_registered;
> +	struct dwapb_gpio	*gpio;
> +};
> +
> +struct dwapb_gpio {
> +	struct device_node	*of_node;
> +	struct	device		*dev;
> +	void __iomem		*regs;
> +	struct dwapb_gpio_bank	*banks;
> +	unsigned int		nr_banks;
> +	struct irq_chip_generic	*irq_gc;
> +	unsigned long		toggle_edge;
> +};
> +
> +static unsigned int dwapb_gpio_nr_banks(struct device_node *of_node)
> +{
> +	unsigned int nr_banks = 0;
> +	struct device_node *np;
> +
> +	for_each_child_of_node(of_node, np)
> +		++nr_banks;
> +
> +	return nr_banks;
> +}
> +
> +static int dwapb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
> +{
> +	struct bgpio_chip *bgc = to_bgpio_chip(gc);
> +	struct dwapb_gpio_bank *bank = container_of(bgc, struct
> +						    dwapb_gpio_bank, bgc);
> +	struct dwapb_gpio *gpio = bank->gpio;
> +
> +	return irq_domain_to_irq(&gpio->irq_gc->domain, offset);
> +}
> +
> +static void dwapb_toggle_trigger(struct dwapb_gpio *gpio, unsigned int offs)
> +{
> +	u32 v = readl(gpio->regs + INT_TYPE_REG_OFFS);
> +
> +	if (gpio_get_value(gpio->banks[0].bgc.gc.base + offs))
> +		v &= ~BIT(offs);
> +	else
> +		v |= BIT(offs);
> +
> +	writel(v, gpio->regs + INT_TYPE_REG_OFFS);
> +}
> +
> +static void dwapb_irq_handler(u32 irq, struct irq_desc *desc)
> +{
> +	struct dwapb_gpio *gpio = irq_get_handler_data(irq);
> +	u32 irq_status = readl(gpio->regs + INT_STATUS_REG_OFFS);
> +
> +	while (irq_status) {
> +		int irqoffset = fls(irq_status) - 1;
> +		int irq = irq_domain_to_irq(&gpio->irq_gc->domain, irqoffset);
> +
> +		generic_handle_irq(irq);
> +		irq_status &= ~(1 << irqoffset);
> +
> +		if (gpio->toggle_edge & BIT(irqoffset))
> +			dwapb_toggle_trigger(gpio, irqoffset);
> +	}
> +}
> +
> +static void dwapb_irq_enable(struct irq_data *d)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct dwapb_gpio *gpio = gc->private;
> +
> +	u32 val = readl(gpio->regs + INT_EN_REG_OFFS);
> +	val |= 1 << d->hwirq;
> +	writel(val, gpio->regs + INT_EN_REG_OFFS);
> +}
> +
> +static void dwapb_irq_disable(struct irq_data *d)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct dwapb_gpio *gpio = gc->private;
> +
> +	u32 val = readl(gpio->regs + INT_EN_REG_OFFS);
> +	val &= ~(1 << d->hwirq);
> +	writel(val, gpio->regs + INT_EN_REG_OFFS);
> +}
> +
> +static int dwapb_irq_set_type(struct irq_data *d, u32 type)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct dwapb_gpio *gpio = gc->private;
> +	int bit = d->hwirq;
> +	unsigned long level, polarity;
> +
> +	if (type & ~(IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
> +		     IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
> +		return -EINVAL;
> +
> +	level = readl(gpio->regs + INT_TYPE_REG_OFFS);
> +	polarity = readl(gpio->regs + INT_POLARITY_REG_OFFS);
> +
> +	gpio->toggle_edge &= ~BIT(bit);
> +	if (type & IRQ_TYPE_EDGE_BOTH) {
> +		gpio->toggle_edge |= BIT(bit);
> +		level |= (1 << bit);
> +		dwapb_toggle_trigger(gpio, bit);
> +	} else if (type & IRQ_TYPE_EDGE_RISING) {
> +		level |= (1 << bit);
> +		polarity |= (1 << bit);
> +	} else if (type & IRQ_TYPE_EDGE_FALLING) {
> +		level |= (1 << bit);
> +		polarity &= ~(1 << bit);
> +	} else if (type & IRQ_TYPE_LEVEL_HIGH) {
> +		level &= ~(1 << bit);
> +		polarity |= (1 << bit);
> +	} else if (type & IRQ_TYPE_LEVEL_LOW) {
> +		level &= ~(1 << bit);
> +		polarity &= ~(1 << bit);
> +	}
> +
> +	writel(level, gpio->regs + INT_TYPE_REG_OFFS);
> +	writel(polarity, gpio->regs + INT_POLARITY_REG_OFFS);
> +
> +	return 0;
> +}
> +
> +static int dwapb_create_irqchip(struct dwapb_gpio *gpio,
> +				struct dwapb_gpio_bank *bank,
> +				unsigned int irq_base)
> +{
> +	struct irq_chip_type *ct;
> +
> +	gpio->irq_gc = irq_alloc_generic_chip("gpio-dwapb", 1, irq_base,
> +					      gpio->regs, handle_level_irq);
> +	if (!gpio->irq_gc)
> +		return -EIO;
> +
> +	gpio->irq_gc->domain.of_node = of_node_get(bank->bgc.gc.of_node);
> +	gpio->irq_gc->private = gpio;
> +	ct = gpio->irq_gc->chip_types;
> +	ct->chip.irq_ack = irq_gc_ack_set_bit;
> +	ct->chip.irq_mask = irq_gc_mask_set_bit;
> +	ct->chip.irq_unmask = irq_gc_mask_clr_bit;
> +	ct->chip.irq_set_type = dwapb_irq_set_type;
> +	ct->chip.irq_enable = dwapb_irq_enable;
> +	ct->chip.irq_disable = dwapb_irq_disable;
> +	ct->regs.ack = EOI_REG_OFFS;
> +	ct->regs.mask = INT_MASK_REG_OFFS;
> +	irq_setup_generic_chip(gpio->irq_gc, IRQ_MSK(bank->bgc.gc.ngpio),
> +			       IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0);
> +
> +	return 0;
> +}
> +
> +static int dwapb_configure_irqs(struct dwapb_gpio *gpio,
> +				struct dwapb_gpio_bank *bank)
> +{
> +	unsigned int m, irq, ngpio = bank->bgc.gc.ngpio;
> +	int irq_base;
> +
> +	for (m = 0; m < ngpio; ++m) {
> +		irq = irq_of_parse_and_map(bank->bgc.gc.of_node, m);
> +		if (!irq && m == 0) {
> +			dev_warn(gpio->dev, "no irq for bank %s\n",
> +				 bank->bgc.gc.of_node->full_name);
> +			return -ENXIO;
> +		} else if (!irq) {
> +			break;
> +		}
> +
> +		irq_set_chained_handler(irq, dwapb_irq_handler);
> +		irq_set_handler_data(irq, gpio);
> +	}
> +	bank->bgc.gc.to_irq = dwapb_gpio_to_irq;
> +
> +	irq_base = irq_alloc_descs(-1, 0, ngpio, NUMA_NO_NODE);
> +	if (irq_base < 0)
> +		return irq_base;
> +
> +	if (dwapb_create_irqchip(gpio, bank, irq_base))
> +		goto out_free_descs;
> +
> +	return 0;
> +
> +out_free_descs:
> +	irq_free_descs(irq_base, ngpio);
> +
> +	return -EIO;
> +}
> +
> +static int dwapb_gpio_add_bank(struct dwapb_gpio *gpio,
> +			       struct device_node *bank_np)
> +{
> +	struct dwapb_gpio_bank *bank;
> +	u32 bank_idx, ngpio;
> +	int err;
> +
> +	if (of_property_read_u32(bank_np, "reg", &bank_idx)) {
> +		dev_err(gpio->dev, "invalid bank index for %s\n",
> +			bank_np->full_name);
> +		return -EINVAL;
> +	}
> +	bank = &gpio->banks[bank_idx];
> +	bank->gpio = gpio;
> +
> +	if (of_property_read_u32(bank_np, "nr-gpio", &ngpio)) {
> +		dev_err(gpio->dev, "failed to get number of gpios for %s\n",
> +			bank_np->full_name);
> +		return -EINVAL;
> +	}
> +
> +	bank->bank_idx = bank_idx;
> +	err = bgpio_init(&bank->bgc, gpio->dev, 4,
> +			 gpio->regs + 0x50 + (bank_idx * 0x4),
> +			 gpio->regs + 0x00 + (bank_idx * 0xc),
> +			 NULL, gpio->regs + 0x04 + (bank_idx * 0xc), NULL,
> +			 false);
> +	if (err) {
> +		dev_err(gpio->dev, "failed to init gpio chip for %s\n",
> +			bank_np->full_name);
> +		return err;
> +	}
> +
> +	bank->bgc.gc.ngpio = ngpio;
> +	bank->bgc.gc.of_node = bank_np;
> +
> +	/*
> +	 * Only bank A can provide interrupts in all configurations of the IP.
> +	 */
> +	if (bank_idx == 0 &&
> +	    of_get_property(bank_np, "interrupt-controller", NULL))
> +		dwapb_configure_irqs(gpio, bank);
> +
> +	err = gpiochip_add(&bank->bgc.gc);
> +	if (err)
> +		dev_err(gpio->dev, "failed to register gpiochip for %s\n",
> +			bank_np->full_name);
> +	else
> +		bank->is_registered = true;
> +
> +	return err;
> +}
> +
> +static void dwapb_gpio_unregister(struct dwapb_gpio *gpio)
> +{
> +	unsigned int m;
> +
> +	for (m = 0; m < gpio->nr_banks; ++m)
> +		if (gpio->banks[m].is_registered)
> +			gpiochip_remove(&gpio->banks[m].bgc.gc);
> +	of_node_put(gpio->of_node);
> +}
> +
> +static int __devinit dwapb_gpio_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct dwapb_gpio *gpio;
> +	struct device_node *np;
> +	int err;
> +
> +	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
> +	if (!gpio)
> +		return -ENOMEM;
> +	gpio->dev = &pdev->dev;
> +
> +	gpio->nr_banks = dwapb_gpio_nr_banks(pdev->dev.of_node);
> +	if (!gpio->nr_banks)
> +		return -EINVAL;
> +	gpio->banks = devm_kzalloc(&pdev->dev, gpio->nr_banks *
> +				   sizeof(*gpio->banks), GFP_KERNEL);
> +	if (!gpio->banks)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "failed to get iomem\n");
> +		return -ENXIO;
> +	}
> +	gpio->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +	if (!gpio->regs)
> +		return -ENOMEM;
> +
> +	gpio->of_node = of_node_get(pdev->dev.of_node);
> +	for_each_child_of_node(pdev->dev.of_node, np) {
> +		err = dwapb_gpio_add_bank(gpio, np);
> +		if (err)
> +			goto out_unregister;
> +	}
> +	platform_set_drvdata(pdev, gpio);
> +
> +	return 0;
> +
> +out_unregister:
> +	dwapb_gpio_unregister(gpio);
> +
> +	return err;
> +}
> +
> +static const struct of_device_id dwapb_of_match_table[] = {
> +	{ .compatible = "snps,dw-apb-gpio" },
> +	{ /* Sentinel */ }
> +};
> +
> +static struct platform_driver dwapb_gpio_driver = {
> +	.driver		= {
> +		.name	= "gpio-dwapb",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = dwapb_of_match_table,
> +	},
> +	.probe		= dwapb_gpio_probe,
> +};
> +
> +static int __init dwapb_gpio_init(void)
> +{
> +	return platform_driver_register(&dwapb_gpio_driver);
> +}
> +postcore_initcall(dwapb_gpio_init);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jamie Iles");
> +MODULE_DESCRIPTION("Synopsys DesignWare APB GPIO driver");

Re: [PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Jamie Iles]

On Tue, Dec 20, 2011 at 09:44:42AM -0600, Rob Herring wrote:
> Jamie,
> 
> On 12/19/2011 07:43 PM, Jamie Iles wrote:
> > The Synopsys DesignWare block is used in some ARM devices (picoxcell)
> > and can be configured to provide multiple banks of GPIO pins.  The first
> > bank (A) can also provide IRQ capabilities.
> > 
> > v2:	- use Rob Herring's irqdomain in generic irq chip patches
> > 	- use reg property to indicate bank index
> > 	- support irqs on both edges based on LinusW's u300 driver
> > 
> > Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> > Cc: Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
> > Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> > ---
[...]
> > +++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> > @@ -0,0 +1,63 @@
> > +* Synopsys DesignWare APB GPIO controller
> > +
> > +Required properties:
> > +- compatible : Should be "snps,dw-apb-gpio"
> > +- reg : Address and length of the register set for the device
> > +
> > +The GPIO controller has a configurable number of banks, each of which are
> > +represented as child nodes with the following properties:
> > +
> > +Required properties:
> > +- compatible : "snps,dw-apb-gpio-bank"
> > +- gpio-controller : Marks the device node as a gpio controller.
> > +- #gpio-cells : Should be two.  The first cell is the pin number and
> > +  the second cell is used to specify optional parameters (currently
> > +  unused).
> > +- reg : The integer bank index of the bank, a single cell.
> > +- nr-gpio : The number of pins in the bank, a single cell.
[...]
> > +gpio: gpio@20000 {
> > +	compatible = "snps,dw-apb-gpio";
> > +	reg = <0x20000 0x1000>;
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	banka: gpio-controller@0 {
> > +		compatible = "snps,dw-apb-gpio-bank";
> > +		gpio-controller;
> > +		#gpio-cells = <2>;
> > +		nr-gpio = <8>;
> > +		reg = <0>;
> 
> Actually, what I meant was to use the bank addresses here. Sorry that
> wasn't clear. Then you don't need a calculation of bank offsets.

Unfortunately the hardware doesn't have a nice register map where the 
registers for each bank are grouped together and they're all interleaved 
so it's a bit of a mess really. 

Some of the picoxcell devices have configurations like 8 pins on bank A, 
16 on bank B then 20 or so on bank D, so I do think it makes sense to 
define the banks as individual nodes and it fits in with the 
programming model.

Jamie

Re: [PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Rob Herring]

On 12/20/2011 09:54 AM, Jamie Iles wrote:
> On Tue, Dec 20, 2011 at 09:44:42AM -0600, Rob Herring wrote:
>> Jamie,
>>
>> On 12/19/2011 07:43 PM, Jamie Iles wrote:
>>> The Synopsys DesignWare block is used in some ARM devices (picoxcell)
>>> and can be configured to provide multiple banks of GPIO pins.  The first
>>> bank (A) can also provide IRQ capabilities.
>>>
>>> v2:	- use Rob Herring's irqdomain in generic irq chip patches
>>> 	- use reg property to indicate bank index
>>> 	- support irqs on both edges based on LinusW's u300 driver
>>>
>>> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>>> Cc: Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
>>> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>>> Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
>>> ---
> [...]
>>> +++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
>>> @@ -0,0 +1,63 @@
>>> +* Synopsys DesignWare APB GPIO controller
>>> +
>>> +Required properties:
>>> +- compatible : Should be "snps,dw-apb-gpio"
>>> +- reg : Address and length of the register set for the device
>>> +
>>> +The GPIO controller has a configurable number of banks, each of which are
>>> +represented as child nodes with the following properties:
>>> +
>>> +Required properties:
>>> +- compatible : "snps,dw-apb-gpio-bank"
>>> +- gpio-controller : Marks the device node as a gpio controller.
>>> +- #gpio-cells : Should be two.  The first cell is the pin number and
>>> +  the second cell is used to specify optional parameters (currently
>>> +  unused).
>>> +- reg : The integer bank index of the bank, a single cell.
>>> +- nr-gpio : The number of pins in the bank, a single cell.
> [...]
>>> +gpio: gpio@20000 {
>>> +	compatible = "snps,dw-apb-gpio";
>>> +	reg = <0x20000 0x1000>;
>>> +	#address-cells = <1>;
>>> +	#size-cells = <0>;
>>> +
>>> +	banka: gpio-controller@0 {
>>> +		compatible = "snps,dw-apb-gpio-bank";
>>> +		gpio-controller;
>>> +		#gpio-cells = <2>;
>>> +		nr-gpio = <8>;
>>> +		reg = <0>;
>>
>> Actually, what I meant was to use the bank addresses here. Sorry that
>> wasn't clear. Then you don't need a calculation of bank offsets.
> 
> Unfortunately the hardware doesn't have a nice register map where the 
> registers for each bank are grouped together and they're all interleaved 
> so it's a bit of a mess really. 
> 
> Some of the picoxcell devices have configurations like 8 pins on bank A, 
> 16 on bank B then 20 or so on bank D, so I do think it makes sense to 
> define the banks as individual nodes and it fits in with the 
> programming model.
> 

Ah. I should have looked more closely at the register layout... From a
DT binding standpoint:

Acked-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

Rob

Re: [PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Jamie Iles]

Grant, Linus,

On Tue, Dec 20, 2011 at 01:43:50AM +0000, Jamie Iles wrote:
> The Synopsys DesignWare block is used in some ARM devices (picoxcell)
> and can be configured to provide multiple banks of GPIO pins.  The first
> bank (A) can also provide IRQ capabilities.
> 
> v2:	- use Rob Herring's irqdomain in generic irq chip patches
> 	- use reg property to indicate bank index
> 	- support irqs on both edges based on LinusW's u300 driver
> 
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Cc: Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>

Is this driver okay by you guys?  I'd like to get this one in for 3.3 if 
at all possible as it's the last patch that allows picoxcell to boot to 
a semi useful state without out of tree patches and that would make 
testing a lot easier!

Thanks,

Jamie

Re: [PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Rob Herring]

On 12/30/2011 01:59 PM, Jamie Iles wrote:
> Grant, Linus,
> 
> On Tue, Dec 20, 2011 at 01:43:50AM +0000, Jamie Iles wrote:
>> The Synopsys DesignWare block is used in some ARM devices (picoxcell)
>> and can be configured to provide multiple banks of GPIO pins.  The first
>> bank (A) can also provide IRQ capabilities.
>>
>> v2:	- use Rob Herring's irqdomain in generic irq chip patches
>> 	- use reg property to indicate bank index
>> 	- support irqs on both edges based on LinusW's u300 driver
>>
>> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> Cc: Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
>> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> 
> Is this driver okay by you guys?  I'd like to get this one in for 3.3 if 
> at all possible as it's the last patch that allows picoxcell to boot to 
> a semi useful state without out of tree patches and that would make 
> testing a lot easier!

We've got to sort out getting irq domains enabled on x86 and MIPS in
order to merge generic irq chip using domains. I'm not sure that will
get done in time.

Can you leave out the interrupt portion of the driver? Or that is needed
to get to a useful state.

Rob

Re: [PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Jamie Iles]

On Fri, Dec 30, 2011 at 02:25:25PM -0600, Rob Herring wrote:
> On 12/30/2011 01:59 PM, Jamie Iles wrote:
> > Grant, Linus,
> > 
> > On Tue, Dec 20, 2011 at 01:43:50AM +0000, Jamie Iles wrote:
> >> The Synopsys DesignWare block is used in some ARM devices (picoxcell)
> >> and can be configured to provide multiple banks of GPIO pins.  The first
> >> bank (A) can also provide IRQ capabilities.
> >>
> >> v2:	- use Rob Herring's irqdomain in generic irq chip patches
> >> 	- use reg property to indicate bank index
> >> 	- support irqs on both edges based on LinusW's u300 driver
> >>
> >> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> >> Cc: Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
> >> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> >> Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> > 
> > Is this driver okay by you guys?  I'd like to get this one in for 3.3 if 
> > at all possible as it's the last patch that allows picoxcell to boot to 
> > a semi useful state without out of tree patches and that would make 
> > testing a lot easier!
> 
> We've got to sort out getting irq domains enabled on x86 and MIPS in
> order to merge generic irq chip using domains. I'm not sure that will
> get done in time.
> 
> Can you leave out the interrupt portion of the driver? Or that is needed
> to get to a useful state.

Sure, the GPIO is only needed for the NAND interface so the IRQ portion 
can easily be dropped.  I'll respin the patch just doing basic GPIO but 
keep the binding details of the interrupt stuff as I can't see that it 
should change.

Thanks,

Jamie

Re: [PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Mark Brown]

On Fri, Dec 30, 2011 at 02:25:25PM -0600, Rob Herring wrote:

> We've got to sort out getting irq domains enabled on x86 and MIPS in
> order to merge generic irq chip using domains. I'm not sure that will
> get done in time.

> Can you leave out the interrupt portion of the driver? Or that is needed
> to get to a useful state.

Or just add a dependency in Kconfig?

Re: [PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Jamie Iles]

On Mon, Jan 02, 2012 at 12:14:28PM +0000, Mark Brown wrote:
> On Fri, Dec 30, 2011 at 02:25:25PM -0600, Rob Herring wrote:
> 
> > We've got to sort out getting irq domains enabled on x86 and MIPS in
> > order to merge generic irq chip using domains. I'm not sure that will
> > get done in time.
> 
> > Can you leave out the interrupt portion of the driver? Or that is needed
> > to get to a useful state.
> 
> Or just add a dependency in Kconfig?

Yeah, I just saw a post from Grant on another topic where he mentioned 
that 'select IRQ_DOMAIN' is the wrong thing anyway so I should introduce 
a dependency on IRQ_DOMAIN.

The IRQ part is dependent on Rob's patches to introduce irq domain 
support into the generic chip though, so how about I split this into two 
patches, one that does the basic GPIO stuff and another that does the 
IRQ portion (with the dependency change) and if Rob's patches don't make 
it in we drop the IRQ support for the GPIO?

Thanks,

Jamie

Re: [PATCHv2 1/2] gpio: add a driver for the Synopsys DesignWare APB GPIO block

[Mark Brown]

On Mon, Jan 02, 2012 at 12:24:00PM +0000, Jamie Iles wrote:

> The IRQ part is dependent on Rob's patches to introduce irq domain 
> support into the generic chip though, so how about I split this into two 
> patches, one that does the basic GPIO stuff and another that does the 
> IRQ portion (with the dependency change) and if Rob's patches don't make 
> it in we drop the IRQ support for the GPIO?

Yeah, epecially since the IRQ domain stuff is still giving unbootable
kernels in -next as far as I know.