diff mbox series

pinctrl: intel: Mask interrupts on driver probe

Message ID 20170921192003.17324-1-kyle.roeschley@ni.com
State New
Headers show
Series pinctrl: intel: Mask interrupts on driver probe | expand

Commit Message

Kyle Roeschley Sept. 21, 2017, 7:20 p.m. UTC
Powering off the system on Apollo Lake does not clear the interrupt
enable registers for the GPIOs. To avoid an interrupt storm on driver
probe, clear all interrupt enables before enabling our interrupt line.

Signed-off-by: Kyle Roeschley <kyle.roeschley@ni.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 43 +++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 20 deletions(-)

Comments

Mika Westerberg Sept. 22, 2017, 5:47 a.m. UTC | #1
On Thu, Sep 21, 2017 at 02:20:03PM -0500, Kyle Roeschley wrote:
> Powering off the system on Apollo Lake does not clear the interrupt
> enable registers for the GPIOs. To avoid an interrupt storm on driver
> probe, clear all interrupt enables before enabling our interrupt line.

It is up to the BIOS to set the proper mask and program the pads
accordingly. Which platform and BIOS this is?

I would rather not do this because it might cause other problems.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Sept. 22, 2017, 1:39 p.m. UTC | #2
On Fri, Sep 22, 2017 at 7:47 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Thu, Sep 21, 2017 at 02:20:03PM -0500, Kyle Roeschley wrote:
>> Powering off the system on Apollo Lake does not clear the interrupt
>> enable registers for the GPIOs. To avoid an interrupt storm on driver
>> probe, clear all interrupt enables before enabling our interrupt line.
>
> It is up to the BIOS to set the proper mask and program the pads
> accordingly. Which platform and BIOS this is?
>
> I would rather not do this because it might cause other problems.

If this needs to happen llike this on some systems or even
just on some BIOS revisions the target systems and/or BIOS
revisions certainly need to be detected first.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kyle Roeschley Sept. 22, 2017, 2:01 p.m. UTC | #3
On Fri, Sep 22, 2017 at 08:47:12AM +0300, Mika Westerberg wrote:
> On Thu, Sep 21, 2017 at 02:20:03PM -0500, Kyle Roeschley wrote:
> > Powering off the system on Apollo Lake does not clear the interrupt
> > enable registers for the GPIOs. To avoid an interrupt storm on driver
> > probe, clear all interrupt enables before enabling our interrupt line.
> 
> It is up to the BIOS to set the proper mask and program the pads
> accordingly. Which platform and BIOS this is?
> 
> I would rather not do this because it might cause other problems.

I haven't seen any issues in testing. This is on a platform based on the Oxbow
Hill CRB, but it's entirely possible that there's a bug in the BIOS power
sequencing behavior. I'll dig around and see if something's been missed.
diff mbox series

Patch

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 71df0f70b61f..f08006417aed 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1043,6 +1043,26 @@  static struct irq_chip intel_gpio_irqchip = {
 	.flags = IRQCHIP_MASK_ON_SUSPEND,
 };
 
+static void intel_gpio_irq_init(struct intel_pinctrl *pctrl)
+{
+	size_t i;
+
+	for (i = 0; i < pctrl->ncommunities; i++) {
+		const struct intel_community *community;
+		void __iomem *base;
+		unsigned gpp;
+
+		community = &pctrl->communities[i];
+		base = community->regs;
+
+		for (gpp = 0; gpp < community->ngpps; gpp++) {
+			/* Mask and clear all interrupts */
+			writel(0, base + community->ie_offset + gpp * 4);
+			writel(0xffff, base + GPI_IS + gpp * 4);
+		}
+	}
+}
+
 static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 {
 	int ret;
@@ -1068,6 +1088,9 @@  static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 		return ret;
 	}
 
+	/* Mask all interrupts */
+	intel_gpio_irq_init(pctrl);
+
 	/*
 	 * We need to request the interrupt here (instead of providing chip
 	 * to the irq directly) because on some platforms several GPIO
@@ -1341,26 +1364,6 @@  int intel_pinctrl_suspend(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(intel_pinctrl_suspend);
 
-static void intel_gpio_irq_init(struct intel_pinctrl *pctrl)
-{
-	size_t i;
-
-	for (i = 0; i < pctrl->ncommunities; i++) {
-		const struct intel_community *community;
-		void __iomem *base;
-		unsigned gpp;
-
-		community = &pctrl->communities[i];
-		base = community->regs;
-
-		for (gpp = 0; gpp < community->ngpps; gpp++) {
-			/* Mask and clear all interrupts */
-			writel(0, base + community->ie_offset + gpp * 4);
-			writel(0xffff, base + GPI_IS + gpp * 4);
-		}
-	}
-}
-
 int intel_pinctrl_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);