[PATCH v4 0/4] GPIO support for BRCMSTB

[PATCH v4 0/4] GPIO support for BRCMSTB

[Gregory Fong]

Adds interrupt support for the GPIO controller (UPG GIO) used on Broadcom's
various BRCMSTB SoCs (BCM7XXX and others).  For all existing hardware, this
block hooks up to the BCM7120 L2 IRQ controller and so will require
CONFIG_BCM7120_L2_IRQ=y.

New in v4:
- add nodes for the BRCMSTB GPIO controller to the BCM7445 dts file
- a few improvements suggested by Linus Walleij on v3
- remove unused 'irq' argument to brcmstb_gpio_irq_bank_handler()

The following are not included in this patchset:
- Initial device tree bindings (merged from v1 to GPIO tree)
- Initial GPIO support w/o interrupts (merged from v2 to GPIO tree)
- ARM Kconfig changes (merged from v2 to arm-soc tree)
- fix for null ptr deref in driver remove (merged from v3)

Previous versions:
v1: https://lkml.org/lkml/2015/5/6/199
v2: https://lkml.org/lkml/2015/5/28/853
v3: https://lkml.org/lkml/2015/6/17/960

Gregory Fong (4):
  dt-bindings: brcmstb-gpio: document properties for wakeup
  gpio: brcmstb: Add interrupt and wakeup source support
  gpio: brcmstb: support wakeup from S5 cold boot
  ARM: dts: brcmstb: add BCM7445 GPIO nodes

 .../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt |  35 ++-
 arch/arm/boot/dts/bcm7445.dtsi                     |  50 ++++
 drivers/gpio/Kconfig                               |   1 +
 drivers/gpio/gpio-brcmstb.c                        | 306 ++++++++++++++++++++-
 4 files changed, 379 insertions(+), 13 deletions(-)

-- 
1.9.1

[PATCH v4 1/4] dt-bindings: brcmstb-gpio: document properties for wakeup

[Gregory Fong]

Some brcmstb GPIO controllers can be used to wake from suspend, so use
the de facto standard property 'wakeup-source' to mark the nodes of
controllers with that capability.

Also document interrupts-extended, which will be used for wakeup
handling because the interrupt parent for the wake IRQ is different
from the regular IRQ.

While we're at it, a few more fixes: We don't actually use the
"interrupt-names" property, so remove it from the listed optional
properties and from the examples.  And since we're modifying the
examples, also follow Brian's suggestions to:
- change #gpio-cells, #interrupt-cells, and brcm,gpio-bank-widths from
  hex to dec
- use phandles

Reviewed-by: Brian Norris <computersforpeace@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>
---
v4: no changes from v3

 .../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt | 35 +++++++++++++++++-----
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
index 435f1bc..b405b44 100644
--- a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.txt
@@ -33,6 +33,13 @@ Optional properties:
 - interrupt-parent:
     phandle of the parent interrupt controller
 
+- interrupts-extended:
+    Alternate form of specifying interrupts and parents that allows for
+    multiple parents.  This takes precedence over 'interrupts' and
+    'interrupt-parent'.  Wakeup-capable GPIO controllers often route their
+    wakeup interrupt lines through a different interrupt controller than the
+    primary interrupt line, making this property necessary.
+
 - #interrupt-cells:
     Should be <2>.  The first cell is the GPIO number, the second should specify
     flags.  The following subset of flags is supported:
@@ -47,19 +54,33 @@ Optional properties:
 - interrupt-controller:
     Marks the device node as an interrupt controller
 
-- interrupt-names:
-    The name of the IRQ resource used by this controller
+- wakeup-source:
+    GPIOs for this controller can be used as a wakeup source
 
 Example:
 	upg_gio: gpio at f040a700 {
-		#gpio-cells = <0x2>;
-		#interrupt-cells = <0x2>;
+		#gpio-cells = <2>;
+		#interrupt-cells = <2>;
 		compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
 		gpio-controller;
 		interrupt-controller;
 		reg = <0xf040a700 0x80>;
-		interrupt-parent = <0xf>;
+		interrupt-parent = <&irq0_intc>;
+		interrupts = <0x6>;
+		brcm,gpio-bank-widths = <32 32 32 24>;
+	};
+
+	upg_gio_aon: gpio at f04172c0 {
+		#gpio-cells = <2>;
+		#interrupt-cells = <2>;
+		compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
+		gpio-controller;
+		interrupt-controller;
+		reg = <0xf04172c0 0x40>;
+		interrupt-parent = <&irq0_aon_intc>;
 		interrupts = <0x6>;
-		interrupt-names = "upg_gio";
-		brcm,gpio-bank-widths = <0x20 0x20 0x20 0x18>;
+		interrupts-extended = <&irq0_aon_intc 0x6>,
+			<&aon_pm_l2_intc 0x5>;
+		wakeup-source;
+		brcm,gpio-bank-widths = <18 4>;
 	};
-- 
1.9.1

[PATCH v4 1/4] dt-bindings: brcmstb-gpio: document properties for wakeup

[Linus Walleij]

On Sat, Aug 1, 2015 at 3:17 AM, Gregory Fong <gregory.0xf0@gmail.com> wrote:

> Some brcmstb GPIO controllers can be used to wake from suspend, so use
> the de facto standard property 'wakeup-source' to mark the nodes of
> controllers with that capability.
>
> Also document interrupts-extended, which will be used for wakeup
> handling because the interrupt parent for the wake IRQ is different
> from the regular IRQ.
>
> While we're at it, a few more fixes: We don't actually use the
> "interrupt-names" property, so remove it from the listed optional
> properties and from the examples.  And since we're modifying the
> examples, also follow Brian's suggestions to:
> - change #gpio-cells, #interrupt-cells, and brcm,gpio-bank-widths from
>   hex to dec
> - use phandles
>
> Reviewed-by: Brian Norris <computersforpeace@gmail.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>
> ---
> v4: no changes from v3

I've already applied this, no action.

Yours,
Linus Walleij

[PATCH v4 2/4] gpio: brcmstb: Add interrupt and wakeup source support

[Gregory Fong]

Uses the gpiolib irqchip helpers.  For this to work, the irq setup
function is called once per bank instead of once per device.  Note
that all known uses of this block have a BCM7120 L2 interrupt
controller as a parent.  Supports interrupts for all GPIOs.

In the IRQ handler, we check for raised IRQs for invalid GPIOs and
warn (ratelimited) if they're encountered.

Also, several drivers (e.g. gpio-keys) allow for GPIOs to be
configured as wakeup sources, and this GPIO controller supports that
through a separate interrupt path.

The de-facto standard DT property "wakeup-source" is checked, since
that indicates whether the GPIO controller hardware can wake.  Uses
the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have
any of its own wakeup source configuration.

Aside regarding gpiolib irqchip helpers: It wasn't obvious (to me)
that you can have multiple chained irqchips and associated IRQ domains
for a single parent IRQ, and as long as the xlate function is written
correctly, a GPIO IRQ request end up checking the correct domain and
will get associated with the correct IRQ.  What helps make this clear
is to read
  drivers/gpio/gpiolib-of.c:
   - of_gpiochip_find_and_xlate()
   - of_get_named_gpiod_flags()
  drivers/gpio/gpiolib.c:
   - gpiochip_find()

Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>
---
v4:
- when checking parent_irq, use <= 0 or > 0 since 0 is NO_IRQ.

 drivers/gpio/Kconfig        |   1 +
 drivers/gpio/gpio-brcmstb.c | 262 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 257 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8f1fe73..0b77175 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -131,6 +131,7 @@ config GPIO_BRCMSTB
 	default y if ARCH_BRCMSTB
 	depends on OF_GPIO && (ARCH_BRCMSTB || COMPILE_TEST)
 	select GPIO_GENERIC
+	select GPIOLIB_IRQCHIP
 	help
 	  Say yes here to enable GPIO support for Broadcom STB (BCM7XXX) SoCs.
 
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 4630a81..46cc4e9 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -17,6 +17,9 @@
 #include <linux/of_irq.h>
 #include <linux/module.h>
 #include <linux/basic_mmio_gpio.h>
+#include <linux/irqdomain.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/interrupt.h>
 
 #define GIO_BANK_SIZE           0x20
 #define GIO_ODEN(bank)          (((bank) * GIO_BANK_SIZE) + 0x00)
@@ -34,14 +37,17 @@ struct brcmstb_gpio_bank {
 	struct bgpio_chip bgc;
 	struct brcmstb_gpio_priv *parent_priv;
 	u32 width;
+	struct irq_chip irq_chip;
 };
 
 struct brcmstb_gpio_priv {
 	struct list_head bank_list;
 	void __iomem *reg_base;
-	int num_banks;
 	struct platform_device *pdev;
+	int parent_irq;
 	int gpio_base;
+	bool can_wake;
+	int parent_wake_irq;
 };
 
 #define MAX_GPIO_PER_BANK           32
@@ -63,6 +69,183 @@ brcmstb_gpio_gc_to_priv(struct gpio_chip *gc)
 	return bank->parent_priv;
 }
 
+static void brcmstb_gpio_set_imask(struct brcmstb_gpio_bank *bank,
+		unsigned int offset, bool enable)
+{
+	struct bgpio_chip *bgc = &bank->bgc;
+	struct brcmstb_gpio_priv *priv = bank->parent_priv;
+	u32 mask = bgc->pin2mask(bgc, offset);
+	u32 imask;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bgc->lock, flags);
+	imask = bgc->read_reg(priv->reg_base + GIO_MASK(bank->id));
+	if (enable)
+		imask |= mask;
+	else
+		imask &= ~mask;
+	bgc->write_reg(priv->reg_base + GIO_MASK(bank->id), imask);
+	spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+/* -------------------- IRQ chip functions -------------------- */
+
+static void brcmstb_gpio_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct brcmstb_gpio_bank *bank = brcmstb_gpio_gc_to_bank(gc);
+
+	brcmstb_gpio_set_imask(bank, d->hwirq, false);
+}
+
+static void brcmstb_gpio_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct brcmstb_gpio_bank *bank = brcmstb_gpio_gc_to_bank(gc);
+
+	brcmstb_gpio_set_imask(bank, d->hwirq, true);
+}
+
+static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct brcmstb_gpio_bank *bank = brcmstb_gpio_gc_to_bank(gc);
+	struct brcmstb_gpio_priv *priv = bank->parent_priv;
+	u32 mask = BIT(d->hwirq);
+	u32 edge_insensitive, iedge_insensitive;
+	u32 edge_config, iedge_config;
+	u32 level, ilevel;
+	unsigned long flags;
+
+	switch (type) {
+	case IRQ_TYPE_LEVEL_LOW:
+		level = 0;
+		edge_config = 0;
+		edge_insensitive = 0;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		level = mask;
+		edge_config = 0;
+		edge_insensitive = 0;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		level = 0;
+		edge_config = 0;
+		edge_insensitive = 0;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		level = 0;
+		edge_config = mask;
+		edge_insensitive = 0;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		level = 0;
+		edge_config = 0;  /* don't care, but want known value */
+		edge_insensitive = mask;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&bank->bgc.lock, flags);
+
+	iedge_config = bank->bgc.read_reg(priv->reg_base +
+			GIO_EC(bank->id)) & ~mask;
+	iedge_insensitive = bank->bgc.read_reg(priv->reg_base +
+			GIO_EI(bank->id)) & ~mask;
+	ilevel = bank->bgc.read_reg(priv->reg_base +
+			GIO_LEVEL(bank->id)) & ~mask;
+
+	bank->bgc.write_reg(priv->reg_base + GIO_EC(bank->id),
+			iedge_config | edge_config);
+	bank->bgc.write_reg(priv->reg_base + GIO_EI(bank->id),
+			iedge_insensitive | edge_insensitive);
+	bank->bgc.write_reg(priv->reg_base + GIO_LEVEL(bank->id),
+			ilevel | level);
+
+	spin_unlock_irqrestore(&bank->bgc.lock, flags);
+	return 0;
+}
+
+static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
+	int ret = 0;
+
+	/*
+	 * Only enable wake IRQ once for however many hwirqs can wake
+	 * since they all use the same wake IRQ.  Mask will be set
+	 * up appropriately thanks to IRQCHIP_MASK_ON_SUSPEND flag.
+	 */
+	if (enable)
+		ret = enable_irq_wake(priv->parent_wake_irq);
+	else
+		ret = disable_irq_wake(priv->parent_wake_irq);
+	if (ret)
+		dev_err(&priv->pdev->dev, "failed to %s wake-up interrupt\n",
+				enable ? "enable" : "disable");
+	return ret;
+}
+
+static irqreturn_t brcmstb_gpio_wake_irq_handler(int irq, void *data)
+{
+	struct brcmstb_gpio_priv *priv = data;
+
+	if (!priv || irq != priv->parent_wake_irq)
+		return IRQ_NONE;
+	pm_wakeup_event(&priv->pdev->dev, 0);
+	return IRQ_HANDLED;
+}
+
+static void brcmstb_gpio_irq_bank_handler(struct brcmstb_gpio_bank *bank)
+{
+	struct brcmstb_gpio_priv *priv = bank->parent_priv;
+	struct irq_domain *irq_domain = bank->bgc.gc.irqdomain;
+	void __iomem *reg_base = priv->reg_base;
+	unsigned long status;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bank->bgc.lock, flags);
+	while ((status = bank->bgc.read_reg(reg_base + GIO_STAT(bank->id)) &
+			 bank->bgc.read_reg(reg_base + GIO_MASK(bank->id)))) {
+		int bit;
+
+		for_each_set_bit(bit, &status, 32) {
+			u32 stat = bank->bgc.read_reg(reg_base +
+						      GIO_STAT(bank->id));
+			if (bit >= bank->width)
+				dev_warn(&priv->pdev->dev,
+					 "IRQ for invalid GPIO (bank=%d, offset=%d)\n",
+					 bank->id, bit);
+			bank->bgc.write_reg(reg_base + GIO_STAT(bank->id),
+					    stat | BIT(bit));
+			generic_handle_irq(irq_find_mapping(irq_domain, bit));
+		}
+	}
+	spin_unlock_irqrestore(&bank->bgc.lock, flags);
+}
+
+/* Each UPG GIO block has one IRQ for all banks */
+static void brcmstb_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
+	struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct list_head *pos;
+
+	/* Interrupts weren't properly cleared during probe */
+	BUG_ON(!priv || !chip);
+
+	chained_irq_enter(chip, desc);
+	list_for_each(pos, &priv->bank_list) {
+		struct brcmstb_gpio_bank *bank =
+			list_entry(pos, struct brcmstb_gpio_bank, node);
+		brcmstb_gpio_irq_bank_handler(bank);
+	}
+	chained_irq_exit(chip, desc);
+}
+
 /* Make sure that the number of banks matches up between properties */
 static int brcmstb_gpio_sanity_check_banks(struct device *dev,
 		struct device_node *np, struct resource *res)
@@ -100,7 +283,7 @@ static int brcmstb_gpio_remove(struct platform_device *pdev)
 		bank = list_entry(pos, struct brcmstb_gpio_bank, node);
 		ret = bgpio_remove(&bank->bgc);
 		if (ret)
-			dev_err(&pdev->dev, "gpiochip_remove fail in cleanup");
+			dev_err(&pdev->dev, "gpiochip_remove fail in cleanup\n");
 	}
 	return ret;
 }
@@ -121,7 +304,7 @@ static int brcmstb_gpio_of_xlate(struct gpio_chip *gc,
 		return -EINVAL;
 
 	offset = gpiospec->args[0] - (gc->base - priv->gpio_base);
-	if (offset >= gc->ngpio)
+	if (offset >= gc->ngpio || offset < 0)
 		return -EINVAL;
 
 	if (unlikely(offset >= bank->width)) {
@@ -136,6 +319,55 @@ static int brcmstb_gpio_of_xlate(struct gpio_chip *gc,
 	return offset;
 }
 
+/* Before calling, must have bank->parent_irq set and gpiochip registered */
+static int brcmstb_gpio_irq_setup(struct platform_device *pdev,
+		struct brcmstb_gpio_bank *bank)
+{
+	struct brcmstb_gpio_priv *priv = bank->parent_priv;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+
+	bank->irq_chip.name = dev_name(dev);
+	bank->irq_chip.irq_mask = brcmstb_gpio_irq_mask;
+	bank->irq_chip.irq_unmask = brcmstb_gpio_irq_unmask;
+	bank->irq_chip.irq_set_type = brcmstb_gpio_irq_set_type;
+
+	/* Ensures that all non-wakeup IRQs are disabled at suspend */
+	bank->irq_chip.flags = IRQCHIP_MASK_ON_SUSPEND;
+
+	if (IS_ENABLED(CONFIG_PM_SLEEP) && !priv->can_wake &&
+			of_property_read_bool(np, "wakeup-source")) {
+		priv->parent_wake_irq = platform_get_irq(pdev, 1);
+		if (priv->parent_wake_irq < 0) {
+			dev_warn(dev,
+				"Couldn't get wake IRQ - GPIOs will not be able to wake from sleep");
+		} else {
+			int err = devm_request_irq(dev, priv->parent_wake_irq,
+					brcmstb_gpio_wake_irq_handler, 0,
+					"brcmstb-gpio-wake", priv);
+
+			if (err < 0) {
+				dev_err(dev, "Couldn't request wake IRQ");
+				return err;
+			}
+
+			device_set_wakeup_capable(dev, true);
+			device_wakeup_enable(dev);
+			priv->can_wake = true;
+		}
+	}
+
+	if (priv->can_wake)
+		bank->irq_chip.irq_set_wake = brcmstb_gpio_irq_set_wake;
+
+	gpiochip_irqchip_add(&bank->bgc.gc, &bank->irq_chip, 0,
+			handle_simple_irq, IRQ_TYPE_NONE);
+	gpiochip_set_chained_irqchip(&bank->bgc.gc, &bank->irq_chip,
+			priv->parent_irq, brcmstb_gpio_irq_handler);
+
+	return 0;
+}
+
 static int brcmstb_gpio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -146,6 +378,7 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 	struct property *prop;
 	const __be32 *p;
 	u32 bank_width;
+	int num_banks = 0;
 	int err;
 	static int gpio_base;
 
@@ -164,6 +397,16 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 	priv->reg_base = reg_base;
 	priv->pdev = pdev;
 
+	if (of_property_read_bool(np, "interrupt-controller")) {
+		priv->parent_irq = platform_get_irq(pdev, 0);
+		if (priv->parent_irq <= 0) {
+			dev_err(dev, "Couldn't get IRQ");
+			return -ENOENT;
+		}
+	} else {
+		priv->parent_irq = -ENOENT;
+	}
+
 	if (brcmstb_gpio_sanity_check_banks(dev, np, res))
 		return -EINVAL;
 
@@ -180,7 +423,7 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 		}
 
 		bank->parent_priv = priv;
-		bank->id = priv->num_banks;
+		bank->id = num_banks;
 		if (bank_width <= 0 || bank_width > MAX_GPIO_PER_BANK) {
 			dev_err(dev, "Invalid bank width %d\n", bank_width);
 			goto fail;
@@ -219,17 +462,24 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 			goto fail;
 		}
 		gpio_base += gc->ngpio;
+
+		if (priv->parent_irq > 0) {
+			err = brcmstb_gpio_irq_setup(pdev, bank);
+			if (err)
+				goto fail;
+		}
+
 		dev_dbg(dev, "bank=%d, base=%d, ngpio=%d, width=%d\n", bank->id,
 			gc->base, gc->ngpio, bank->width);
 
 		/* Everything looks good, so add bank to list */
 		list_add(&bank->node, &priv->bank_list);
 
-		priv->num_banks++;
+		num_banks++;
 	}
 
 	dev_info(dev, "Registered %d banks (GPIO(s): %d-%d)\n",
-			priv->num_banks, priv->gpio_base, gpio_base - 1);
+			num_banks, priv->gpio_base, gpio_base - 1);
 
 	return 0;
 
-- 
1.9.1

[PATCH v4 2/4] gpio: brcmstb: Add interrupt and wakeup source support

[Linus Walleij]

On Sat, Aug 1, 2015 at 3:17 AM, Gregory Fong <gregory.0xf0@gmail.com> wrote:

> Uses the gpiolib irqchip helpers.  For this to work, the irq setup
> function is called once per bank instead of once per device.  Note
> that all known uses of this block have a BCM7120 L2 interrupt
> controller as a parent.  Supports interrupts for all GPIOs.
>
> In the IRQ handler, we check for raised IRQs for invalid GPIOs and
> warn (ratelimited) if they're encountered.
>
> Also, several drivers (e.g. gpio-keys) allow for GPIOs to be
> configured as wakeup sources, and this GPIO controller supports that
> through a separate interrupt path.
>
> The de-facto standard DT property "wakeup-source" is checked, since
> that indicates whether the GPIO controller hardware can wake.  Uses
> the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have
> any of its own wakeup source configuration.
>
> Aside regarding gpiolib irqchip helpers: It wasn't obvious (to me)
> that you can have multiple chained irqchips and associated IRQ domains
> for a single parent IRQ, and as long as the xlate function is written
> correctly, a GPIO IRQ request end up checking the correct domain and
> will get associated with the correct IRQ.  What helps make this clear
> is to read
>   drivers/gpio/gpiolib-of.c:
>    - of_gpiochip_find_and_xlate()
>    - of_get_named_gpiod_flags()
>   drivers/gpio/gpiolib.c:
>    - gpiochip_find()
>
> Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>
> ---
> v4:
> - when checking parent_irq, use <= 0 or > 0 since 0 is NO_IRQ.

Patch applied, but:


> +       if (of_property_read_bool(np, "interrupt-controller")) {
> +               priv->parent_irq = platform_get_irq(pdev, 0);
> +               if (priv->parent_irq <= 0) {
> +                       dev_err(dev, "Couldn't get IRQ");
> +                       return -ENOENT;
> +               }
> +       } else {
> +               priv->parent_irq = -ENOENT;
> +       }
> +
>         if (brcmstb_gpio_sanity_check_banks(dev, np, res))
>                 return -EINVAL;

This patch does not apply cleanly because the version in my
tree has INIT_LIST_HEAD(&priv->bank_list);
before the sanity check.

I applied it manually, but check that things are working right.

Yours,
Linus Walleij

[PATCH v4 2/4] gpio: brcmstb: Add interrupt and wakeup source support

[Gregory Fong]

On Thu, Aug 13, 2015 at 4:11 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Aug 1, 2015 at 3:17 AM, Gregory Fong <gregory.0xf0@gmail.com> wrote:
>
>> Uses the gpiolib irqchip helpers.  For this to work, the irq setup
>> function is called once per bank instead of once per device.  Note
>> that all known uses of this block have a BCM7120 L2 interrupt
>> controller as a parent.  Supports interrupts for all GPIOs.
>>
>> In the IRQ handler, we check for raised IRQs for invalid GPIOs and
>> warn (ratelimited) if they're encountered.
>>
>> Also, several drivers (e.g. gpio-keys) allow for GPIOs to be
>> configured as wakeup sources, and this GPIO controller supports that
>> through a separate interrupt path.
>>
>> The de-facto standard DT property "wakeup-source" is checked, since
>> that indicates whether the GPIO controller hardware can wake.  Uses
>> the IRQCHIP_MASK_ON_SUSPEND irq_chip flag because UPG GIO doesn't have
>> any of its own wakeup source configuration.
>>
>> Aside regarding gpiolib irqchip helpers: It wasn't obvious (to me)
>> that you can have multiple chained irqchips and associated IRQ domains
>> for a single parent IRQ, and as long as the xlate function is written
>> correctly, a GPIO IRQ request end up checking the correct domain and
>> will get associated with the correct IRQ.  What helps make this clear
>> is to read
>>   drivers/gpio/gpiolib-of.c:
>>    - of_gpiochip_find_and_xlate()
>>    - of_get_named_gpiod_flags()
>>   drivers/gpio/gpiolib.c:
>>    - gpiochip_find()
>>
>> Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>
>> ---
>> v4:
>> - when checking parent_irq, use <= 0 or > 0 since 0 is NO_IRQ.
>
> Patch applied, but:
>
>
>> +       if (of_property_read_bool(np, "interrupt-controller")) {
>> +               priv->parent_irq = platform_get_irq(pdev, 0);
>> +               if (priv->parent_irq <= 0) {
>> +                       dev_err(dev, "Couldn't get IRQ");
>> +                       return -ENOENT;
>> +               }
>> +       } else {
>> +               priv->parent_irq = -ENOENT;
>> +       }
>> +
>>         if (brcmstb_gpio_sanity_check_banks(dev, np, res))
>>                 return -EINVAL;
>
> This patch does not apply cleanly because the version in my
> tree has INIT_LIST_HEAD(&priv->bank_list);
> before the sanity check.
>
> I applied it manually, but check that things are working right.

Your tree seems to be missing

commit 2252607d327d5219a6331b50e6ec266d56402be0
Author: Gregory Fong <gregory.0xf0@gmail.com>
Date:   Wed Jun 17 18:00:40 2015 -0700

    gpio: brcmstb: fix null ptr dereference in driver remove

which you had pulled in and sent for 4.2-rc1.  It should apply cleanly
on top of that.

[PATCH v4 2/4] gpio: brcmstb: Add interrupt and wakeup source support

[Linus Walleij]

On Fri, Aug 14, 2015 at 1:49 AM, Gregory Fong <gregory.0xf0@gmail.com> wrote:
> On Thu, Aug 13, 2015 at 4:11 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Sat, Aug 1, 2015 at 3:17 AM, Gregory Fong <gregory.0xf0@gmail.com> wrote:

>> This patch does not apply cleanly because the version in my
>> tree has INIT_LIST_HEAD(&priv->bank_list);
>> before the sanity check.
>>
>> I applied it manually, but check that things are working right.
>
> Your tree seems to be missing
>
> commit 2252607d327d5219a6331b50e6ec266d56402be0
> Author: Gregory Fong <gregory.0xf0@gmail.com>
> Date:   Wed Jun 17 18:00:40 2015 -0700
>
>     gpio: brcmstb: fix null ptr dereference in driver remove
>
> which you had pulled in and sent for 4.2-rc1.  It should apply cleanly
> on top of that.

Yes sorry, my bad.

I merged in v4.2-rc4 and now it looks better.

Yours,
Linus Walleij

[PATCH v4 3/4] gpio: brcmstb: support wakeup from S5 cold boot

[Gregory Fong]

For wake from S5, we need to:
- register a reboot handler
- set wakeup capability before requesting IRQ so wakeup count is
  incremented
- mask all GPIO IRQs and clear any pending interrupts during driver
  probe to since no driver will yet be registered to handle any IRQs
  carried over from boot at that time, and it's possible that the
  booted kernel does not request the same IRQ anyway.

This means that /sys/.../power/wakeup_count is valid at boot time, and
we can properly account for S5 wakeup stats. e.g.:

  ### After waking from S5 from a GPIO key
  # cat /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup
  enabled
  # cat /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup_count
  1

Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>
---
v4: rename __brcmstb_gpio_irq_set_wake() to brcmstb_gpio_priv_set_wake().

 drivers/gpio/gpio-brcmstb.c | 56 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 46cc4e9..9ea86d2 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -20,6 +20,7 @@
 #include <linux/irqdomain.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/interrupt.h>
+#include <linux/reboot.h>
 
 #define GIO_BANK_SIZE           0x20
 #define GIO_ODEN(bank)          (((bank) * GIO_BANK_SIZE) + 0x00)
@@ -48,6 +49,7 @@ struct brcmstb_gpio_priv {
 	int gpio_base;
 	bool can_wake;
 	int parent_wake_irq;
+	struct notifier_block reboot_notifier;
 };
 
 #define MAX_GPIO_PER_BANK           32
@@ -167,10 +169,9 @@ static int brcmstb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 	return 0;
 }
 
-static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
+static int brcmstb_gpio_priv_set_wake(struct brcmstb_gpio_priv *priv,
+		unsigned int enable)
 {
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
 	int ret = 0;
 
 	/*
@@ -188,6 +189,14 @@ static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
 	return ret;
 }
 
+static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
+
+	return brcmstb_gpio_priv_set_wake(priv, enable);
+}
+
 static irqreturn_t brcmstb_gpio_wake_irq_handler(int irq, void *data)
 {
 	struct brcmstb_gpio_priv *priv = data;
@@ -246,6 +255,19 @@ static void brcmstb_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
+static int brcmstb_gpio_reboot(struct notifier_block *nb,
+		unsigned long action, void *data)
+{
+	struct brcmstb_gpio_priv *priv =
+		container_of(nb, struct brcmstb_gpio_priv, reboot_notifier);
+
+	/* Enable GPIO for S5 cold boot */
+	if (action == SYS_POWER_OFF)
+		brcmstb_gpio_priv_set_wake(priv, 1);
+
+	return NOTIFY_DONE;
+}
+
 /* Make sure that the number of banks matches up between properties */
 static int brcmstb_gpio_sanity_check_banks(struct device *dev,
 		struct device_node *np, struct resource *res)
@@ -285,6 +307,12 @@ static int brcmstb_gpio_remove(struct platform_device *pdev)
 		if (ret)
 			dev_err(&pdev->dev, "gpiochip_remove fail in cleanup\n");
 	}
+	if (priv->reboot_notifier.notifier_call) {
+		ret = unregister_reboot_notifier(&priv->reboot_notifier);
+		if (ret)
+			dev_err(&pdev->dev,
+				"failed to unregister reboot notifier\n");
+	}
 	return ret;
 }
 
@@ -342,7 +370,16 @@ static int brcmstb_gpio_irq_setup(struct platform_device *pdev,
 			dev_warn(dev,
 				"Couldn't get wake IRQ - GPIOs will not be able to wake from sleep");
 		} else {
-			int err = devm_request_irq(dev, priv->parent_wake_irq,
+			int err;
+
+			/*
+			 * Set wakeup capability before requesting wakeup
+			 * interrupt, so we can process boot-time "wakeups"
+			 * (e.g., from S5 cold boot)
+			 */
+			device_set_wakeup_capable(dev, true);
+			device_wakeup_enable(dev);
+			err = devm_request_irq(dev, priv->parent_wake_irq,
 					brcmstb_gpio_wake_irq_handler, 0,
 					"brcmstb-gpio-wake", priv);
 
@@ -351,8 +388,9 @@ static int brcmstb_gpio_irq_setup(struct platform_device *pdev,
 				return err;
 			}
 
-			device_set_wakeup_capable(dev, true);
-			device_wakeup_enable(dev);
+			priv->reboot_notifier.notifier_call =
+				brcmstb_gpio_reboot;
+			register_reboot_notifier(&priv->reboot_notifier);
 			priv->can_wake = true;
 		}
 	}
@@ -455,6 +493,12 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
 		/* not all ngpio lines are valid, will use bank width later */
 		gc->ngpio = MAX_GPIO_PER_BANK;
 
+		/*
+		 * Mask all interrupts by default, since wakeup interrupts may
+		 * be retained from S5 cold boot
+		 */
+		bank->bgc.write_reg(reg_base + GIO_MASK(bank->id), 0);
+
 		err = gpiochip_add(gc);
 		if (err) {
 			dev_err(dev, "Could not add gpiochip for bank %d\n",
-- 
1.9.1

[PATCH v4 3/4] gpio: brcmstb: support wakeup from S5 cold boot

[Linus Walleij]

On Sat, Aug 1, 2015 at 3:17 AM, Gregory Fong <gregory.0xf0@gmail.com> wrote:

> For wake from S5, we need to:
> - register a reboot handler
> - set wakeup capability before requesting IRQ so wakeup count is
>   incremented
> - mask all GPIO IRQs and clear any pending interrupts during driver
>   probe to since no driver will yet be registered to handle any IRQs
>   carried over from boot at that time, and it's possible that the
>   booted kernel does not request the same IRQ anyway.
>
> This means that /sys/.../power/wakeup_count is valid at boot time, and
> we can properly account for S5 wakeup stats. e.g.:
>
>   ### After waking from S5 from a GPIO key
>   # cat /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup
>   enabled
>   # cat /sys/bus/platform/drivers/brcmstb-gpio/f04172c0.gpio/power/wakeup_count
>   1
>
> Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>
> ---
> v4: rename __brcmstb_gpio_irq_set_wake() to brcmstb_gpio_priv_set_wake().

Patch applied.

Yours,
Linus Walleij

[PATCH v4 4/4] ARM: dts: brcmstb: add BCM7445 GPIO nodes

[Gregory Fong]

Need the aon_pm_l2_intc and irq0_aon_intc descriptions, so included
those as well.

Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>
---
New in v4.

 arch/arm/boot/dts/bcm7445.dtsi | 50 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm/boot/dts/bcm7445.dtsi b/arch/arm/boot/dts/bcm7445.dtsi
index 58dcd66..3b6b175 100644
--- a/arch/arm/boot/dts/bcm7445.dtsi
+++ b/arch/arm/boot/dts/bcm7445.dtsi
@@ -109,6 +109,20 @@
 			brcm,int-fwd-mask = <0x70000>;
 		};
 
+		irq0_aon_intc: interrupt-controller at 417280 {
+			compatible = "brcm,bcm7120-l2-intc";
+			reg = <0x417280 0x8>;
+			interrupt-parent = <&gic>;
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			interrupts = <GIC_SPI 0x46 0x0>,
+				     <GIC_SPI 0x44 0x0>,
+				     <GIC_SPI 0x49 0x0>;
+			brcm,int-map-mask = <0x1e3 0x18000000 0x100000>;
+			brcm,int-fwd-mask = <0x0>;
+			brcm,irq-can-wake;
+		};
+
 		hif_intr2_intc: interrupt-controller at 3e1000 {
 			compatible = "brcm,l2-intc";
 			reg = <0x3e1000 0x30>;
@@ -119,6 +133,16 @@
 			interrupt-names = "hif";
 		};
 
+                aon_pm_l2_intc: interrupt-controller at 410640 {
+			compatible = "brcm,l2-intc";
+			reg = <0x410640 0x30>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			interrupts = <GIC_SPI 0x40 0x0>;
+			interrupt-parent = <&gic>;
+			brcm,irq-can-wake;
+		};
+
 		nand: nand at 3e2800 {
 			status = "disabled";
 			#address-cells = <1>;
@@ -167,6 +191,32 @@
 				#phy-cells = <0>;
 			};
 		};
+
+		upg_gio: gpio at 40a700 {
+			compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
+			reg = <0x40a700 0x80>;
+			#gpio-cells = <2>;
+			#interrupt-cells = <2>;
+			gpio-controller;
+			interrupt-controller;
+			interrupt-parent = <&irq0_intc>;
+			interrupts = <6>;
+			brcm,gpio-bank-widths = <32 32 32 24>;
+		};
+
+		upg_gio_aon: gpio at 4172c0 {
+			compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
+			reg = <0x4172c0 0x40>;
+			#gpio-cells = <2>;
+			#interrupt-cells = <2>;
+			gpio-controller;
+			interrupt-controller;
+			interrupts-extended = <&irq0_aon_intc 0x6>,
+					      <&aon_pm_l2_intc 0x5>;
+			wakeup-source;
+			brcm,gpio-bank-widths = <18 4>;
+		};
+
 	};
 
 	smpboot {
-- 
1.9.1

[PATCH v4 4/4] ARM: dts: brcmstb: add BCM7445 GPIO nodes

[Florian Fainelli]

On 31/07/15 18:17, Gregory Fong wrote:
> Need the aon_pm_l2_intc and irq0_aon_intc descriptions, so included
> those as well.
> 
> Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>

Applied to devicetree/next, thanks!
-- 
Florian

[PATCH v4 0/4] GPIO support for BRCMSTB

[Florian Fainelli]

On 31/07/15 18:17, Gregory Fong wrote:
> Adds interrupt support for the GPIO controller (UPG GIO) used on Broadcom's
> various BRCMSTB SoCs (BCM7XXX and others).  For all existing hardware, this
> block hooks up to the BCM7120 L2 IRQ controller and so will require
> CONFIG_BCM7120_L2_IRQ=y.

For this entire series:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks!

> 
> New in v4:
> - add nodes for the BRCMSTB GPIO controller to the BCM7445 dts file
> - a few improvements suggested by Linus Walleij on v3
> - remove unused 'irq' argument to brcmstb_gpio_irq_bank_handler()
> 
> The following are not included in this patchset:
> - Initial device tree bindings (merged from v1 to GPIO tree)
> - Initial GPIO support w/o interrupts (merged from v2 to GPIO tree)
> - ARM Kconfig changes (merged from v2 to arm-soc tree)
> - fix for null ptr deref in driver remove (merged from v3)
> 
> Previous versions:
> v1: https://lkml.org/lkml/2015/5/6/199
> v2: https://lkml.org/lkml/2015/5/28/853
> v3: https://lkml.org/lkml/2015/6/17/960
> 
> Gregory Fong (4):
>   dt-bindings: brcmstb-gpio: document properties for wakeup
>   gpio: brcmstb: Add interrupt and wakeup source support
>   gpio: brcmstb: support wakeup from S5 cold boot
>   ARM: dts: brcmstb: add BCM7445 GPIO nodes
> 
>  .../devicetree/bindings/gpio/brcm,brcmstb-gpio.txt |  35 ++-
>  arch/arm/boot/dts/bcm7445.dtsi                     |  50 ++++
>  drivers/gpio/Kconfig                               |   1 +
>  drivers/gpio/gpio-brcmstb.c                        | 306 ++++++++++++++++++++-
>  4 files changed, 379 insertions(+), 13 deletions(-)
> 


-- 
Florian