[PATCH 0/4] gpio: hierarchical IRQ improvements

[PATCH 0/4] gpio: hierarchical IRQ improvements

[Brian Masney]

This builds on top of Linus Walleij's existing patches that adds
hierarchical IRQ support to the GPIO core [1] so that Qualcomm's
spmi-gpio and ssbi-gpio can be converted to use these new helpers.

Linus: Feel free to squash these into your existing patches if you'd
like to use any of this code. Just give me some kind of mention in the
commit description.

[1] https://lore.kernel.org/linux-gpio/20190624132531.6184-1-linus.walleij@linaro.org/

Brian Masney (4):
  gpio: introduce gpiochip_populate_parent_fwspec_{two,four}cell
    functions
  gpio: allow customizing hierarchical IRQ chips
  gpio: use handler in gpio_irq_chip instead of handle_bad_irq
  qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core

 drivers/gpio/gpiolib.c                   | 78 ++++++++++++++++----
 drivers/pinctrl/qcom/Kconfig             |  1 +
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 92 +++++++-----------------
 include/linux/gpio/driver.h              | 65 +++++++++++++++++
 4 files changed, 154 insertions(+), 82 deletions(-)

-- 
2.20.1

[PATCH 1/4] gpio: introduce gpiochip_populate_parent_fwspec_{two,four}cell functions

[Brian Masney]

Introduce the functions gpiochip_populate_parent_fwspec_{two,four}cell
that will be used for hierarchical IRQ support in GPIO drivers.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpio/gpiolib.c      | 24 ++++++++++++++++++++++++
 include/linux/gpio/driver.h | 30 ++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d767e111694d..06c9cf714c99 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1946,6 +1946,30 @@ static bool gpiochip_hierarchy_is_hierarchical(struct gpio_chip *gc)
 	return !!gc->irq.parent_domain;
 }
 
+void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
+					     struct irq_fwspec *fwspec,
+					     unsigned int parent_hwirq,
+					     unsigned int parent_type)
+{
+	fwspec->param_count = 2;
+	fwspec->param[0] = parent_hwirq;
+	fwspec->param[1] = parent_type;
+}
+EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_twocell);
+
+void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
+					      struct irq_fwspec *fwspec,
+					      unsigned int parent_hwirq,
+					      unsigned int parent_type)
+{
+	fwspec->param_count = 4;
+	fwspec->param[0] = 0;
+	fwspec->param[1] = parent_hwirq;
+	fwspec->param[2] = 0;
+	fwspec->param[3] = parent_type;
+}
+EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_fourcell);
+
 #else
 
 static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 682ad09e29f4..6b6bca20c8f9 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -493,6 +493,36 @@ struct bgpio_pdata {
 	int ngpio;
 };
 
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+
+void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
+					     struct irq_fwspec *fwspec,
+					     unsigned int parent_hwirq,
+					     unsigned int parent_type);
+void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
+					      struct irq_fwspec *fwspec,
+					      unsigned int parent_hwirq,
+					      unsigned int parent_type);
+
+#else
+
+static void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
+						    struct irq_fwspec *fwspec,
+						    unsigned int parent_hwirq,
+						    unsigned int parent_type)
+{
+}
+
+static void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
+						     struct irq_fwspec *fwspec,
+						     unsigned int parent_hwirq,
+						     unsigned int parent_type)
+{
+}
+
+#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
+
+
 #if IS_ENABLED(CONFIG_GPIO_GENERIC)
 
 int bgpio_init(struct gpio_chip *gc, struct device *dev,
-- 
2.20.1

[PATCH 2/4] gpio: allow customizing hierarchical IRQ chips

[Brian Masney]

Now that the GPIO core has support for hierarchical IRQ chips, let's add
support for three new callbacks in struct gpio_irq_chip:

populate_parent_fwspec:
    This optional callback populates the struct irq_fwspec for the
    parent's IRQ domain. If this is not specified, then
    gpiochip_populate_parent_fwspec_twocell will be used. A four-cell
    variant named &gpiochip_populate_parent_fwspec_twocell is also
    available.

child_pin_to_irq:
    This optional callback is used to translate the child's GPIO pin
    number to an IRQ number for the GPIO to_irq() callback. If this is
    not specified, then a default callback will be provided that
    returns the pin number.

child_irq_domain_ops:
    The IRQ domain operations that will be used for this GPIO IRQ
    chip. If no operations are provided, then default callbacks will
    be populated to setup the IRQ hierarchy. Some drivers need to
    supply their own translate function.

These will be initially used by Qualcomm's spmi-gpio and ssbi-gpio.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Note: checkpatch doesn't like that child_irq_domain_ops is not const.

 drivers/gpio/gpiolib.c      | 52 +++++++++++++++++++++++++++----------
 include/linux/gpio/driver.h | 35 +++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 06c9cf714c99..5423242deb81 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1778,7 +1778,7 @@ static void gpiochip_set_hierarchical_irqchip(struct gpio_chip *gc,
 
 			fwspec.fwnode = gc->irq.fwnode;
 			/* This is the hwirq for the GPIO line side of things */
-			fwspec.param[0] = i;
+			fwspec.param[0] = girq->child_pin_to_irq(gc, i);
 			/* Just pick something */
 			fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
 			fwspec.param_count = 2;
@@ -1841,7 +1841,7 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 
 	chip_info(gc, "called %s\n", __func__);
 
-	ret = gpiochip_hierarchy_irq_domain_translate(d, fwspec, &hwirq, &type);
+	ret = gc->irq.child_irq_domain_ops.translate(d, fwspec, &hwirq, &type);
 	if (ret)
 		return ret;
 
@@ -1882,10 +1882,9 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 		 * all together up the chain.
 		 */
 		parent_fwspec.fwnode = d->parent->fwnode;
-		parent_fwspec.param_count = 2;
-		parent_fwspec.param[0] = parent_hwirq;
 		/* This parent only handles asserted level IRQs */
-		parent_fwspec.param[1] = parent_type;
+		girq->populate_parent_fwspec(gc, &parent_fwspec, parent_hwirq,
+					     parent_type);
 		chip_info(gc, "alloc_irqs_parent for %d parent hwirq %d\n",
 			  irq + i, parent_hwirq);
 		ret = irq_domain_alloc_irqs_parent(d, irq + i, 1,
@@ -1899,13 +1898,29 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 	return 0;
 }
 
-static const struct irq_domain_ops gpiochip_hierarchy_domain_ops = {
-	.activate = gpiochip_irq_domain_activate,
-	.deactivate = gpiochip_irq_domain_deactivate,
-	.translate = gpiochip_hierarchy_irq_domain_translate,
-	.alloc = gpiochip_hierarchy_irq_domain_alloc,
-	.free = irq_domain_free_irqs_common,
-};
+static unsigned int gpiochip_child_pin_to_irq_noop(struct gpio_chip *chip,
+						   unsigned int pin)
+{
+	return pin;
+}
+
+static void gpiochip_add_default_irq_domain_ops(struct irq_domain_ops *ops)
+{
+	if (!ops->activate)
+		ops->activate = gpiochip_irq_domain_activate;
+
+	if (!ops->deactivate)
+		ops->deactivate = gpiochip_irq_domain_deactivate;
+
+	if (!ops->translate)
+		ops->translate = gpiochip_hierarchy_irq_domain_translate;
+
+	if (!ops->alloc)
+		ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
+
+	if (!ops->free)
+		ops->free = irq_domain_free_irqs_common;
+}
 
 static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
 {
@@ -1921,12 +1936,21 @@ static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
 		return -EINVAL;
 	}
 
+	if (!gc->irq.child_pin_to_irq)
+		gc->irq.child_pin_to_irq = gpiochip_child_pin_to_irq_noop;
+
+	if (!gc->irq.populate_parent_fwspec)
+		gc->irq.populate_parent_fwspec =
+			gpiochip_populate_parent_fwspec_twocell;
+
+	gpiochip_add_default_irq_domain_ops(&gc->irq.child_irq_domain_ops);
+
 	gc->irq.domain = irq_domain_create_hierarchy(
 		gc->irq.parent_domain,
 		IRQ_DOMAIN_FLAG_HIERARCHY,
 		gc->ngpio,
 		gc->irq.fwnode,
-		&gpiochip_hierarchy_domain_ops,
+		&gc->irq.child_irq_domain_ops,
 		gc);
 
 	if (!gc->irq.domain) {
@@ -2106,7 +2130,7 @@ static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
 
 		spec.fwnode = domain->fwnode;
 		spec.param_count = 2;
-		spec.param[0] = offset;
+		spec.param[0] = chip->irq.child_pin_to_irq(chip, offset);
 		spec.param[1] = IRQ_TYPE_NONE;
 
 		return irq_create_fwspec_mapping(&spec);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 6b6bca20c8f9..fb9c126dd8f0 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -93,6 +93,41 @@ struct gpio_irq_chip {
 				     unsigned int child_type,
 				     unsigned int *parent_hwirq,
 				     unsigned int *parent_type);
+
+	/**
+	 * @populate_parent_fwspec:
+	 *
+	 * This optional callback populates the &struct irq_fwspec for the
+	 * parent's IRQ domain. If this is not specified, then
+	 * &gpiochip_populate_parent_fwspec_twocell will be used. A four-cell
+	 * variant named &gpiochip_populate_parent_fwspec_twocell is also
+	 * available.
+	 */
+	void (*populate_parent_fwspec)(struct gpio_chip *chip,
+				       struct irq_fwspec *fwspec,
+				       unsigned int parent_hwirq,
+				       unsigned int parent_type);
+
+	/**
+	 * @child_pin_to_irq:
+	 *
+	 * This optional callback is used to translate the child's GPIO pin
+	 * number to an IRQ number for the GPIO to_irq() callback. If this is
+	 * not specified, then a default callback will be provided that
+	 * returns the pin number.
+	 */
+	unsigned int (*child_pin_to_irq)(struct gpio_chip *chip,
+					 unsigned int pin);
+
+	/**
+	 * @child_irq_domain_ops:
+	 *
+	 * The IRQ domain operations that will be used for this GPIO IRQ
+	 * chip. If no operations are provided, then default callbacks will
+	 * be populated to setup the IRQ hierarchy. Some drivers need to
+	 * supply their own translate function.
+	 */
+	struct irq_domain_ops child_irq_domain_ops;
 #endif
 
 	/**
-- 
2.20.1

Re: [PATCH 2/4] gpio: allow customizing hierarchical IRQ chips

[Linus Walleij]

On Mon, Jul 8, 2019 at 1:01 PM Brian Masney <masneyb@onstation.org> wrote:

> Now that the GPIO core has support for hierarchical IRQ chips, let's add
> support for three new callbacks in struct gpio_irq_chip:
>
> populate_parent_fwspec:
>     This optional callback populates the struct irq_fwspec for the
>     parent's IRQ domain. If this is not specified, then
>     gpiochip_populate_parent_fwspec_twocell will be used. A four-cell
>     variant named &gpiochip_populate_parent_fwspec_twocell is also
>     available.
>
> child_pin_to_irq:
>     This optional callback is used to translate the child's GPIO pin
>     number to an IRQ number for the GPIO to_irq() callback. If this is
>     not specified, then a default callback will be provided that
>     returns the pin number.
>
> child_irq_domain_ops:
>     The IRQ domain operations that will be used for this GPIO IRQ
>     chip. If no operations are provided, then default callbacks will
>     be populated to setup the IRQ hierarchy. Some drivers need to
>     supply their own translate function.
>
> These will be initially used by Qualcomm's spmi-gpio and ssbi-gpio.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

This is overall looking very appetizing!

I want to apply this on top of my patch and respin it
with some of Masahiro's comments as well and then let's
try to just apply all of this.

> Note: checkpatch doesn't like that child_irq_domain_ops is not const.

Hm? I suspect some janitor will find the problem and patch it for us.

> +static void gpiochip_add_default_irq_domain_ops(struct irq_domain_ops *ops)
> +{
> +       if (!ops->activate)
> +               ops->activate = gpiochip_irq_domain_activate;
> +
> +       if (!ops->deactivate)
> +               ops->deactivate = gpiochip_irq_domain_deactivate;
> +
> +       if (!ops->translate)
> +               ops->translate = gpiochip_hierarchy_irq_domain_translate;
> +
> +       if (!ops->alloc)
> +               ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
> +
> +       if (!ops->free)
> +               ops->free = irq_domain_free_irqs_common;
> +}

I'm fine with translate(), this seems to be what Lina needs too.

But do we really need to make them all optional? activate() and
deactivate() will require the driver to lock the irq etc which is hairy.

Yours,
Linus Walleij

Re: [PATCH 2/4] gpio: allow customizing hierarchical IRQ chips

[Brian Masney]

On Mon, Jul 29, 2019 at 12:49:55AM +0200, Linus Walleij wrote:
> On Mon, Jul 8, 2019 at 1:01 PM Brian Masney <masneyb@onstation.org> wrote:
> > +static void gpiochip_add_default_irq_domain_ops(struct irq_domain_ops *ops)
> > +{
> > +       if (!ops->activate)
> > +               ops->activate = gpiochip_irq_domain_activate;
> > +
> > +       if (!ops->deactivate)
> > +               ops->deactivate = gpiochip_irq_domain_deactivate;
> > +
> > +       if (!ops->translate)
> > +               ops->translate = gpiochip_hierarchy_irq_domain_translate;
> > +
> > +       if (!ops->alloc)
> > +               ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
> > +
> > +       if (!ops->free)
> > +               ops->free = irq_domain_free_irqs_common;
> > +}
> 
> I'm fine with translate(), this seems to be what Lina needs too.
> 
> But do we really need to make them all optional? activate() and
> deactivate() will require the driver to lock the irq etc which is hairy.

I can't think of a reason right now that we'd need to override the
others. Since the structure was there, I made all of them optional to
try to future proof this a little bit.

It probably would be better to only make translate optional at this
point. If needed, someone can submit a patch for one or more of the
others with their use case.

Brian

[PATCH 3/4] gpio: use handler in gpio_irq_chip instead of handle_bad_irq

[Brian Masney]

Use the IRQ handler field that's available in the struct gpio_irq_chip
when allocating an IRQ rather than hardcoding the handler to
handle_bad_irq(). The kernel reboots without any messages when testing
this using spmi-gpio on the Nexus 5.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
I didn't have time to dig into more detail about why this is happening.
I suspect the issue is that __irq_do_set_handler() has a special check
for handle_bad_irq:

https://elixir.bootlin.com/linux/latest/source/kernel/irq/chip.c#L974

My post about this:
https://lore.kernel.org/linux-gpio/20190707014620.GA9690@onstation.org/

 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5423242deb81..bc68ebb8f40e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1872,7 +1872,7 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 				    hwirq + i,
 				    gc->irq.chip,
 				    gc,
-				    handle_bad_irq,
+				    girq->handler,
 				    NULL, NULL);
 		irq_set_probe(irq + i);
 
-- 
2.20.1

[PATCH 4/4] qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core

[Brian Masney]

Now that the GPIO core has support for hierarchical IRQ chips, convert
Qualcomm's spmi-gpio over to use these new helpers to reduce duplicated
code across drivers.

This change was tested on a LG Nexus 5 (hammerhead) phone.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/pinctrl/qcom/Kconfig             |  1 +
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 92 +++++++-----------------
 2 files changed, 26 insertions(+), 67 deletions(-)

diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 27ab585a639c..9b36da702925 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -138,6 +138,7 @@ config PINCTRL_QCOM_SPMI_PMIC
        select PINMUX
        select PINCONF
        select GENERIC_PINCONF
+       select GPIOLIB_IRQCHIP
        select IRQ_DOMAIN_HIERARCHY
        help
          This is the pinctrl, pinmux, pinconf and gpiolib driver for the
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index f39da87ea185..0bef149c2f16 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -170,8 +170,6 @@ struct pmic_gpio_state {
 	struct regmap	*map;
 	struct pinctrl_dev *ctrl;
 	struct gpio_chip chip;
-	struct fwnode_handle *fwnode;
-	struct irq_domain *domain;
 };
 
 static const struct pinconf_generic_params pmic_gpio_bindings[] = {
@@ -751,23 +749,6 @@ static int pmic_gpio_of_xlate(struct gpio_chip *chip,
 	return gpio_desc->args[0] - PMIC_GPIO_PHYSICAL_OFFSET;
 }
 
-static int pmic_gpio_to_irq(struct gpio_chip *chip, unsigned pin)
-{
-	struct pmic_gpio_state *state = gpiochip_get_data(chip);
-	struct irq_fwspec fwspec;
-
-	fwspec.fwnode = state->fwnode;
-	fwspec.param_count = 2;
-	fwspec.param[0] = pin + PMIC_GPIO_PHYSICAL_OFFSET;
-	/*
-	 * Set the type to a safe value temporarily. This will be overwritten
-	 * later with the proper value by irq_set_type.
-	 */
-	fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
-
-	return irq_create_fwspec_mapping(&fwspec);
-}
-
 static void pmic_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 {
 	struct pmic_gpio_state *state = gpiochip_get_data(chip);
@@ -787,7 +768,6 @@ static const struct gpio_chip pmic_gpio_gpio_template = {
 	.request		= gpiochip_generic_request,
 	.free			= gpiochip_generic_free,
 	.of_xlate		= pmic_gpio_of_xlate,
-	.to_irq			= pmic_gpio_to_irq,
 	.dbg_show		= pmic_gpio_dbg_show,
 };
 
@@ -964,46 +944,24 @@ static int pmic_gpio_domain_translate(struct irq_domain *domain,
 	return 0;
 }
 
-static int pmic_gpio_domain_alloc(struct irq_domain *domain, unsigned int virq,
-				  unsigned int nr_irqs, void *data)
+static unsigned int pmic_gpio_child_pin_to_irq(struct gpio_chip *chip,
+					       unsigned int pin)
 {
-	struct pmic_gpio_state *state = container_of(domain->host_data,
-						     struct pmic_gpio_state,
-						     chip);
-	struct irq_fwspec *fwspec = data;
-	struct irq_fwspec parent_fwspec;
-	irq_hw_number_t hwirq;
-	unsigned int type;
-	int ret, i;
-
-	ret = pmic_gpio_domain_translate(domain, fwspec, &hwirq, &type);
-	if (ret)
-		return ret;
-
-	for (i = 0; i < nr_irqs; i++)
-		irq_domain_set_info(domain, virq + i, hwirq + i,
-				    &pmic_gpio_irq_chip, state,
-				    handle_level_irq, NULL, NULL);
+	return pin + PMIC_GPIO_PHYSICAL_OFFSET;
+}
 
-	parent_fwspec.fwnode = domain->parent->fwnode;
-	parent_fwspec.param_count = 4;
-	parent_fwspec.param[0] = 0;
-	parent_fwspec.param[1] = hwirq + 0xc0;
-	parent_fwspec.param[2] = 0;
-	parent_fwspec.param[3] = fwspec->param[1];
+static int pmic_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
+					   unsigned int child_hwirq,
+					   unsigned int child_type,
+					   unsigned int *parent_hwirq,
+					   unsigned int *parent_type)
+{
+	*parent_hwirq = child_hwirq + 0xc0;
+	*parent_type = child_type;
 
-	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
-					    &parent_fwspec);
+	return 0;
 }
 
-static const struct irq_domain_ops pmic_gpio_domain_ops = {
-	.activate = gpiochip_irq_domain_activate,
-	.alloc = pmic_gpio_domain_alloc,
-	.deactivate = gpiochip_irq_domain_deactivate,
-	.free = irq_domain_free_irqs_common,
-	.translate = pmic_gpio_domain_translate,
-};
-
 static int pmic_gpio_probe(struct platform_device *pdev)
 {
 	struct irq_domain *parent_domain;
@@ -1013,6 +971,7 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	struct pinctrl_desc *pctrldesc;
 	struct pmic_gpio_pad *pad, *pads;
 	struct pmic_gpio_state *state;
+	struct gpio_irq_chip *girq;
 	int ret, npins, i;
 	u32 reg;
 
@@ -1092,19 +1051,21 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	if (!parent_domain)
 		return -ENXIO;
 
-	state->fwnode = of_node_to_fwnode(state->dev->of_node);
-	state->domain = irq_domain_create_hierarchy(parent_domain, 0,
-						    state->chip.ngpio,
-						    state->fwnode,
-						    &pmic_gpio_domain_ops,
-						    &state->chip);
-	if (!state->domain)
-		return -ENODEV;
+	girq = &state->chip.irq;
+	girq->chip = &pmic_gpio_irq_chip;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_level_irq;
+	girq->fwnode = of_node_to_fwnode(state->dev->of_node);
+	girq->parent_domain = parent_domain;
+	girq->child_to_parent_hwirq = pmic_gpio_child_to_parent_hwirq;
+	girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
+	girq->child_pin_to_irq = pmic_gpio_child_pin_to_irq;
+	girq->child_irq_domain_ops.translate = pmic_gpio_domain_translate;
 
 	ret = gpiochip_add_data(&state->chip, state);
 	if (ret) {
 		dev_err(state->dev, "can't add gpio chip\n");
-		goto err_chip_add_data;
+		return ret;
 	}
 
 	/*
@@ -1130,8 +1091,6 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 
 err_range:
 	gpiochip_remove(&state->chip);
-err_chip_add_data:
-	irq_domain_remove(state->domain);
 	return ret;
 }
 
@@ -1140,7 +1099,6 @@ static int pmic_gpio_remove(struct platform_device *pdev)
 	struct pmic_gpio_state *state = platform_get_drvdata(pdev);
 
 	gpiochip_remove(&state->chip);
-	irq_domain_remove(state->domain);
 	return 0;
 }
 
-- 
2.20.1

Re: [PATCH 0/4] gpio: hierarchical IRQ improvements

[Linus Walleij]

On Mon, Jul 8, 2019 at 1:01 PM Brian Masney <masneyb@onstation.org> wrote:

> This builds on top of Linus Walleij's existing patches that adds
> hierarchical IRQ support to the GPIO core [1] so that Qualcomm's
> spmi-gpio and ssbi-gpio can be converted to use these new helpers.
>
> Linus: Feel free to squash these into your existing patches if you'd
> like to use any of this code. Just give me some kind of mention in the
> commit description.
>
> [1] https://lore.kernel.org/linux-gpio/20190624132531.6184-1-linus.walleij@linaro.org/
>
> Brian Masney (4):
>   gpio: introduce gpiochip_populate_parent_fwspec_{two,four}cell
>     functions
>   gpio: allow customizing hierarchical IRQ chips
>   gpio: use handler in gpio_irq_chip instead of handle_bad_irq
>   qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core

I solved things like this:

- I kept patches 1 & 4 as-is
- I squashed patches 2 and 3 into the main patch with minor modifications.
- I added Co-developed-by: for your contributions

Now I need to address Masahiro's comments on top and let's see if the
result looks acceptable!

Yours,
Linus Walleij

Re: [PATCH 0/4] gpio: hierarchical IRQ improvements

[Linus Walleij]

On Wed, Aug 7, 2019 at 3:41 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Jul 8, 2019 at 1:01 PM Brian Masney <masneyb@onstation.org> wrote:
>
> > This builds on top of Linus Walleij's existing patches that adds
> > hierarchical IRQ support to the GPIO core [1] so that Qualcomm's
> > spmi-gpio and ssbi-gpio can be converted to use these new helpers.
> >
> > Linus: Feel free to squash these into your existing patches if you'd
> > like to use any of this code. Just give me some kind of mention in the
> > commit description.
> >
> > [1] https://lore.kernel.org/linux-gpio/20190624132531.6184-1-linus.walleij@linaro.org/
> >
> > Brian Masney (4):
> >   gpio: introduce gpiochip_populate_parent_fwspec_{two,four}cell
> >     functions
> >   gpio: allow customizing hierarchical IRQ chips
> >   gpio: use handler in gpio_irq_chip instead of handle_bad_irq
> >   qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core
>
> I solved things like this:
>
> - I kept patches 1 & 4 as-is

Ooops had to squash patch 1 as well...

Yours,
Linus Walleij

Re: [PATCH 0/4] gpio: hierarchical IRQ improvements

[Brian Masney]

On Wed, Aug 07, 2019 at 03:41:05PM +0200, Linus Walleij wrote:
> On Mon, Jul 8, 2019 at 1:01 PM Brian Masney <masneyb@onstation.org> wrote:
> 
> > This builds on top of Linus Walleij's existing patches that adds
> > hierarchical IRQ support to the GPIO core [1] so that Qualcomm's
> > spmi-gpio and ssbi-gpio can be converted to use these new helpers.
> >
> > Linus: Feel free to squash these into your existing patches if you'd
> > like to use any of this code. Just give me some kind of mention in the
> > commit description.
> >
> > [1] https://lore.kernel.org/linux-gpio/20190624132531.6184-1-linus.walleij@linaro.org/
> >
> > Brian Masney (4):
> >   gpio: introduce gpiochip_populate_parent_fwspec_{two,four}cell
> >     functions
> >   gpio: allow customizing hierarchical IRQ chips
> >   gpio: use handler in gpio_irq_chip instead of handle_bad_irq
> >   qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core
> 
> I solved things like this:
> 
> - I kept patches 1 & 4 as-is
> - I squashed patches 2 and 3 into the main patch with minor modifications.
> - I added Co-developed-by: for your contributions
> 
> Now I need to address Masahiro's comments on top and let's see if the
> result looks acceptable!

> Ooops had to squash patch 1 as well...

All of this sounds good. I'll retest once you send out the updated
series.

Brian