[PATCH v1] pinctrl: intel: Allocate IRQ chip dynamic

[PATCH v1] pinctrl: intel: Allocate IRQ chip dynamic

[Andy Shevchenko]

Keeping the IRQ chip definition static shares it with multiple instances of
the GPIO chip in the system. This is bad and now we get this warning from
GPIO library:

"detected irqchip that is shared with multiple gpiochips: please fix the driver."

Hence, move the IRQ chip definition from being driver static into the struct
intel_pinctrl. So a unique IRQ chip is used for each GPIO chip instance.

Fixes: ee1a6ca43dba ("Add Intel Broxton pin controller support")
Depends-on: 5ff56b015e85 ("Disable GPIO pin interrupts in suspend")
Reported-by: Federico Ricchiuto <fed.ricchiuto@gmail.com>
Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 1f13bcd0e4e1..bc013599a9a3 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -96,6 +96,7 @@ struct intel_pinctrl_context {
  * @pctldesc: Pin controller description
  * @pctldev: Pointer to the pin controller device
  * @chip: GPIO chip in this pin controller
+ * @irqchip: IRQ chip in this pin controller
  * @soc: SoC/PCH specific pin configuration data
  * @communities: All communities in this pin controller
  * @ncommunities: Number of communities in this pin controller
@@ -108,6 +109,7 @@ struct intel_pinctrl {
 	struct pinctrl_desc pctldesc;
 	struct pinctrl_dev *pctldev;
 	struct gpio_chip chip;
+	struct irq_chip irqchip;
 	const struct intel_pinctrl_soc_data *soc;
 	struct intel_community *communities;
 	size_t ncommunities;
@@ -1139,16 +1141,6 @@ static irqreturn_t intel_gpio_irq(int irq, void *data)
 	return ret;
 }
 
-static struct irq_chip intel_gpio_irqchip = {
-	.name = "intel-gpio",
-	.irq_ack = intel_gpio_irq_ack,
-	.irq_mask = intel_gpio_irq_mask,
-	.irq_unmask = intel_gpio_irq_unmask,
-	.irq_set_type = intel_gpio_irq_type,
-	.irq_set_wake = intel_gpio_irq_wake,
-	.flags = IRQCHIP_MASK_ON_SUSPEND,
-};
-
 static int intel_gpio_add_pin_ranges(struct intel_pinctrl *pctrl,
 				     const struct intel_community *community)
 {
@@ -1198,12 +1190,22 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 
 	pctrl->chip = intel_gpio_chip;
 
+	/* Setup GPIO chip */
 	pctrl->chip.ngpio = intel_gpio_ngpio(pctrl);
 	pctrl->chip.label = dev_name(pctrl->dev);
 	pctrl->chip.parent = pctrl->dev;
 	pctrl->chip.base = -1;
 	pctrl->irq = irq;
 
+	/* Setup IRQ chip */
+	pctrl->irqchip.name = dev_name(pctrl->dev);
+	pctrl->irqchip.irq_ack = intel_gpio_irq_ack;
+	pctrl->irqchip.irq_mask = intel_gpio_irq_mask;
+	pctrl->irqchip.irq_unmask = intel_gpio_irq_unmask;
+	pctrl->irqchip.irq_set_type = intel_gpio_irq_type;
+	pctrl->irqchip.irq_set_wake = intel_gpio_irq_wake;
+	pctrl->irqchip.flags = IRQCHIP_MASK_ON_SUSPEND;
+
 	ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl);
 	if (ret) {
 		dev_err(pctrl->dev, "failed to register gpiochip\n");
@@ -1233,15 +1235,14 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 		return ret;
 	}
 
-	ret = gpiochip_irqchip_add(&pctrl->chip, &intel_gpio_irqchip, 0,
+	ret = gpiochip_irqchip_add(&pctrl->chip, &pctrl->irqchip, 0,
 				   handle_bad_irq, IRQ_TYPE_NONE);
 	if (ret) {
 		dev_err(pctrl->dev, "failed to add irqchip\n");
 		return ret;
 	}
 
-	gpiochip_set_chained_irqchip(&pctrl->chip, &intel_gpio_irqchip, irq,
-				     NULL);
+	gpiochip_set_chained_irqchip(&pctrl->chip, &pctrl->irqchip, irq, NULL);
 	return 0;
 }
 
-- 
2.23.0

Re: [PATCH v1] pinctrl: intel: Allocate IRQ chip dynamic

[Mika Westerberg]

On Mon, Sep 16, 2019 at 05:38:47PM +0200, Federico Ricchiuto wrote:
>    That's good!
> 
>    There's a place where I can watch fro updates on the topic?

We typically keep Intel pinctrl/GPIO patches here:

  https://git.kernel.org/pub/scm/linux/kernel/git/pinctrl/intel.git/

>    I'm going to recompile the kernel soon, if the update is ready I'd like
>    to add it.

Great. Let us know if it solves the issue you reported so I can apply
the patch to that tree.

Re: [PATCH v1] pinctrl: intel: Allocate IRQ chip dynamic

[Mika Westerberg]

On Mon, Sep 16, 2019 at 05:56:55PM +0200, Federico Ricchiuto wrote:
>    Okay.. I've just checked the repository, even on "fixes" branch but I
>    see that the last change was 7 days ago, am I looking in the wrong
>    place?

I mean I will apply the patch there once you have verified that it fixes
the issue :)

If you want to try the patch out, one way is to save it to a file using
your mail client and then run "git am" against the resulting file. Then
rebuild, intall, reboot and see if the warning still persists.

Re: [PATCH v1] pinctrl: intel: Allocate IRQ chip dynamic

[Mika Westerberg]

On Mon, Sep 16, 2019 at 05:47:51PM +0300, Andy Shevchenko wrote:
> Keeping the IRQ chip definition static shares it with multiple instances of
> the GPIO chip in the system. This is bad and now we get this warning from
> GPIO library:
> 
> "detected irqchip that is shared with multiple gpiochips: please fix the driver."
> 
> Hence, move the IRQ chip definition from being driver static into the struct
> intel_pinctrl. So a unique IRQ chip is used for each GPIO chip instance.
> 
> Fixes: ee1a6ca43dba ("Add Intel Broxton pin controller support")
> Depends-on: 5ff56b015e85 ("Disable GPIO pin interrupts in suspend")
> Reported-by: Federico Ricchiuto <fed.ricchiuto@gmail.com>
> Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to intel.git/fixes, thanks!