diff mbox series

[v1] pinctrl: intel: Allocate IRQ chip dynamic

Message ID 20190916144751.21525-1-andriy.shevchenko@linux.intel.com
State New
Headers show
Series [v1] pinctrl: intel: Allocate IRQ chip dynamic | expand

Commit Message

Andy Shevchenko Sept. 16, 2019, 2:47 p.m. UTC 
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(-)

Comments

Mika Westerberg Sept. 16, 2019, 3:47 p.m. UTC | #1 
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.
Mika Westerberg Sept. 16, 2019, 4:05 p.m. UTC | #2 
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.
Mika Westerberg Oct. 1, 2019, 8:31 a.m. UTC | #3 
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!
diff mbox series

Patch

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;
 }