Re: RFC: qcom spmi/ssbi-gpio hierarchical irqchip problems

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Dec 17, 2018 at 02:00:14PM +0100, Linus Walleij wrote:
> Tell me if you need some specific help!

Hi Linus,

I attached my latest work. I have the IRQ hierarchy created, and
interrupts are now working properly on spmi-arb for the ADCs and other
devices. When I press the volume up button on the phone, I see an
interrupt fire on the parent IRQ domain (Linux virq 54 on spmi-arb)
instead of on the child IRQ domain (Linux virq 110 on spmi-gpio). Here
is some information from the board that shows that the IRQ hierarchy is
setup:

/ # grep volume /proc/interrupts 
110:          0          0  spmi-gpio   1 Edge      volume_up
111:          0          0  spmi-gpio   2 Edge      volume_down
# Note: 54/55 are the parent virqs and they don't show up in
# /proc/interrupts as I would expect.


/ # cat /sys/kernel/debug/irq/irqs/110 
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

Do I need to in the parent IRQ domain somehow translate the parent virq
to the child virq and call generic_handle_irq() on that child virq? Or
something else?

Brian
>From 3d74d23bac7d4bbaa1612b4eeb11ed0e8d969bdd Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@xxxxxxxxxxxxx>
Date: Tue, 25 Dec 2018 19:39:05 -0500
Subject: [PATCH 1/3] spmi: pmic-arb: convert to v2 irq interfaces to support
 hierarchical IRQ chips

This patch is a work in progress that converts the IRQ code to use the
version 2 interfaces in order to support hierarchical IRQ chips. Code
was tested on a LG Nexus 5 (hammerhead) phone.

Signed-off-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
---
 drivers/spmi/spmi-pmic-arb.c | 70 +++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 28 deletions(-)

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 360b8218f322..bec7a94547c6 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,36 +693,37 @@ 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;
 	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]);
+	dev_dbg(&pmic_arb->spmic->dev,
+		"param[0] 0x%1x param[1] 0x%02x param[2] 0x%02x\n",
+		fwspec->param[0], fwspec->param[1], fwspec->param[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)
+	if (fwspec->param[0] > 0xF || fwspec->param[1] > 0xFF ||
+	    fwspec->param[2] > 0x7)
 		return -EINVAL;
 
-	ppid = intspec[0] << 8 | intspec[1];
+	ppid = fwspec->param[0] << 8 | fwspec->param[1];
 	rc = pmic_arb->ver_ops->ppid_to_apid(pmic_arb, ppid);
 	if (rc < 0) {
-		dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u rc = %d\n",
-		intspec[0], intspec[1], intspec[2], rc);
+		dev_dbg(&pmic_arb->spmic->dev,
+			"failed to xlate sid = %#x, periph = %#x, irq = %u rc = %d\n",
+			fwspec->param[0], fwspec->param[1], fwspec->param[2],
+			rc);
 		return rc;
 	}
 
@@ -732,25 +734,35 @@ static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
 	if (apid < pmic_arb->min_apid)
 		pmic_arb->min_apid = apid;
 
-	*out_hwirq = spec_to_hwirq(intspec[0], intspec[1], intspec[2], apid);
-	*out_type  = intspec[3] & IRQ_TYPE_SENSE_MASK;
+	*out_hwirq = spec_to_hwirq(fwspec->param[0], fwspec->param[1],
+				   fwspec->param[2], apid);
+	*out_type  = fwspec->param[3] & IRQ_TYPE_SENSE_MASK;
 
 	dev_dbg(&pmic_arb->spmic->dev, "out_hwirq = %lu\n", *out_hwirq);
 
 	return 0;
 }
 
-static int qpnpint_irq_domain_map(struct irq_domain *d,
-				  unsigned int virq,
-				  irq_hw_number_t hwirq)
+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 = d->host_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;
+
+	if (WARN_ON(nr_irqs != 1))
+		return -EINVAL;
+
+	ret = qpnpint_irq_domain_translate(domain, fwspec, &hwirq, &type);
+	if (ret)
+		return ret;
 
-	dev_dbg(&pmic_arb->spmic->dev, "virq = %u, hwirq = %lu\n", virq, hwirq);
+	irq_domain_set_info(domain, virq, hwirq, &pmic_arb_irqchip, pmic_arb,
+			    handle_level_irq, NULL, NULL);
 
-	irq_set_chip_and_handler(virq, &pmic_arb_irqchip, handle_level_irq);
-	irq_set_chip_data(virq, d->host_data);
-	irq_set_noprobe(virq);
 	return 0;
 }
 
@@ -1126,8 +1138,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

>From 1bac1d35e084bf626dbb68facf752d0304d85d50 Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@xxxxxxxxxxxxx>
Date: Tue, 25 Dec 2018 19:42:40 -0500
Subject: [PATCH 2/3] qcom: spmi-gpio: convert to hierarchical IRQ chip

This is a work in progress that adds support for hierarchical irqs.

Signed-off-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
---
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 120 ++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 4458d44dfcf6..e45b415c8d1b 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>
@@ -179,6 +180,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 +764,16 @@ 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 = 4;
+	fwspec.param[0] = 0;
+	fwspec.param[1] = pin;
+	fwspec.param[2] = 0;
+	fwspec.param[3] = 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 +943,92 @@ 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_irq_activate(struct irq_domain *domain,
+				  struct irq_data *data, bool reserve)
+{
+	struct pmic_gpio_state *state = domain->host_data;
+
+	return gpiochip_lock_as_irq(&state->chip, data->hwirq);
+}
+
+static void pmic_gpio_irq_deactivate(struct irq_domain *domain,
+				     struct irq_data *data)
+{
+	struct pmic_gpio_state *state = domain->host_data;
+
+	gpiochip_unlock_as_irq(&state->chip, data->hwirq);
+}
+
+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 = domain->host_data;
+
+	if ((fwspec->param_count != 4) ||
+	    (fwspec->param[1] >= state->chip.ngpio))
+		return -EINVAL;
+
+	*hwirq = fwspec->param[1];
+	*type = fwspec->param[3];
+
+	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 = domain->host_data;
+	struct irq_fwspec *fwspec = data;
+	struct irq_fwspec parent_fwspec;
+	irq_hw_number_t hwirq;
+	unsigned int type;
+	int ret;
+
+	if (WARN_ON(nr_irqs != 1))
+		return -EINVAL;
+
+	ret = pmic_gpio_domain_translate(domain, fwspec, &hwirq, &type);
+	if (ret)
+		return ret;
+
+	irq_domain_set_info(domain, virq, hwirq, &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] = fwspec->param[0];
+	parent_fwspec.param[1] = fwspec->param[1] + 0xc0;
+	parent_fwspec.param[2] = fwspec->param[2];
+	parent_fwspec.param[3] = fwspec->param[3];
+
+	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
+					    &parent_fwspec);
+}
+
+static const struct irq_domain_ops pmic_gpio_domain_ops = {
+	.activate = pmic_gpio_irq_activate,
+	.alloc = pmic_gpio_domain_alloc,
+	.deactivate = pmic_gpio_irq_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;
@@ -1022,10 +1114,27 @@ 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);
+	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);
+	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;
 	}
 
 	/*
@@ -1051,6 +1160,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;
 }
 
@@ -1059,6 +1170,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

>From 718427e670d2210dc50ec02d2e02f890fc7453dd Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@xxxxxxxxxxxxx>
Date: Tue, 25 Dec 2018 19:47:11 -0500
Subject: [PATCH 3/3] irq hierarchy test

This is what I am using to test that hierarchical irqs are working
correctly on spmi-gpio without calling gpio[d]_to_irq().

Since pmic_gpio_to_irq() now calls irq_create_fwspec_mapping() in this
patch series, /proc/interrupts correctly shows spmi-gpio as the owner of
the interrupt.

This particular patch won't be submitted for inclusion in upstream once
this series is ready.

Signed-off-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
---
 arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts | 6 ++++--
 arch/arm/boot/dts/qcom-pm8941.dtsi                       | 3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
index 551d262a044a..92991a985864 100644
--- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -333,14 +333,16 @@
 
 		volume-up {
 			label = "volume_up";
-			gpios = <&pm8941_gpios 2 GPIO_ACTIVE_LOW>;
+			//gpios = <&pm8941_gpios 2 GPIO_ACTIVE_LOW>;
+			interrupts-extended = <&pm8941_gpios 0 1 0 IRQ_TYPE_EDGE_BOTH>;
 			linux,input-type = <1>;
 			linux,code = <KEY_VOLUMEUP>;
 		};
 
 		volume-down {
 			label = "volume_down";
-			gpios = <&pm8941_gpios 3 GPIO_ACTIVE_LOW>;
+			//gpios = <&pm8941_gpios 3 GPIO_ACTIVE_LOW>;
+			interrupts-extended = <&pm8941_gpios 0 2 0 IRQ_TYPE_EDGE_BOTH>;
 			linux,input-type = <1>;
 			linux,code = <KEY_VOLUMEDOWN>;
 		};
diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
index 9a91b758f7aa..d0b7f0eff288 100644
--- a/arch/arm/boot/dts/qcom-pm8941.dtsi
+++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
@@ -65,6 +65,9 @@
 			gpio-controller;
 			gpio-ranges = <&pm8941_gpios 0 0 36>;
 			#gpio-cells = <2>;
+			interrupt-parent = <&spmi_bus>;
+			interrupt-controller;
+			#interrupt-cells = <4>;
 			interrupts = <0 0xc0 0 IRQ_TYPE_NONE>,
 				     <0 0xc1 0 IRQ_TYPE_NONE>,
 				     <0 0xc2 0 IRQ_TYPE_NONE>,
-- 
2.17.2


[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux