On Tue, Dec 04, 2018 at 11:20:54AM +0100, Linus Walleij wrote:
> I'm a bit confused because the PM8xxx has its irqdomain
> inside drivers/mfd/qcom-pm8xxx.c and the PM8941 driver
> in drivers/mfd/qcom-spmi-pmic.c does not.
>
> This on the PM8941 there appears to be no irqchip.
> Then it falls upward to the spmi bus. This is
> (qcom-msm8974.dtsi) an interrupt-controller with
> four cells, so it should be:
>
> interrupt-parent = <&spmi_bus>;
>
> The irqchip and domain of the parent should be what
> you find in drivers/spmi/spmi-pmic-arb.c
I attached what I have so far for hierarchical IRQs for spmi-gpio.
I hope it is alright to attach them directly and not use git send-email.
1) Patch 1 adds an alloc function to spmi-pmic-arb.c so that the IRQ
chip is set properly for the parent IRQ domain.
2) Patch 2 adds the IRQ hierarchy support to pinctrl-spmi-gpio.c.
3) Patch 3 is how I'm planning to test that this is working properly on
the Nexus 5 phone.
With this series, I see a long delay on startup and the following errors
in the console log:
[ 1.374586] spmi spmi-0: pmic_arb_wait_for_done: transaction denied (0x5)
[ 1.393541] qcom-spmi-iadc fc4cf000.spmi:pm8941@0:iadc@3600: conversion failed
[ 1.393587] qcom-spmi-iadc fc4cf000.spmi:pm8941@0:iadc@3600: failed offset calibration
[ 1.399681] spmi spmi-0: pmic_arb_wait_for_done: transaction denied (0x5)
[ 1.407629] qcom-spmi-iadc: probe of fc4cf000.spmi:pm8941@0:iadc@3600 failed with error -110
[ 1.414900] spmi spmi-0: pmic_arb_wait_for_done: transaction denied (0x5)
[ 1.422949] spmi spmi-0: pmic_arb_wait_for_done: transaction denied (0x5)
[ 41.433579] qcom-spmi-vadc fc4cf000.spmi:pm8941@0:vadc@3100: conversion failed
[ 41.433626] qcom-spmi-vadc fc4cf000.spmi:pm8941@0:vadc@3100: measure reference points failed
[ 41.439722] spmi spmi-0: pmic_arb_wait_for_done: transaction denied (0x5)
[ 41.448296] qpnpint_spmi_write: 21 callbacks suppressed
[ 41.448305] spmi spmi-0: failed irqchip transaction on d7
I'm going to keep digging but I'm posting this to see if anyone has any
suggestions.
Brian
>From c47691608856bb93fbbbfd74e4c1387376a7bc56 Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@xxxxxxxxxxxxx>
Date: Sun, 16 Dec 2018 17:31:18 -0500
Subject: [PATCH 1/3] spmi: pmic-arb: irq hierarchy WIP
This is a work in progress that adds support for hierarchical irqs.
Signed-off-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
---
drivers/spmi/spmi-pmic-arb.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 360b8218f322..ce3e72003d95 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -1125,7 +1125,33 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
.apid_map_offset = pmic_arb_apid_map_offset_v5,
};
+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_dt_translate(domain,
+ pmic_arb->spmic->dev.of_node,
+ fwspec->param,
+ fwspec->param_count, &hwirq,
+ &type);
+
+ for (i = 0; i < nr_irqs; i++) {
+ ret = qpnpint_irq_domain_map(domain, virq + i, hwirq + i);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static const struct irq_domain_ops pmic_arb_irq_domain_ops = {
+ .alloc = qpnpint_irq_domain_alloc,
.map = qpnpint_irq_domain_map,
.xlate = qpnpint_irq_domain_dt_translate,
};
--
2.17.2
>From f5a1924dac8212c5ae3bad61530b6f25c398b620 Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@xxxxxxxxxxxxx>
Date: Sun, 16 Dec 2018 20:32:55 -0500
Subject: [PATCH 2/3] pinctrl: qcom: spmi-gpio: irq hierarchy WIP
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 | 131 ++++++++++++++++++++++-
1 file changed, 130 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 4458d44dfcf6..03393cf3b883 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -179,6 +179,7 @@ struct pmic_gpio_state {
struct regmap *map;
struct pinctrl_dev *ctrl;
struct gpio_chip chip;
+ struct irq_domain *parent_domain;
};
static const struct pinconf_generic_params pmic_gpio_bindings[] = {
@@ -935,13 +936,121 @@ static int pmic_gpio_populate(struct pmic_gpio_state *state,
return 0;
}
+static int pmic_gpio_irq_request_resources(struct irq_data *data)
+{
+ struct pmic_gpio_state *state = data->domain->host_data;
+ struct irq_data *parent_data = data->parent_data;
+ int ret;
+
+ ret = gpiochip_lock_as_irq(&state->chip, data->hwirq);
+ if (ret) {
+ dev_err(state->dev, "unable to lock HW IRQ %lu for IRQ: %d\n",
+ data->hwirq, ret);
+ return ret;
+ }
+
+ if (parent_data && parent_data->chip->irq_request_resources) {
+ ret = parent_data->chip->irq_request_resources(parent_data);
+ if (ret)
+ goto error;
+ }
+
+ return 0;
+error:
+ gpiochip_unlock_as_irq(&state->chip, data->hwirq);
+ return ret;
+}
+
+static void pmic_gpio_irq_release_resources(struct irq_data *data)
+{
+ struct pmic_gpio_state *state = data->domain->host_data;
+ struct irq_data *parent_data = data->parent_data;
+
+ if (parent_data && parent_data->chip->irq_release_resources)
+ parent_data->chip->irq_release_resources(parent_data);
+
+ gpiochip_unlock_as_irq(&state->chip, data->hwirq);
+}
+
+static struct irq_chip pmic_gpio_irq_chip = {
+ .name = "spmi-gpio",
+ .irq_eoi = irq_chip_eoi_parent,
+ .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,
+ .irq_request_resources = pmic_gpio_irq_request_resources,
+ .irq_release_resources = pmic_gpio_irq_release_resources,
+};
+
+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;
+
+ ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+ &pmic_gpio_irq_chip, state);
+ if (ret)
+ dev_info(state->dev,
+ "Error setting up domain hwirq and chip: %d", ret);
+
+ 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];
+ 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 = {
+ .alloc = pmic_gpio_domain_alloc,
+ .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 *dev = &pdev->dev;
struct pinctrl_pin_desc *pindesc;
struct pinctrl_desc *pctrldesc;
struct pmic_gpio_pad *pad, *pads;
struct pmic_gpio_state *state;
+ struct fwnode_handle *fwnode;
+ struct device_node *parent;
int ret, npins, i;
u32 reg;
@@ -1022,10 +1131,27 @@ static int pmic_gpio_probe(struct platform_device *pdev)
if (IS_ERR(state->ctrl))
return PTR_ERR(state->ctrl);
+ parent = of_irq_find_parent(state->dev->of_node);
+ if (!parent)
+ return -ENXIO;
+
+ parent_domain = irq_find_host(parent);
+ if (!parent_domain)
+ return -ENXIO;
+
+ fwnode = of_node_to_fwnode(state->dev->of_node);
+ state->parent_domain = irq_domain_create_hierarchy(parent_domain, 0,
+ state->chip.ngpio,
+ fwnode,
+ &pmic_gpio_domain_ops,
+ state);
+ if (!state->parent_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 +1177,8 @@ static int pmic_gpio_probe(struct platform_device *pdev)
err_range:
gpiochip_remove(&state->chip);
+err_chip_add_data:
+ irq_domain_remove(state->parent_domain);
return ret;
}
@@ -1059,6 +1187,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->parent_domain);
return 0;
}
--
2.17.2
>From 3a6fff025d9c6423d2023090700654bb1a3fcff9 Mon Sep 17 00:00:00 2001
From: Brian Masney <masneyb@xxxxxxxxxxxxx>
Date: Sun, 16 Dec 2018 20:33:10 -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.
Signed-off-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
---
arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts | 4 ++--
arch/arm/boot/dts/qcom-pm8941.dtsi | 3 +++
2 files changed, 5 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..afe396293bf3 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,14 @@
volume-up {
label = "volume_up";
- gpios = <&pm8941_gpios 2 GPIO_ACTIVE_LOW>;
+ interrupts-extended = <&pm8941_gpios 0 2 0 IRQ_TYPE_EDGE_RISING>;
linux,input-type = <1>;
linux,code = <KEY_VOLUMEUP>;
};
volume-down {
label = "volume_down";
- gpios = <&pm8941_gpios 3 GPIO_ACTIVE_LOW>;
+ interrupts-extended = <&pm8941_gpios 0 3 0 IRQ_TYPE_EDGE_RISING>;
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
