[PATCH v2 0/5] qcom: spmi: add support for hierarchical IRQ chip

[PATCH v2 0/5] 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.
The first two patches in this series removes some IRQ count code that
was causing the IRQs to be prematurely initialized. Patch three
converts pmic-arb to use the version 2 IRQ interface. Patches 4/5
add support for the hierarchical IRQ chip to spmi-gpio.

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.

Patches 3-5 need to be applied in the order that are presented here in
order to allow for a proper bisection. Patch 5 depends on the presence
of 1-4. Part of the series can be applied without any breakage.

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 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 (5):
  spmi: pmic-arb: hardcode IRQ counts
  mfd: qcom-spmi-pmic: use devm_mfd_add_devices instead of
    devm_of_platform_populate
  spmi: pmic-arb: convert to v2 irq interfaces to support hierarchical
    IRQ chips
  qcom: spmi-gpio: add support for hierarchical IRQ chip
  ARM: dts: qcom: msm8974: add interrupt controller properties

 arch/arm/boot/dts/qcom-pm8941.dtsi       |  39 +-----
 drivers/mfd/Kconfig                      |   1 +
 drivers/mfd/qcom-spmi-pmic.c             |  61 ++++++++-
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 160 +++++++++++++++++++----
 drivers/spmi/spmi-pmic-arb.c             |  58 +++++---
 5 files changed, 233 insertions(+), 86 deletions(-)

-- 
2.17.2

[PATCH v2 1/5] spmi: pmic-arb: hardcode IRQ counts

[Brian Masney]

The probing of this driver calls platform_irq_count, which will setup
all of the IRQs that are available. This is a problem since some of
these IRQs may be setup in an IRQ hierarchy later in the boot process
by spmi-gpio. This will cause these hwirqs to be associated with
multiple Linux virqs and interrupts will not work as expected. This
patch changes the pmic-arb driver so that the IRQ counts are hard
coded in the data field of the of_device_id structure so that IRQs
are setup on an as-needed basis.

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.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Note: The qcom,pms405-gpio looks suspicious to me. The existing code
will start at gpio1, which is supposed to be a hole according to the
comment. I can fix this in a later patch if there is a desire. I didn't
do it now to try to keep this series as small as possible. Its no
worse than what was there before.

 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 37 ++++++++++++------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 4458d44dfcf6..d910fa6c49fe 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -14,6 +14,7 @@
 #include <linux/gpio/driver.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/of_irq.h>
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinconf.h>
@@ -935,8 +936,23 @@ static int pmic_gpio_populate(struct pmic_gpio_state *state,
 	return 0;
 }
 
+/* data contains the number of GPIOs */
+static const struct of_device_id pmic_gpio_of_match[] = {
+	{ .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 },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, pmic_gpio_of_match);
+
 static int pmic_gpio_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
 	struct pinctrl_pin_desc *pindesc;
 	struct pinctrl_desc *pctrldesc;
@@ -951,13 +967,11 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	npins = platform_irq_count(pdev);
-	if (!npins)
+	of_id = of_match_device(pmic_gpio_of_match, &pdev->dev);
+	if (!of_id)
 		return -EINVAL;
-	if (npins < 0)
-		return npins;
 
-	BUG_ON(npins > ARRAY_SIZE(pmic_gpio_groups));
+	npins = (int) of_id->data;
 
 	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
 	if (!state)
@@ -1062,19 +1076,6 @@ static int pmic_gpio_remove(struct platform_device *pdev)
 	return 0;
 }
 
-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 */
-	{ },
-};
-
-MODULE_DEVICE_TABLE(of, pmic_gpio_of_match);
-
 static struct platform_driver pmic_gpio_driver = {
 	.driver = {
 		   .name = "qcom-spmi-gpio",
-- 
2.17.2

Re: [PATCH v2 1/5] spmi: pmic-arb: hardcode IRQ counts

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5248 bytes --]

Hi Brian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on pinctrl/devel]
[also build test WARNING on v5.0-rc1 next-20190103]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Brian-Masney/qcom-spmi-add-support-for-hierarchical-IRQ-chip/20190107-110503
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All warnings (new ones prefixed by >>):

   drivers/pinctrl//qcom/pinctrl-spmi-gpio.c: In function 'pmic_gpio_probe':
>> drivers/pinctrl//qcom/pinctrl-spmi-gpio.c:974:10: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     npins = (int) of_id->data;
             ^

vim +974 drivers/pinctrl//qcom/pinctrl-spmi-gpio.c

   952	
   953	static int pmic_gpio_probe(struct platform_device *pdev)
   954	{
   955		const struct of_device_id *of_id;
   956		struct device *dev = &pdev->dev;
   957		struct pinctrl_pin_desc *pindesc;
   958		struct pinctrl_desc *pctrldesc;
   959		struct pmic_gpio_pad *pad, *pads;
   960		struct pmic_gpio_state *state;
   961		int ret, npins, i;
   962		u32 reg;
   963	
   964		ret = of_property_read_u32(dev->of_node, "reg", &reg);
   965		if (ret < 0) {
   966			dev_err(dev, "missing base address");
   967			return ret;
   968		}
   969	
   970		of_id = of_match_device(pmic_gpio_of_match, &pdev->dev);
   971		if (!of_id)
   972			return -EINVAL;
   973	
 > 974		npins = (int) of_id->data;
   975	
   976		state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
   977		if (!state)
   978			return -ENOMEM;
   979	
   980		platform_set_drvdata(pdev, state);
   981	
   982		state->dev = &pdev->dev;
   983		state->map = dev_get_regmap(dev->parent, NULL);
   984	
   985		pindesc = devm_kcalloc(dev, npins, sizeof(*pindesc), GFP_KERNEL);
   986		if (!pindesc)
   987			return -ENOMEM;
   988	
   989		pads = devm_kcalloc(dev, npins, sizeof(*pads), GFP_KERNEL);
   990		if (!pads)
   991			return -ENOMEM;
   992	
   993		pctrldesc = devm_kzalloc(dev, sizeof(*pctrldesc), GFP_KERNEL);
   994		if (!pctrldesc)
   995			return -ENOMEM;
   996	
   997		pctrldesc->pctlops = &pmic_gpio_pinctrl_ops;
   998		pctrldesc->pmxops = &pmic_gpio_pinmux_ops;
   999		pctrldesc->confops = &pmic_gpio_pinconf_ops;
  1000		pctrldesc->owner = THIS_MODULE;
  1001		pctrldesc->name = dev_name(dev);
  1002		pctrldesc->pins = pindesc;
  1003		pctrldesc->npins = npins;
  1004		pctrldesc->num_custom_params = ARRAY_SIZE(pmic_gpio_bindings);
  1005		pctrldesc->custom_params = pmic_gpio_bindings;
  1006	#ifdef CONFIG_DEBUG_FS
  1007		pctrldesc->custom_conf_items = pmic_conf_items;
  1008	#endif
  1009	
  1010		for (i = 0; i < npins; i++, pindesc++) {
  1011			pad = &pads[i];
  1012			pindesc->drv_data = pad;
  1013			pindesc->number = i;
  1014			pindesc->name = pmic_gpio_groups[i];
  1015	
  1016			pad->irq = platform_get_irq(pdev, i);
  1017			if (pad->irq < 0)
  1018				return pad->irq;
  1019	
  1020			pad->base = reg + i * PMIC_GPIO_ADDRESS_RANGE;
  1021	
  1022			ret = pmic_gpio_populate(state, pad);
  1023			if (ret < 0)
  1024				return ret;
  1025		}
  1026	
  1027		state->chip = pmic_gpio_gpio_template;
  1028		state->chip.parent = dev;
  1029		state->chip.base = -1;
  1030		state->chip.ngpio = npins;
  1031		state->chip.label = dev_name(dev);
  1032		state->chip.of_gpio_n_cells = 2;
  1033		state->chip.can_sleep = false;
  1034	
  1035		state->ctrl = devm_pinctrl_register(dev, pctrldesc, state);
  1036		if (IS_ERR(state->ctrl))
  1037			return PTR_ERR(state->ctrl);
  1038	
  1039		ret = gpiochip_add_data(&state->chip, state);
  1040		if (ret) {
  1041			dev_err(state->dev, "can't add gpio chip\n");
  1042			return ret;
  1043		}
  1044	
  1045		/*
  1046		 * For DeviceTree-supported systems, the gpio core checks the
  1047		 * pinctrl's device node for the "gpio-ranges" property.
  1048		 * If it is present, it takes care of adding the pin ranges
  1049		 * for the driver. In this case the driver can skip ahead.
  1050		 *
  1051		 * In order to remain compatible with older, existing DeviceTree
  1052		 * files which don't set the "gpio-ranges" property or systems that
  1053		 * utilize ACPI the driver has to call gpiochip_add_pin_range().
  1054		 */
  1055		if (!of_property_read_bool(dev->of_node, "gpio-ranges")) {
  1056			ret = gpiochip_add_pin_range(&state->chip, dev_name(dev), 0, 0,
  1057						     npins);
  1058			if (ret) {
  1059				dev_err(dev, "failed to add pin range\n");
  1060				goto err_range;
  1061			}
  1062		}
  1063	
  1064		return 0;
  1065	
  1066	err_range:
  1067		gpiochip_remove(&state->chip);
  1068		return ret;
  1069	}
  1070	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61775 bytes --]

Re: [PATCH v2 1/5] spmi: pmic-arb: hardcode IRQ counts

[Linus Walleij]

On Mon, Jan 7, 2019 at 3:11 AM Brian Masney <masneyb@onstation.org> wrote:

> Note: The qcom,pms405-gpio looks suspicious to me. The existing code
> will start at gpio1, which is supposed to be a hole according to the
> comment. I can fix this in a later patch if there is a desire. I didn't
> do it now to try to keep this series as small as possible. Its no
> worse than what was there before.

If the range has holes the valid_mask should be set up to mask
them off for this variant.

That would be a separate patch though and hardly your problem
as you say.

Yours,
Linus Walleij

Re: [PATCH v2 1/5] spmi: pmic-arb: hardcode IRQ counts

[Stephen Boyd]

Quoting Brian Masney (2019-01-06 18:11:41)
> @@ -1062,19 +1076,6 @@ static int pmic_gpio_remove(struct platform_device *pdev)
>         return 0;
>  }
>  
> -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 */
> -       { },
> -};
> -
> -MODULE_DEVICE_TABLE(of, pmic_gpio_of_match);
> -

Please don't move the array. It isn't necessary if you use
device_get_match_data() instead of of_match_device().

[PATCH v2 2/5] mfd: qcom-spmi-pmic: use devm_mfd_add_devices instead of devm_of_platform_populate

[Brian Masney]

pmic_spmi_probe calls devm_of_platform_populate, which traverses all
of the children in device tree from the parent down to the children,
grandchildren, etc. of_irq_count is called on most of the nodes (via
of_device_alloc) and this initializes all of the IRQs. Further along in
the boot process, spmi-gpio is initialized as a hierarchical IRQ chip
in a later patch (with spmi-arb as the parent IRQ domain), and the same
hwirq is now associated with two Linux virqs and IRQs will not work as
expected. Correct this issue by using devm_mfd_add_devices to initialize
just the children so that IRQs are initialized on an as-needed basis.

This patch also selects CONFIG_MFD_CORE since this is required by
devm_mfd_add_devices.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/mfd/Kconfig          |  1 +
 drivers/mfd/qcom-spmi-pmic.c | 61 ++++++++++++++++++++++++++++++++++--
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index f461460a2aeb..ef27b3927295 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -960,6 +960,7 @@ config MFD_SPMI_PMIC
 	depends on ARCH_QCOM || COMPILE_TEST
 	depends on OF
 	depends on SPMI
+	select MFD_CORE
 	select REGMAP_SPMI
 	help
 	  This enables support for the Qualcomm SPMI PMICs.
diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
index e2e95de649a4..21aa880e3503 100644
--- a/drivers/mfd/qcom-spmi-pmic.c
+++ b/drivers/mfd/qcom-spmi-pmic.c
@@ -12,10 +12,10 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/mfd/core.h>
 #include <linux/module.h>
 #include <linux/spmi.h>
 #include <linux/regmap.h>
-#include <linux/of_platform.h>
 
 #define PMIC_REV2		0x101
 #define PMIC_REV3		0x102
@@ -124,6 +124,61 @@ static const struct regmap_config spmi_regmap_config = {
 	.fast_io	= true,
 };
 
+static const struct mfd_cell pmic_spmi_cells[] = {
+	{
+		.name = "spmi-mpp",
+		.of_compatible = "qcom,spmi-mpp",
+	},
+	{
+		.name = "spmi-temp-alarm",
+		.of_compatible = "qcom,spmi-temp-alarm",
+	},
+	{
+		.name = "pm8941-rtc",
+		.of_compatible = "qcom,pm8941-rtc",
+	},
+	{
+		.name = "pm8941-pwrkey",
+		.of_compatible = "qcom,pm8941-pwrkey",
+	},
+	{
+		.name = "pm8941-misc",
+		.of_compatible = "qcom,pm8941-misc",
+	},
+	{
+		.name = "pm8941-charger",
+		.of_compatible = "qcom,pm8941-charger",
+	},
+	{
+		.name = "spmi-gpio",
+		.of_compatible = "qcom,spmi-gpio",
+	},
+	{
+		.name = "spmi-mpp",
+		.of_compatible = "qcom,spmi-mpp",
+	},
+	{
+		.name = "spmi-vadc",
+		.of_compatible = "qcom,spmi-vadc",
+	},
+	{
+		.name = "spmi-iadc",
+		.of_compatible = "qcom,spmi-iadc",
+	},
+	{
+		.name = "pm8941-coincell",
+		.of_compatible = "qcom,pm8941-coincell",
+	},
+	{
+		.name = "pm8941-wled",
+		.of_compatible = "qcom,pm8941-wled",
+	},
+	{
+		.name = "pm8941-regulators",
+		.of_compatible = "qcom,pm8941-regulators",
+	},
+};
+
 static int pmic_spmi_probe(struct spmi_device *sdev)
 {
 	struct regmap *regmap;
@@ -136,7 +191,9 @@ static int pmic_spmi_probe(struct spmi_device *sdev)
 	if (sdev->usid % 2 == 0)
 		pmic_spmi_show_revid(regmap, &sdev->dev);
 
-	return devm_of_platform_populate(&sdev->dev);
+	return devm_mfd_add_devices(&sdev->dev, PLATFORM_DEVID_AUTO,
+				    pmic_spmi_cells,
+				    ARRAY_SIZE(pmic_spmi_cells), NULL, 0, NULL);
 }
 
 MODULE_DEVICE_TABLE(of, pmic_spmi_id_table);
-- 
2.17.2

Re: [PATCH v2 2/5] mfd: qcom-spmi-pmic: use devm_mfd_add_devices instead of devm_of_platform_populate

[Linus Walleij]

On Mon, Jan 7, 2019 at 3:11 AM Brian Masney <masneyb@onstation.org> wrote:

> pmic_spmi_probe calls devm_of_platform_populate, which traverses all
> of the children in device tree from the parent down to the children,
> grandchildren, etc. of_irq_count is called on most of the nodes (via
> of_device_alloc) and this initializes all of the IRQs. Further along in
> the boot process, spmi-gpio is initialized as a hierarchical IRQ chip
> in a later patch (with spmi-arb as the parent IRQ domain), and the same
> hwirq is now associated with two Linux virqs and IRQs will not work as
> expected. Correct this issue by using devm_mfd_add_devices to initialize
> just the children so that IRQs are initialized on an as-needed basis.
>
> This patch also selects CONFIG_MFD_CORE since this is required by
> devm_mfd_add_devices.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Good catch! Now I see why this was acting so weird for you.
Also the MFD semantics are standardized and make much more
sense after this.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I suppose Lee can merge this in orthogonal in the MFD tree,
the end result will be functional after the v5.1 merge window.

I think I should also fix qcom-pm8xxx after this, as well as
the similar SSBI code for elder platforms.

Yours,
Linus Walleij

Re: [PATCH v2 2/5] mfd: qcom-spmi-pmic: use devm_mfd_add_devices instead of devm_of_platform_populate

[Brian Masney]

On Mon, Jan 07, 2019 at 11:53:07AM +0100, Linus Walleij wrote:
> I suppose Lee can merge this in orthogonal in the MFD tree,
> the end result will be functional after the v5.1 merge window.

To keep git bisect happy, it would be nice if either all of the patches
go in through the same tree (with the proper ACKs), or if the first four
patches are applied during the v5.1 merge window, and the last patch is
applied during the v5.2 merge window.

Brian

Re: [PATCH v2 2/5] mfd: qcom-spmi-pmic: use devm_mfd_add_devices instead of devm_of_platform_populate

[Stephen Boyd]

Quoting Brian Masney (2019-01-06 18:11:42)
> +       },
> +       {
> +               .name = "pm8941-regulators",
> +               .of_compatible = "qcom,pm8941-regulators",
> +       },
> +};
> +
>  static int pmic_spmi_probe(struct spmi_device *sdev)
>  {
>         struct regmap *regmap;
> @@ -136,7 +191,9 @@ static int pmic_spmi_probe(struct spmi_device *sdev)
>         if (sdev->usid % 2 == 0)
>                 pmic_spmi_show_revid(regmap, &sdev->dev);
>  
> -       return devm_of_platform_populate(&sdev->dev);
> +       return devm_mfd_add_devices(&sdev->dev, PLATFORM_DEVID_AUTO,
> +                                   pmic_spmi_cells,
> +                                   ARRAY_SIZE(pmic_spmi_cells), NULL, 0, NULL);

Now this seems worse. We have gotten by without having to explicitly
list all the devices that are inside the PMIC as mfd cells. But now, to
avoid creating the irqs before the hierarchy is installed, we have to
undo all of that and rely on the difference in behavior of
of_platform_populate() and mfd_add_devices(). That's pretty obscure to
figure out.

I'd prefer we drop this patch and keep disassociating virqs and
reassociating them in the gpio driver. Then we can remove the interrupts
properties in all the DTS files and finally remove the disassociate and
reassociating code in the gpio driver when all the DT files are cleaned
up. It makes things less confusing that way and doesn't require updates
to this driver.

Re: [PATCH v2 2/5] mfd: qcom-spmi-pmic: use devm_mfd_add_devices instead of devm_of_platform_populate

[Brian Masney]

On Mon, Jan 07, 2019 at 01:41:30PM -0800, Stephen Boyd wrote:
> Now this seems worse. We have gotten by without having to explicitly
> list all the devices that are inside the PMIC as mfd cells. But now, to
> avoid creating the irqs before the hierarchy is installed, we have to
> undo all of that and rely on the difference in behavior of
> of_platform_populate() and mfd_add_devices(). That's pretty obscure to
> figure out.
> 
> I'd prefer we drop this patch and keep disassociating virqs and
> reassociating them in the gpio driver. Then we can remove the interrupts
> properties in all the DTS files and finally remove the disassociate and
> reassociating code in the gpio driver when all the DT files are cleaned
> up. It makes things less confusing that way and doesn't require updates
> to this driver.

You are right that we can get this working without this patch. The issue
that I experienced was caused by the interrupts property on the
spmi-gpio node. I thought that I tested this with that configuration but
I obviously didn't.

qcom-pm8941.dtsi and qcom-pma8084.dtsi are the only two in-tree users of
spmi-gpio. I'll include the fix for qcom-pma8084.dtsi as well.

Brian

[PATCH v2 3/5] 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 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 v2 4/5] 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
 64:          0          0  spmi-gpio   1 Edge      volume_up

$ cat /sys/kernel/debug/irq/irqs/64
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 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 | 123 +++++++++++++++++++++--
 1 file changed, 113 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index d910fa6c49fe..66b24fa70b3a 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_device.h>
@@ -137,7 +138,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,
@@ -157,7 +157,6 @@ enum pmic_gpio_func_index {
  */
 struct pmic_gpio_pad {
 	u16		base;
-	int		irq;
 	bool		is_enabled;
 	bool		out_value;
 	bool		have_buffer;
@@ -180,6 +179,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[] = {
@@ -762,11 +763,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 + 1;
+	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)
@@ -936,6 +940,86 @@ 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 != 2 || fwspec->param[0] >= state->chip.ngpio)
+		return -EINVAL;
+
+	*hwirq = fwspec->param[0] - 1;
+	*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 = domain->host_data;
+	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] = fwspec->param[0] + 0xc0 - 1;
+	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 = 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,
+};
+
 /* data contains the number of GPIOs */
 static const struct of_device_id pmic_gpio_of_match[] = {
 	{ .compatible = "qcom,pm8916-gpio", .data = (void *) 4 },
@@ -953,6 +1037,8 @@ MODULE_DEVICE_TABLE(of, pmic_gpio_of_match);
 static int pmic_gpio_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id;
+	struct irq_domain *parent_domain;
+	struct device_node *parent_node;
 	struct device *dev = &pdev->dev;
 	struct pinctrl_pin_desc *pindesc;
 	struct pinctrl_desc *pctrldesc;
@@ -1013,10 +1099,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);
@@ -1036,10 +1118,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);
+	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;
 	}
 
 	/*
@@ -1065,6 +1165,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;
 }
 
@@ -1073,6 +1175,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

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

[Stephen Boyd]

Quoting Brian Masney (2019-01-06 18:11:44)
> @@ -762,11 +763,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 + 1;

There seems to be PMIC_GPIO_PHYSICAL_OFFSET for these purposes.

> +       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)
> @@ -936,6 +940,86 @@ 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);
> +}

I meant these sorts of wrappers that could be used by other drivers
possibly, if all they need to store in their domain->host_data is the
gpiochip structure (they can probably indirect through the gpiochip
again to get their driver specific structure).

int gpiochip_irq_domain_activate(struct irq_domain *domain,
				 struct irq_data *data, bool reserve)
{
	struct gpiochip *chip = domain->host_data;

	return gpiochip_lock_as_irq(chip, data->hwirq);
}

void gpiochip_irq_domain_deactivate(struct irq_domain *domain,
				    struct irq_data *data)
{
	struct gpiochip *chip = domain->host_data;

	return gpiochip_unlock_as_irq(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 != 2 || fwspec->param[0] >= state->chip.ngpio)
> +               return -EINVAL;
> +
> +       *hwirq = fwspec->param[0] - 1;

There seems to be PMIC_GPIO_PHYSICAL_OFFSET for these purposes.

> +       *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 = domain->host_data;
> +       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] = fwspec->param[0] + 0xc0 - 1;

Is the -1 there twice, once in pmic_gpio_domain_translate() and then
here?

> +       parent_fwspec.param[2] = 0;
> +       parent_fwspec.param[3] = fwspec->param[1];
> +
> +       return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> +                                           &parent_fwspec);
> +}
> +

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

[Brian Masney]

On Mon, Jan 07, 2019 at 01:55:42PM -0800, Stephen Boyd wrote:
> > +       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] = fwspec->param[0] + 0xc0 - 1;
> 
> Is the -1 there twice, once in pmic_gpio_domain_translate() and then
> here?

That should be 'hwirq + c0' to improve the code readability.

Brian

[PATCH v2 5/5] ARM: dts: qcom: msm8974: 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.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v1:
- Interrupts are now two cells instead of four cells.
- Drop unnecessary interrupts property.

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

diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
index 2515c5c217ac..16e6b4272d16 100644
--- a/arch/arm/boot/dts/qcom-pm8941.dtsi
+++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
@@ -64,42 +64,9 @@
 			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-parent = <&spmi_bus>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
 
 			boost_bypass_n_pin: boost-bypass {
 				pins = "gpio21";
-- 
2.17.2

Re: [PATCH v2 5/5] ARM: dts: qcom: msm8974: add interrupt controller properties

[Stephen Boyd]

Quoting Brian Masney (2019-01-06 18:11:45)
> -                                    <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-parent = <&spmi_bus>;

If I'm reading of_irq_find_parent() correctly, we don't need to specify
this? It becomes implicit because spmi_bus has the interrupt-cells
property and this is a child node of that.

> +                       interrupt-controller;
> +                       #interrupt-cells = <2>;
>  
>                         boost_bypass_n_pin: boost-bypass {
>                                 pins = "gpio21";