[PATCHv3 0/3] Synopsys DesignWare GPIO support

[PATCHv3 0/3] Synopsys DesignWare GPIO support

[Jamie Iles]

Pretty much the same as last time with the exception that I've split the
IRQ portion from the GPIO bit and added a dependency on IRQ_DOMAIN
rather than selecting it.

The 2nd patch is dependent on Rob's irqdomain support for the generic
irq chip so can be dropped if needed.

Jamie Iles (3):
  gpio: add a driver for the Synopsys DesignWare APB GPIO block
  gpio: dwapb: add support for GPIO interrupts
  ARM: picoxcell: use new Synopsys Designware GPIO binding

 .../devicetree/bindings/gpio/snps-dwapb-gpio.txt   |   63 ++++
 arch/arm/boot/dts/picoxcell-pc3x2.dtsi             |   19 +-
 arch/arm/boot/dts/picoxcell-pc3x3.dtsi             |   28 +-
 drivers/gpio/Kconfig                               |    9 +
 drivers/gpio/Makefile                              |    1 +
 drivers/gpio/gpio-dwapb.c                          |  351 ++++++++++++++++++++
 6 files changed, 443 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
 create mode 100644 drivers/gpio/gpio-dwapb.c

-- 
1.7.5.4

[PATCHv3 1/3] 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.

v3:	- depend on rather than select IRQ_DOMAIN
	- split IRQ support into a separate patch
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>
Acked-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
---
 .../devicetree/bindings/gpio/snps-dwapb-gpio.txt   |   63 +++++++
 drivers/gpio/Kconfig                               |    9 +
 drivers/gpio/Makefile                              |    1 +
 drivers/gpio/gpio-dwapb.c                          |  170 ++++++++++++++++++++
 4 files changed, 243 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..dccc113
--- /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..a82f61e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -85,6 +85,15 @@ 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
+	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..fb98274
--- /dev/null
+++ b/drivers/gpio/gpio-dwapb.c
@@ -0,0 +1,170 @@
+/*
+ * 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/module.h>
+#include <linux/basic_mmio_gpio.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+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;
+};
+
+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_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;
+
+	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)
+			WARN_ON(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

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

[Mark Brown]

On Mon, Jan 02, 2012 at 12:53:16PM +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.
> 
> v3:	- depend on rather than select IRQ_DOMAIN
> 	- split IRQ support into a separate patch
> 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

Put stuff like this after the ---, it shouldn't end up in git history.

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

This looks like a straight mapping of the Linux constants to device
tree.  This seems sensible and reasonable and since we're forced to use
magic numbers by the binding it'd be really good if we could standardise
on using this for new drivers to reduce the pain for people writing and
reading device tree bindings.  To help with that could you factor this
out into a separate document that other drivers can reference?

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

[Linus Walleij]

On Mon, Jan 2, 2012 at 2:25 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

> On Mon, Jan 02, 2012 at 12:53:16PM +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.
>>
>> v3:   - depend on rather than select IRQ_DOMAIN
>>       - split IRQ support into a separate patch
>> 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
>
> Put stuff like this after the ---, it shouldn't end up in git history.

The other day I was being told the exact opposite by Grant
Likely, so which one is it?

This was however for a totally new driver.

I think Grants request made sense since the gitlog should
contain the changelog and the more the better, but what do I
know.

Yours,
Linus Walleij

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

[Grant Likely]

On Mon, Jan 02, 2012 at 02:48:40PM +0100, Linus Walleij wrote:
> On Mon, Jan 2, 2012 at 2:25 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> 
> > On Mon, Jan 02, 2012 at 12:53:16PM +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.
> >>
> >> v3:   - depend on rather than select IRQ_DOMAIN
> >>       - split IRQ support into a separate patch
> >> 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
> >
> > Put stuff like this after the ---, it shouldn't end up in git history.
> 
> The other day I was being told the exact opposite by Grant
> Likely, so which one is it?

It's the preference of whatever maintainer will actually be picking up
the patch.  :-)  There is no definitive answer.

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

[Jamie Iles]

Hi Mark,

On Mon, Jan 02, 2012 at 01:25:00PM +0000, Mark Brown wrote:
> On Mon, Jan 02, 2012 at 12:53:16PM +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.
> > 
> > v3:	- depend on rather than select IRQ_DOMAIN
> > 	- split IRQ support into a separate patch
> > 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
> 
> Put stuff like this after the ---, it shouldn't end up in git history.

Grant (and others I believe) have asked to have it above the separator 
in the past to ensure that it does end up in the history.  This helps 
make it clear what version got applied and it's a lot easier for patch 
authors to manage rather than keeping it out of band.

> > +- #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
> 
> This looks like a straight mapping of the Linux constants to device
> tree.  This seems sensible and reasonable and since we're forced to use
> magic numbers by the binding it'd be really good if we could standardise
> on using this for new drivers to reduce the pain for people writing and
> reading device tree bindings.  To help with that could you factor this
> out into a separate document that other drivers can reference?

Something like this?

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index 9b4b82a..d13f7ce 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -24,12 +24,8 @@ Main node required properties:
   SPI interrupts are in the range [0-987].  PPI interrupts are in the
   range [0-15].
 
-  The 3rd cell is the flags, encoded as follows:
-	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
+  The 3rd cell is the flags, encoded as the trigger masks from
+  Documentation/devicetree/bindings/interrupts.txt and:
 	bits[15:8] PPI interrupt cpu mask.  Each bit corresponds to each of
 	the 8 possible cpus attached to the GIC.  A bit set to '1' indicated
 	the interrupt is wired to that CPU.  Only valid for PPI interrupts.
diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
index dccc113..73adf37 100644
--- a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
@@ -21,14 +21,8 @@ Optional properties:
 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
-
+the second encodes the triger flags encoded as described in
+Documentation/devicetree/bindings/interrupts.txt
 - interrupt-parent : The parent interrupt controller.
 - interrupts : The interrupts to the parent controller raised when GPIOs
 generate the interrupts.
diff --git a/Documentation/devicetree/bindings/interrupts.txt b/Documentation/devicetree/bindings/interrupts.txt
new file mode 100644
index 0000000..1545941
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupts.txt
@@ -0,0 +1,17 @@
+Common Interrupt Binding Details
+
+For controllers that need to encode trigger types and senses, where possible
+it is encouraged to use the following encoding (a direct mapping of the
+IRQF_TRIGGER_* constants in include/linux/interrupt.h):
+
+	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
+
+For example, an interrupt may be encoded as (with #interrupt-cells = <2>):
+
+	interrupts = <4 0x3>;
+
+to have interrupt 4 raise an interrupt on both edges of the input.

Jamie

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

[Mark Brown]

On Mon, Jan 02, 2012 at 01:51:45PM +0000, Jamie Iles wrote:
> On Mon, Jan 02, 2012 at 01:25:00PM +0000, Mark Brown wrote:

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

> > Put stuff like this after the ---, it shouldn't end up in git history.

> Grant (and others I believe) have asked to have it above the separator 
> in the past to ensure that it does end up in the history.  This helps 
> make it clear what version got applied and it's a lot easier for patch 
> authors to manage rather than keeping it out of band.

You end up with content-free junk in the changelogs like "v2: rewrite
with new API" (I've actually had that...) which just isn't instructive
to anyone reading the logs later; if there were some way of going from
the changelog back to the actual code that'd be one thing but then
there's a reason why we don't carry the out of tree revision control
history.

Note that patch changelogs are one of the specific examples given in
SubmittingPatches for inclusion after the ---.

Personally I'm not convinced carrying any out of tree change history
except the delta from the last version submitted is useful as the actual
code changes aren't readily available and the fact that there were
earlier versions with bugs or whatever isn't terribly instructive.  The
delta from the last version is useful for helping to focus review but
the rest of it gets very noisy.  If the history is really useful
probably merging an actual branch with is much more helpful, if it's
getting troublesome to manage it and that's not suitable then I'd just
drop it.

> Something like this?

Yes, exactly thanks.

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

[Grant Likely]

On Mon, Jan 02, 2012 at 01:25:00PM +0000, Mark Brown wrote:
> On Mon, Jan 02, 2012 at 12:53:16PM +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.
> > 
> > v3:	- depend on rather than select IRQ_DOMAIN
> > 	- split IRQ support into a separate patch
> > 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
> 
> Put stuff like this after the ---, it shouldn't end up in git history.

I disagree.  Keeping the revision history in the git commit is
actually quite useful when trying to figure out which version of a
patch actually got applied.  I'm asking patch authors to keep it above
the ---

g.

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

[Mark Brown]

On Wed, Jan 04, 2012 at 11:21:33AM -0700, Grant Likely wrote:
> On Mon, Jan 02, 2012 at 01:25:00PM +0000, Mark Brown wrote:

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

> > Put stuff like this after the ---, it shouldn't end up in git history.

> I disagree.  Keeping the revision history in the git commit is
> actually quite useful when trying to figure out which version of a
> patch actually got applied.  I'm asking patch authors to keep it above
> the ---

I wouldn't be completely opposed to keeping *a* version in, the thing
that gets me is the entire changelog - it gets really noisy quite
quickly and the history for the patch ends up dominating the changelog.
The older entries normally have vanishingly little utility without the
changes (and where they do should normally be written into the actual
description of the change) and all you need to know which version got
applied is information on the most recent revision.

But TBH you could also get the version by just putting it in the subject
outside of [ ] so it doesn't get stripped.

[PATCHv3 2/3] gpio: dwapb: add support for GPIO interrupts

[Jamie Iles]

The controller supports interrupts on bank A (up to 8 interrupt
sources).  Use the generic IRQ chip to implement interrupt support for
this bank.

Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Linus Walleij <linus.walleij@stericsson.com>
Acked-by: Rob Herring <rob.herring@calxeda.com>
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 drivers/gpio/Kconfig      |    2 +-
 drivers/gpio/gpio-dwapb.c |  181 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 182 insertions(+), 1 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a82f61e..86967ea 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -89,7 +89,7 @@ config GPIO_DWAPB
 	bool "Synopsys DesignWare APB GPIO driver"
 	select GPIO_GENERIC
 	select GENERIC_IRQ_CHIP
-	depends on OF_GPIO
+	depends on OF_GPIO && IRQ_DOMAIN
 	help
 	  Say Y or M here to build support for the Synopsys DesignWare APB
 	  GPIO block.  This requires device tree support.
diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index fb98274..c9d0aac 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -10,12 +10,22 @@
 #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 {
@@ -31,6 +41,8 @@ struct dwapb_gpio {
 	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)
@@ -44,6 +56,168 @@ static unsigned int dwapb_gpio_nr_banks(struct device_node *of_node)
 	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)
 {
@@ -80,6 +254,13 @@ static int dwapb_gpio_add_bank(struct dwapb_gpio *gpio,
 	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",
-- 
1.7.5.4

[PATCHv3 3/3] ARM: picoxcell: use new Synopsys Designware GPIO binding

[Jamie Iles]

Use the DesignWare specific binding rather than the generic binding
which isn't supported in mainline.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 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