[PATCH v3 0/6] qcom: spmi: add support for hierarchical IRQ chip

[PATCH v3 0/6] qcom: spmi: add support for hierarchical IRQ chip

[Brian Masney]

This patch series adds hierarchical IRQ chip support to spmi-gpio so
that device tree consumers can request an IRQ directly from the GPIO
block rather than having to request an IRQ from the underlying PMIC.
 
For more background information, see the email thread with Linus
Walleij's excellent description of the problem at
https://www.spinics.net/lists/linux-gpio/msg34655.html.

This work was tested on a LG Nexus 5 (hammerhead) phone. My status page
at https://masneyb.github.io/nexus-5-upstream/ describes what is working
so far with an upstream kernel.

High-level changes since v2:
- Dropped patch to mfd/qcom-spmi-pmic.c
- Patch 3 is new and adds two new functions to gpiolib
- Patch 6 is new and corrects the only other upstream user of spmi-gpio

High-level changes since v1:
- Patches 1 and 2 are new. This brought in a third subsystem (mfd).
- I have detailed changelogs attached to the notes on patches 3-5.

Brian Masney (6):
  pinctrl: qcom: spmi-gpio: hardcode IRQ counts
  spmi: pmic-arb: convert to v2 irq interfaces to support hierarchical
    IRQ chips
  gpio: add irq domain activate/deactivate functions
  qcom: spmi-gpio: add support for hierarchical IRQ chip
  ARM: dts: qcom: pm8941: add interrupt controller properties
  ARM: dts: qcom: pma8084: add interrupt controller properties

 arch/arm/boot/dts/qcom-pm8941.dtsi       |  38 +------
 arch/arm/boot/dts/qcom-pma8084.dtsi      |  24 +---
 drivers/gpio/gpiolib.c                   |  37 +++++++
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 134 +++++++++++++++++++----
 drivers/spmi/spmi-pmic-arb.c             |  58 ++++++----
 include/linux/gpio/driver.h              |   5 +
 6 files changed, 194 insertions(+), 102 deletions(-)

-- 
2.17.2

[PATCH v3 1/6] pinctrl: qcom: spmi-gpio: hardcode IRQ counts

[Brian Masney]

The probing of this driver calls platform_irq_count, which will
setup all of the IRQs that are configured in device tree. In
preparation for converting this driver to be a hierarchical IRQ
chip, hardcode the IRQ count based on the hardware type so that all
the IRQs are not configured immediately and are configured on an
as-needed basis later in the boot process. This change will also
allow for the removal of the interrupts property later in this
patch series once the hierarchical IRQ chip support is in.

This patch also removes the generic qcom,spmi-gpio OF match since we
don't know the number of pins. All of the existing upstream bindings
already include the more-specific binding.

The pm8941 code was tested on a LG Nexus 5 (hammerhead) phone.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v2:
- Don't move pmic_gpio_of_match block. Use device_get_match_data instead
  of of_match_device.
- Correct build warning on arm64: warning: cast from pointer to integer
  of different size

 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 4458d44dfcf6..d3ce7a0e1203 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -951,13 +951,7 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	npins = platform_irq_count(pdev);
-	if (!npins)
-		return -EINVAL;
-	if (npins < 0)
-		return npins;
-
-	BUG_ON(npins > ARRAY_SIZE(pmic_gpio_groups));
+	npins = (u16)(uintptr_t) device_get_match_data(&pdev->dev);
 
 	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
 	if (!state)
@@ -1062,14 +1056,15 @@ static int pmic_gpio_remove(struct platform_device *pdev)
 	return 0;
 }
 
+/* data contains the number of GPIOs */
 static const struct of_device_id pmic_gpio_of_match[] = {
-	{ .compatible = "qcom,pm8916-gpio" },	/* 4 GPIO's */
-	{ .compatible = "qcom,pm8941-gpio" },	/* 36 GPIO's */
-	{ .compatible = "qcom,pm8994-gpio" },	/* 22 GPIO's */
-	{ .compatible = "qcom,pmi8994-gpio" },  /* 10 GPIO's */
-	{ .compatible = "qcom,pma8084-gpio" },	/* 22 GPIO's */
-	{ .compatible = "qcom,pms405-gpio" },	/* 12 GPIO's, holes on 1 9 10 */
-	{ .compatible = "qcom,spmi-gpio" }, /* Generic */
+	{ .compatible = "qcom,pm8916-gpio", .data = (void *) 4 },
+	{ .compatible = "qcom,pm8941-gpio", .data = (void *) 36 },
+	{ .compatible = "qcom,pm8994-gpio", .data = (void *) 22 },
+	{ .compatible = "qcom,pmi8994-gpio", .data = (void *) 10 },
+	{ .compatible = "qcom,pma8084-gpio", .data = (void *) 22 },
+	/* pms405 has 12 GPIOs with holes on 1, 9, and 10 */
+	{ .compatible = "qcom,pms405-gpio", .data = (void *) 12 },
 	{ },
 };
 
-- 
2.17.2

[PATCH v3 2/6] spmi: pmic-arb: convert to v2 irq interfaces to support hierarchical IRQ chips

[Brian Masney]

Convert the spmi-pmic-arb IRQ code to use the version 2 IRQ interface
in order to support hierarchical IRQ chips. This is necessary so that
spmi-gpio can be setup as a hierarchical IRQ chip with pmic-arb as the
parent. IRQ chips in device tree should be usable from the start without
the consumer having to make an additional call to gpio[d]_to_irq() to
get the proper IRQ on the parent. Driver was tested on a LG Nexus 5
(hammerhead) phone.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v2:
- None

Changes since v1:
- Add intspec variable to qpnpint_irq_domain_translate to reduce the
  overall diff.
- Remove irq_domain_disassociate hack.

 drivers/spmi/spmi-pmic-arb.c | 58 +++++++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 360b8218f322..356bc3f66e22 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -666,7 +666,8 @@ static int qpnpint_get_irqchip_state(struct irq_data *d,
 	return 0;
 }
 
-static int qpnpint_irq_request_resources(struct irq_data *d)
+static int qpnpint_irq_domain_activate(struct irq_domain *domain,
+				       struct irq_data *d, bool reserve)
 {
 	struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
 	u16 periph = hwirq_to_per(d->hwirq);
@@ -692,27 +693,25 @@ static struct irq_chip pmic_arb_irqchip = {
 	.irq_set_type	= qpnpint_irq_set_type,
 	.irq_set_wake	= qpnpint_irq_set_wake,
 	.irq_get_irqchip_state	= qpnpint_get_irqchip_state,
-	.irq_request_resources = qpnpint_irq_request_resources,
 	.flags		= IRQCHIP_MASK_ON_SUSPEND,
 };
 
-static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
-					   struct device_node *controller,
-					   const u32 *intspec,
-					   unsigned int intsize,
-					   unsigned long *out_hwirq,
-					   unsigned int *out_type)
+static int qpnpint_irq_domain_translate(struct irq_domain *d,
+					struct irq_fwspec *fwspec,
+					unsigned long *out_hwirq,
+					unsigned int *out_type)
 {
 	struct spmi_pmic_arb *pmic_arb = d->host_data;
+	u32 *intspec = fwspec->param;
 	u16 apid, ppid;
 	int rc;
 
 	dev_dbg(&pmic_arb->spmic->dev, "intspec[0] 0x%1x intspec[1] 0x%02x intspec[2] 0x%02x\n",
 		intspec[0], intspec[1], intspec[2]);
 
-	if (irq_domain_get_of_node(d) != controller)
+	if (irq_domain_get_of_node(d) != pmic_arb->spmic->dev.of_node)
 		return -EINVAL;
-	if (intsize != 4)
+	if (fwspec->param_count != 4)
 		return -EINVAL;
 	if (intspec[0] > 0xF || intspec[1] > 0xFF || intspec[2] > 0x7)
 		return -EINVAL;
@@ -740,17 +739,34 @@ static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
 	return 0;
 }
 
-static int qpnpint_irq_domain_map(struct irq_domain *d,
-				  unsigned int virq,
-				  irq_hw_number_t hwirq)
-{
-	struct spmi_pmic_arb *pmic_arb = d->host_data;
 
+static void qpnpint_irq_domain_map(struct spmi_pmic_arb *pmic_arb,
+				   struct irq_domain *domain, unsigned int virq,
+				   irq_hw_number_t hwirq)
+{
 	dev_dbg(&pmic_arb->spmic->dev, "virq = %u, hwirq = %lu\n", virq, hwirq);
 
-	irq_set_chip_and_handler(virq, &pmic_arb_irqchip, handle_level_irq);
-	irq_set_chip_data(virq, d->host_data);
-	irq_set_noprobe(virq);
+	irq_domain_set_info(domain, virq, hwirq, &pmic_arb_irqchip, pmic_arb,
+			    handle_level_irq, NULL, NULL);
+}
+
+static int qpnpint_irq_domain_alloc(struct irq_domain *domain,
+				    unsigned int virq, unsigned int nr_irqs,
+				    void *data)
+{
+	struct spmi_pmic_arb *pmic_arb = domain->host_data;
+	struct irq_fwspec *fwspec = data;
+	irq_hw_number_t hwirq;
+	unsigned int type;
+	int ret, i;
+
+	ret = qpnpint_irq_domain_translate(domain, fwspec, &hwirq, &type);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < nr_irqs; i++)
+		qpnpint_irq_domain_map(pmic_arb, domain, virq + i, hwirq + i);
+
 	return 0;
 }
 
@@ -1126,8 +1142,10 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
 };
 
 static const struct irq_domain_ops pmic_arb_irq_domain_ops = {
-	.map	= qpnpint_irq_domain_map,
-	.xlate	= qpnpint_irq_domain_dt_translate,
+	.activate = qpnpint_irq_domain_activate,
+	.alloc = qpnpint_irq_domain_alloc,
+	.free = irq_domain_free_irqs_common,
+	.translate = qpnpint_irq_domain_translate,
 };
 
 static int spmi_pmic_arb_probe(struct platform_device *pdev)
-- 
2.17.2

[PATCH v3 3/6] gpio: add irq domain activate/deactivate functions

[Brian Masney]

This adds the two new functions gpiochip_irq_domain_activate and
gpiochip_irq_domain_deactivate that can be used as the activate and
deactivate functions in the struct irq_domain_ops. This is for
situations where only gpiochip_{lock,unlock}_as_irq needs to be called.
SPMI and SSBI GPIO are two users that will initially use these
functions.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Stephen: Reply if you want a Suggested-by tag for this.

 drivers/gpio/gpiolib.c      | 37 +++++++++++++++++++++++++++++++++++++
 include/linux/gpio/driver.h |  5 +++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1651d7f0a303..1589f34d5c9b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1775,6 +1775,43 @@ static const struct irq_domain_ops gpiochip_domain_ops = {
 	.xlate	= irq_domain_xlate_twocell,
 };
 
+/**
+ * gpiochip_irq_domain_activate() - Lock a GPIO to be used as an IRQ
+ * @domain: The IRQ domain used by this IRQ chip
+ * @data: Outermost irq_data associated with the IRQ
+ * @reserve: If set, only reserve an interrupt vector instead of assigning one
+ *
+ * This function is a wrapper that calls &gpiochip_lock_as_irq and is to be
+ * used as the activate function for the struct &irq_domain_ops. The host_data
+ * for the IRQ domain must be the struct &gpiochip.
+ */
+int gpiochip_irq_domain_activate(struct irq_domain *domain,
+				 struct irq_data *data, bool reserve)
+{
+	struct gpio_chip *chip = domain->host_data;
+
+	return gpiochip_lock_as_irq(chip, data->hwirq);
+}
+EXPORT_SYMBOL_GPL(gpiochip_irq_domain_activate);
+
+/**
+ * gpiochip_irq_domain_deactivate() - Unlock a GPIO used as an IRQ
+ * @domain: The IRQ domain used by this IRQ chip
+ * @data: Outermost irq_data associated with the IRQ
+ *
+ * This function is a wrapper that will call &gpiochip_unlock_as_irq and is to
+ * be used as the deactivate function for the struct &irq_domain_ops. The
+ * host_data for the IRQ domain must be the struct &gpiochip.
+ */
+void gpiochip_irq_domain_deactivate(struct irq_domain *domain,
+				    struct irq_data *data)
+{
+	struct gpio_chip *chip = domain->host_data;
+
+	return gpiochip_unlock_as_irq(chip, data->hwirq);
+}
+EXPORT_SYMBOL_GPL(gpiochip_irq_domain_deactivate);
+
 static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 	if (!gpiochip_irqchip_irq_valid(chip, offset))
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 07cddbf45186..01497910f023 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -472,6 +472,11 @@ int gpiochip_irq_map(struct irq_domain *d, unsigned int irq,
 		     irq_hw_number_t hwirq);
 void gpiochip_irq_unmap(struct irq_domain *d, unsigned int irq);
 
+int gpiochip_irq_domain_activate(struct irq_domain *domain,
+				 struct irq_data *data, bool reserve);
+void gpiochip_irq_domain_deactivate(struct irq_domain *domain,
+				    struct irq_data *data);
+
 void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip,
 		struct irq_chip *irqchip,
 		unsigned int parent_irq,
-- 
2.17.2

[PATCH v3 4/6] qcom: spmi-gpio: add support for hierarchical IRQ chip

[Brian Masney]

spmi-gpio did not have any irqchip support so consumers of this in
device tree would need to call gpio[d]_to_irq() in order to get the
proper IRQ on the underlying PMIC. IRQ chips in device tree should
be usable from the start without the consumer having to make an
additional call to get the proper IRQ on the parent. This patch adds
hierarchical IRQ chip support to the spmi-gpio code to correct this
issue.

Driver was tested using the volume buttons (via gpio-keys) on the LG
Nexus 5 (hammerhead) phone with the following two configurations.

volume-up {
        interrupts-extended = <&pm8941_gpios 2 IRQ_TYPE_EDGE_BOTH>;
        ...
};

volume-up {
        gpios = <&pm8941_gpios 2 GPIO_ACTIVE_LOW>;
        ...
};

Both configurations now show that spmi-gpio is the IRQ domain and that
the IRQ is setup in a hierarchy.

$ grep volume_up /proc/interrupts
 72:          6          0  spmi-gpio   1 Edge      volume_up

$ cat /sys/kernel/debug/irq/irqs/72
handler:  handle_edge_irq
device:   (null)
status:   0x00000403
            _IRQ_NOPROBE
istate:   0x00000000
ddepth:   0
wdepth:   0
dstate:   0x02400203
            IRQ_TYPE_EDGE_RISING
            IRQ_TYPE_EDGE_FALLING
            IRQD_ACTIVATED
            IRQD_IRQ_STARTED
node:     0
affinity: 0-3
effectiv: 
domain:  :soc:spmi@fc4cf000:pm8941@0:gpios@c000
 hwirq:   0x1
 chip:    spmi-gpio
  flags:   0x4
             IRQCHIP_MASK_ON_SUSPEND
 parent:
    domain:  :soc:spmi@fc4cf000
     hwirq:   0xc100057
     chip:    pmic_arb
      flags:   0x4
                 IRQCHIP_MASK_ON_SUSPEND

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v2:
- Use PMIC_GPIO_PHYSICAL_OFFSET instead of the 1 constant
- Use gpiochip_irq_domain_{activate,deactivate}
- Changed 'fwspec->param[0] + 0xc0 - 1' to 'hwirq + c0' in call to
  irq_domain_alloc_irqs_parent

Changes since v1:
- Use two cells for interrupts instead of four.
- Pin numbers in interrupts-extended are now one based instead of zero
  based so that they match the GPIO pin number.
- Drop unnecessary parenthesis in pmic_gpio_domain_translate
- Add missing of_node_put()
- Remove irq field from pmic_gpio_pad struct that is no longer
  necessary.

 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 111 +++++++++++++++++++++--
 1 file changed, 101 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index d3ce7a0e1203..a1f411a0501e 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -12,6 +12,7 @@
  */
 
 #include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
@@ -136,7 +137,6 @@ enum pmic_gpio_func_index {
 /**
  * struct pmic_gpio_pad - keep current GPIO settings
  * @base: Address base in SPMI device.
- * @irq: IRQ number which this GPIO generate.
  * @is_enabled: Set to false when GPIO should be put in high Z state.
  * @out_value: Cached pin output value
  * @have_buffer: Set to true if GPIO output could be configured in push-pull,
@@ -156,7 +156,6 @@ enum pmic_gpio_func_index {
  */
 struct pmic_gpio_pad {
 	u16		base;
-	int		irq;
 	bool		is_enabled;
 	bool		out_value;
 	bool		have_buffer;
@@ -179,6 +178,8 @@ 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[] = {
@@ -761,11 +762,14 @@ static int pmic_gpio_of_xlate(struct gpio_chip *chip,
 static int pmic_gpio_to_irq(struct gpio_chip *chip, unsigned pin)
 {
 	struct pmic_gpio_state *state = gpiochip_get_data(chip);
-	struct pmic_gpio_pad *pad;
+	struct irq_fwspec fwspec;
 
-	pad = state->ctrl->desc->pins[pin].drv_data;
+	fwspec.fwnode = state->fwnode;
+	fwspec.param_count = 2;
+	fwspec.param[0] = pin + PMIC_GPIO_PHYSICAL_OFFSET;
+	fwspec.param[1] = IRQ_TYPE_NONE;
 
-	return pad->irq;
+	return irq_create_fwspec_mapping(&fwspec);
 }
 
 static void pmic_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
@@ -935,8 +939,78 @@ static int pmic_gpio_populate(struct pmic_gpio_state *state,
 	return 0;
 }
 
+static struct irq_chip pmic_gpio_irq_chip = {
+	.name = "spmi-gpio",
+	.irq_ack = irq_chip_ack_parent,
+	.irq_mask = irq_chip_mask_parent,
+	.irq_unmask = irq_chip_unmask_parent,
+	.irq_set_type = irq_chip_set_type_parent,
+	.irq_set_wake = irq_chip_set_wake_parent,
+	.flags = IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static int pmic_gpio_domain_translate(struct irq_domain *domain,
+				      struct irq_fwspec *fwspec,
+				      unsigned long *hwirq,
+				      unsigned int *type)
+{
+	struct pmic_gpio_state *state = container_of(domain->host_data,
+						     struct pmic_gpio_state,
+						     chip);
+
+	if (fwspec->param_count != 2 || fwspec->param[0] >= state->chip.ngpio)
+		return -EINVAL;
+
+	*hwirq = fwspec->param[0] - PMIC_GPIO_PHYSICAL_OFFSET;
+	*type = fwspec->param[1];
+
+	return 0;
+}
+
+static int pmic_gpio_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				  unsigned int nr_irqs, void *data)
+{
+	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);
+
+	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];
+
+	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
+					    &parent_fwspec);
+}
+
+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;
+	struct device_node *parent_node;
 	struct device *dev = &pdev->dev;
 	struct pinctrl_pin_desc *pindesc;
 	struct pinctrl_desc *pctrldesc;
@@ -993,10 +1067,6 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 		pindesc->number = i;
 		pindesc->name = pmic_gpio_groups[i];
 
-		pad->irq = platform_get_irq(pdev, i);
-		if (pad->irq < 0)
-			return pad->irq;
-
 		pad->base = reg + i * PMIC_GPIO_ADDRESS_RANGE;
 
 		ret = pmic_gpio_populate(state, pad);
@@ -1016,10 +1086,28 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(state->ctrl))
 		return PTR_ERR(state->ctrl);
 
+	parent_node = of_irq_find_parent(state->dev->of_node);
+	if (!parent_node)
+		return -ENXIO;
+
+	parent_domain = irq_find_host(parent_node);
+	of_node_put(parent_node);
+	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;
+
 	ret = gpiochip_add_data(&state->chip, state);
 	if (ret) {
 		dev_err(state->dev, "can't add gpio chip\n");
-		return ret;
+		goto err_chip_add_data;
 	}
 
 	/*
@@ -1045,6 +1133,8 @@ 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;
 }
 
@@ -1053,6 +1143,7 @@ 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.17.2

[PATCH v3 5/6] ARM: dts: qcom: pm8941: add interrupt controller properties

[Brian Masney]

Add interrupt controller properties now that spmi-gpio is a proper
hierarchical IRQ chip. The interrupts property is no longer needed so
remove it. Code was tested on the LG Nexus 5 (hammerhead) phone.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v2:
- Remove interrupt-parent property

Changes since v1:
- Interrupts are now two cells instead of four cells.
- Drop unnecessary interrupts property.

 arch/arm/boot/dts/qcom-pm8941.dtsi | 38 ++----------------------------
 1 file changed, 2 insertions(+), 36 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
index 2515c5c217ac..e65049e0fb45 100644
--- a/arch/arm/boot/dts/qcom-pm8941.dtsi
+++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
@@ -64,42 +64,8 @@
 			reg = <0xc000>;
 			gpio-controller;
 			#gpio-cells = <2>;
-			interrupts = <0 0xc0 0 IRQ_TYPE_NONE>,
-				     <0 0xc1 0 IRQ_TYPE_NONE>,
-				     <0 0xc2 0 IRQ_TYPE_NONE>,
-				     <0 0xc3 0 IRQ_TYPE_NONE>,
-				     <0 0xc4 0 IRQ_TYPE_NONE>,
-				     <0 0xc5 0 IRQ_TYPE_NONE>,
-				     <0 0xc6 0 IRQ_TYPE_NONE>,
-				     <0 0xc7 0 IRQ_TYPE_NONE>,
-				     <0 0xc8 0 IRQ_TYPE_NONE>,
-				     <0 0xc9 0 IRQ_TYPE_NONE>,
-				     <0 0xca 0 IRQ_TYPE_NONE>,
-				     <0 0xcb 0 IRQ_TYPE_NONE>,
-				     <0 0xcc 0 IRQ_TYPE_NONE>,
-				     <0 0xcd 0 IRQ_TYPE_NONE>,
-				     <0 0xce 0 IRQ_TYPE_NONE>,
-				     <0 0xcf 0 IRQ_TYPE_NONE>,
-				     <0 0xd0 0 IRQ_TYPE_NONE>,
-				     <0 0xd1 0 IRQ_TYPE_NONE>,
-				     <0 0xd2 0 IRQ_TYPE_NONE>,
-				     <0 0xd3 0 IRQ_TYPE_NONE>,
-				     <0 0xd4 0 IRQ_TYPE_NONE>,
-				     <0 0xd5 0 IRQ_TYPE_NONE>,
-				     <0 0xd6 0 IRQ_TYPE_NONE>,
-				     <0 0xd7 0 IRQ_TYPE_NONE>,
-				     <0 0xd8 0 IRQ_TYPE_NONE>,
-				     <0 0xd9 0 IRQ_TYPE_NONE>,
-				     <0 0xda 0 IRQ_TYPE_NONE>,
-				     <0 0xdb 0 IRQ_TYPE_NONE>,
-				     <0 0xdc 0 IRQ_TYPE_NONE>,
-				     <0 0xdd 0 IRQ_TYPE_NONE>,
-				     <0 0xde 0 IRQ_TYPE_NONE>,
-				     <0 0xdf 0 IRQ_TYPE_NONE>,
-				     <0 0xe0 0 IRQ_TYPE_NONE>,
-				     <0 0xe1 0 IRQ_TYPE_NONE>,
-				     <0 0xe2 0 IRQ_TYPE_NONE>,
-				     <0 0xe3 0 IRQ_TYPE_NONE>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
 
 			boost_bypass_n_pin: boost-bypass {
 				pins = "gpio21";
-- 
2.17.2

[PATCH v3 6/6] ARM: dts: qcom: pma8084: add interrupt controller properties

[Brian Masney]

Add interrupt controller properties now that spmi-gpio is a proper
hierarchical IRQ chip. The interrupts property is no longer needed so
remove it.

This change was not tested on any hardware but the same change was
tested on qcom-pm8941.dtsi using a LG Nexus 5 (hammerhead) phone with
no issues.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 arch/arm/boot/dts/qcom-pma8084.dtsi | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-pma8084.dtsi b/arch/arm/boot/dts/qcom-pma8084.dtsi
index aac7e73b6872..8f5ea7add20f 100644
--- a/arch/arm/boot/dts/qcom-pma8084.dtsi
+++ b/arch/arm/boot/dts/qcom-pma8084.dtsi
@@ -32,28 +32,8 @@
 			reg = <0xc000>;
 			gpio-controller;
 			#gpio-cells = <2>;
-			interrupts = <0 0xc0 0 IRQ_TYPE_NONE>,
-				     <0 0xc1 0 IRQ_TYPE_NONE>,
-				     <0 0xc2 0 IRQ_TYPE_NONE>,
-				     <0 0xc3 0 IRQ_TYPE_NONE>,
-				     <0 0xc4 0 IRQ_TYPE_NONE>,
-				     <0 0xc5 0 IRQ_TYPE_NONE>,
-				     <0 0xc6 0 IRQ_TYPE_NONE>,
-				     <0 0xc7 0 IRQ_TYPE_NONE>,
-				     <0 0xc8 0 IRQ_TYPE_NONE>,
-				     <0 0xc9 0 IRQ_TYPE_NONE>,
-				     <0 0xca 0 IRQ_TYPE_NONE>,
-				     <0 0xcb 0 IRQ_TYPE_NONE>,
-				     <0 0xcc 0 IRQ_TYPE_NONE>,
-				     <0 0xcd 0 IRQ_TYPE_NONE>,
-				     <0 0xce 0 IRQ_TYPE_NONE>,
-				     <0 0xcf 0 IRQ_TYPE_NONE>,
-				     <0 0xd0 0 IRQ_TYPE_NONE>,
-				     <0 0xd1 0 IRQ_TYPE_NONE>,
-				     <0 0xd2 0 IRQ_TYPE_NONE>,
-				     <0 0xd3 0 IRQ_TYPE_NONE>,
-				     <0 0xd4 0 IRQ_TYPE_NONE>,
-				     <0 0xd5 0 IRQ_TYPE_NONE>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
 		};
 
 		pma8084_mpps: mpps@a000 {
-- 
2.17.2

Re: [PATCH v3 6/6] ARM: dts: qcom: pma8084: add interrupt controller properties

[Stephen Boyd]

Quoting Brian Masney (2019-01-09 17:12:58)
> Add interrupt controller properties now that spmi-gpio is a proper
> hierarchical IRQ chip. The interrupts property is no longer needed so
> remove it.
> 
> This change was not tested on any hardware but the same change was
> tested on qcom-pm8941.dtsi using a LG Nexus 5 (hammerhead) phone with
> no issues.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v3 5/6] ARM: dts: qcom: pm8941: add interrupt controller properties

[Stephen Boyd]

Quoting Brian Masney (2019-01-09 17:12:57)
> Add interrupt controller properties now that spmi-gpio is a proper
> hierarchical IRQ chip. The interrupts property is no longer needed so
> remove it. Code was tested on the LG Nexus 5 (hammerhead) phone.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v3 1/6] pinctrl: qcom: spmi-gpio: hardcode IRQ counts

[Stephen Boyd]

Quoting Brian Masney (2019-01-09 17:12:53)
> @@ -1062,14 +1056,15 @@ static int pmic_gpio_remove(struct platform_device *pdev)
>         return 0;
>  }
>  
> +/* data contains the number of GPIOs */
>  static const struct of_device_id pmic_gpio_of_match[] = {
> -       { .compatible = "qcom,pm8916-gpio" },   /* 4 GPIO's */
> -       { .compatible = "qcom,pm8941-gpio" },   /* 36 GPIO's */
> -       { .compatible = "qcom,pm8994-gpio" },   /* 22 GPIO's */
> -       { .compatible = "qcom,pmi8994-gpio" },  /* 10 GPIO's */
> -       { .compatible = "qcom,pma8084-gpio" },  /* 22 GPIO's */
> -       { .compatible = "qcom,pms405-gpio" },   /* 12 GPIO's, holes on 1 9 10 */
> -       { .compatible = "qcom,spmi-gpio" }, /* Generic */
> +       { .compatible = "qcom,pm8916-gpio", .data = (void *) 4 },
> +       { .compatible = "qcom,pm8941-gpio", .data = (void *) 36 },
> +       { .compatible = "qcom,pm8994-gpio", .data = (void *) 22 },
> +       { .compatible = "qcom,pmi8994-gpio", .data = (void *) 10 },
> +       { .compatible = "qcom,pma8084-gpio", .data = (void *) 22 },
> +       /* pms405 has 12 GPIOs with holes on 1, 9, and 10 */
> +       { .compatible = "qcom,pms405-gpio", .data = (void *) 12 },

We've been lazy and not been updating this file.

 $ git grep "qcom,spmi-gpio"
 arch/arm/boot/dts/qcom-pm8941.dtsi:                     compatible = "qcom,pm8941-gpio", "qcom,spmi-gpio";
 arch/arm/boot/dts/qcom-pma8084.dtsi:                    compatible = "qcom,pma8084-gpio", "qcom,spmi-gpio";
 arch/arm64/boot/dts/qcom/pm8005.dtsi:                   compatible = "qcom,pm8005-gpio", "qcom,spmi-gpio";
 arch/arm64/boot/dts/qcom/pm8998.dtsi:                   compatible = "qcom,pm8998-gpio", "qcom,spmi-gpio";
 arch/arm64/boot/dts/qcom/pmi8994.dtsi:                  compatible = "qcom,pmi8994-gpio", "qcom,spmi-gpio";
 arch/arm64/boot/dts/qcom/pmi8998.dtsi:                  compatible = "qcom,pmi8998-gpio", "qcom,spmi-gpio";

So we'll need to add pm8005, pm8998, pmi8998 here with their pin counts
too to avoid breaking those devices with this patch.

Re: [PATCH v3 3/6] gpio: add irq domain activate/deactivate functions

[Stephen Boyd]

Quoting Brian Masney (2019-01-09 17:12:55)
> This adds the two new functions gpiochip_irq_domain_activate and
> gpiochip_irq_domain_deactivate that can be used as the activate and
> deactivate functions in the struct irq_domain_ops. This is for
> situations where only gpiochip_{lock,unlock}_as_irq needs to be called.
> SPMI and SSBI GPIO are two users that will initially use these
> functions.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
> Stephen: Reply if you want a Suggested-by tag for this.

Sure. Add it please.

Suggested-by: Stephen Boyd <sboyd@kernel.org>

Minor question but otherwise

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

> +/**
> + * gpiochip_irq_domain_deactivate() - Unlock a GPIO used as an IRQ
> + * @domain: The IRQ domain used by this IRQ chip
> + * @data: Outermost irq_data associated with the IRQ
> + *
> + * This function is a wrapper that will call &gpiochip_unlock_as_irq and is to

Is this kernel-doc notation to refer to functions and structures with
ampersand only? According to the docs[1] we should put () after
functions and '&struct' before structures.

> + * be used as the deactivate function for the struct &irq_domain_ops. The
> + * host_data for the IRQ domain must be the struct &gpiochip.
> + */
> +void gpiochip_irq_domain_deactivate(struct irq_domain *domain,
> +                                   struct irq_data *data)
> +{
> +       struct gpio_chip *chip = domain->host_data;
> +

[1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html?highlight=cross%20references#highlights-and-cross-references

Re: [PATCH v3 2/6] spmi: pmic-arb: convert to v2 irq interfaces to support hierarchical IRQ chips

[Stephen Boyd]

Quoting Brian Masney (2019-01-09 17:12:54)
> Convert the spmi-pmic-arb IRQ code to use the version 2 IRQ interface
> in order to support hierarchical IRQ chips. This is necessary so that
> spmi-gpio can be setup as a hierarchical IRQ chip with pmic-arb as the
> parent. IRQ chips in device tree should be usable from the start without
> the consumer having to make an additional call to gpio[d]_to_irq() to
> get the proper IRQ on the parent. Driver was tested on a LG Nexus 5
> (hammerhead) phone.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
> Changes since v2:
> - None
> 
> Changes since v1:
> - Add intspec variable to qpnpint_irq_domain_translate to reduce the
>   overall diff.
> - Remove irq_domain_disassociate hack.

I thought we would need to keep that around? And then dispose of the
hack later on. Won't this just wreck the world almost immediately
because the MFD is counting irqs from DT?

Re: [PATCH v3 4/6] qcom: spmi-gpio: add support for hierarchical IRQ chip

[Stephen Boyd]

Quoting Brian Masney (2019-01-09 17:12:56)
> spmi-gpio did not have any irqchip support so consumers of this in
> device tree would need to call gpio[d]_to_irq() in order to get the
> proper IRQ on the underlying PMIC. IRQ chips in device tree should
> be usable from the start without the consumer having to make an
> additional call to get the proper IRQ on the parent. This patch adds
> hierarchical IRQ chip support to the spmi-gpio code to correct this
> issue.
> 
> Driver was tested using the volume buttons (via gpio-keys) on the LG
> Nexus 5 (hammerhead) phone with the following two configurations.
> 
> volume-up {
>         interrupts-extended = <&pm8941_gpios 2 IRQ_TYPE_EDGE_BOTH>;
>         ...
> };
> 
> volume-up {
>         gpios = <&pm8941_gpios 2 GPIO_ACTIVE_LOW>;
>         ...
> };
> 
> Both configurations now show that spmi-gpio is the IRQ domain and that
> the IRQ is setup in a hierarchy.
> 
> $ grep volume_up /proc/interrupts
>  72:          6          0  spmi-gpio   1 Edge      volume_up
> 
> $ cat /sys/kernel/debug/irq/irqs/72
> handler:  handle_edge_irq
> device:   (null)
> status:   0x00000403
>             _IRQ_NOPROBE
> istate:   0x00000000
> ddepth:   0
> wdepth:   0
> dstate:   0x02400203
>             IRQ_TYPE_EDGE_RISING
>             IRQ_TYPE_EDGE_FALLING
>             IRQD_ACTIVATED
>             IRQD_IRQ_STARTED
> node:     0
> affinity: 0-3
> effectiv: 
> domain:  :soc:spmi@fc4cf000:pm8941@0:gpios@c000
>  hwirq:   0x1
>  chip:    spmi-gpio
>   flags:   0x4
>              IRQCHIP_MASK_ON_SUSPEND
>  parent:
>     domain:  :soc:spmi@fc4cf000
>      hwirq:   0xc100057
>      chip:    pmic_arb
>       flags:   0x4
>                  IRQCHIP_MASK_ON_SUSPEND
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v3 2/6] spmi: pmic-arb: convert to v2 irq interfaces to support hierarchical IRQ chips

[Brian Masney]

On Fri, Jan 11, 2019 at 02:07:09PM -0800, Stephen Boyd wrote:
> Quoting Brian Masney (2019-01-09 17:12:54)
> > Convert the spmi-pmic-arb IRQ code to use the version 2 IRQ interface
> > in order to support hierarchical IRQ chips. This is necessary so that
> > spmi-gpio can be setup as a hierarchical IRQ chip with pmic-arb as the
> > parent. IRQ chips in device tree should be usable from the start without
> > the consumer having to make an additional call to gpio[d]_to_irq() to
> > get the proper IRQ on the parent. Driver was tested on a LG Nexus 5
> > (hammerhead) phone.
> > 
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > ---
> > Changes since v2:
> > - None
> > 
> > Changes since v1:
> > - Add intspec variable to qpnpint_irq_domain_translate to reduce the
> >   overall diff.
> > - Remove irq_domain_disassociate hack.
> 
> I thought we would need to keep that around? And then dispose of the
> hack later on. Won't this just wreck the world almost immediately
> because the MFD is counting irqs from DT?

We won't run into the issue with the MFD counting the IRQs since the
interrupts property was removed and the interrupt-controller property
was added to device tree in the same patch. The interrupts property that
we ultimately don't need was the issue that I ran into with the MFD
subsystem.

However to be certain, I retested and we need the temporary hack since
pmic_gpio_to_irq in this series uses the new SPMI IRQ controller. It
breaks as soon as patch 4 in this series (qcom: spmi-gpio: add support
for hierarchical IRQ chip) is applied but starts working again at patch
5 (ARM: dts: qcom: pm8941: add interrupt controller).

I'll respin this series this weekend with the temporary hack and drop it
later in the series. Sorry about the noise and thanks for the
reviewed-bys.

Brian

Re: [PATCH v3 2/6] spmi: pmic-arb: convert to v2 irq interfaces to support hierarchical IRQ chips

[Stephen Boyd]

Quoting Brian Masney (2019-01-11 15:00:04)
> On Fri, Jan 11, 2019 at 02:07:09PM -0800, Stephen Boyd wrote:
> > Quoting Brian Masney (2019-01-09 17:12:54)
> > > Convert the spmi-pmic-arb IRQ code to use the version 2 IRQ interface
> > > in order to support hierarchical IRQ chips. This is necessary so that
> > > spmi-gpio can be setup as a hierarchical IRQ chip with pmic-arb as the
> > > parent. IRQ chips in device tree should be usable from the start without
> > > the consumer having to make an additional call to gpio[d]_to_irq() to
> > > get the proper IRQ on the parent. Driver was tested on a LG Nexus 5
> > > (hammerhead) phone.
> > > 
> > > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > > ---
> > > Changes since v2:
> > > - None
> > > 
> > > Changes since v1:
> > > - Add intspec variable to qpnpint_irq_domain_translate to reduce the
> > >   overall diff.
> > > - Remove irq_domain_disassociate hack.
> > 
> > I thought we would need to keep that around? And then dispose of the
> > hack later on. Won't this just wreck the world almost immediately
> > because the MFD is counting irqs from DT?
> 
> We won't run into the issue with the MFD counting the IRQs since the
> interrupts property was removed and the interrupt-controller property
> was added to device tree in the same patch. The interrupts property that
> we ultimately don't need was the issue that I ran into with the MFD
> subsystem.
> 
> However to be certain, I retested and we need the temporary hack since
> pmic_gpio_to_irq in this series uses the new SPMI IRQ controller. It
> breaks as soon as patch 4 in this series (qcom: spmi-gpio: add support
> for hierarchical IRQ chip) is applied but starts working again at patch
> 5 (ARM: dts: qcom: pm8941: add interrupt controller).
> 
> I'll respin this series this weekend with the temporary hack and drop it
> later in the series. Sorry about the noise and thanks for the
> reviewed-bys.
> 

Ok sounds good. Can you also fix all the spmi-gpio specifying dts files?
There are more than just two PMIC dts files that need changes (per my
grep results sent to the list earlier).

Re: [PATCH v3 2/6] spmi: pmic-arb: convert to v2 irq interfaces to support hierarchical IRQ chips

[Brian Masney]

On Fri, Jan 11, 2019 at 03:39:27PM -0800, Stephen Boyd wrote:
> Ok sounds good. Can you also fix all the spmi-gpio specifying dts files?
> There are more than just two PMIC dts files that need changes (per my
> grep results sent to the list earlier).

Yes, I'll take care of all of them in the next version.

Brian