Re: [PATCH] pinctrl: intel: Mask interrupts on driver probe

[Kyle Roeschley]

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.

-- 
Kyle Roeschley
Software Engineer
National Instruments

Re: [PATCH] pinctrl: intel: Mask interrupts on driver probe

[Kyle Roeschley]

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.

-- 
Kyle Roeschley
Software Engineer
National Instruments

Re: [PATCH] pinctrl: intel: Mask interrupts on driver probe

[Linus Walleij]

On Fri, Sep 22, 2017 at 7:47 AM, 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.

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

Re: [PATCH] pinctrl: intel: Mask interrupts on driver probe

[Linus Walleij]

On Fri, Sep 22, 2017 at 7:47 AM, 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.

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

Re: [PATCH] pinctrl: intel: Mask interrupts on driver probe

[Mika Westerberg]

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.

Re: [PATCH] pinctrl: intel: Mask interrupts on driver probe

[Mika Westerberg]

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.

[PATCH] pinctrl: intel: Mask interrupts on driver probe

[Kyle Roeschley]

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 
---
 drivers/pinctrl/intel/pinctrl-intel.c | 43 +++
 1 file changed, 23 insertions(+), 20 deletions(-)

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 = >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(0x, 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 = >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(0x, base + GPI_IS + gpp * 4);
-   }
-   }
-}
-
 int intel_pinctrl_resume(struct device *dev)
 {
struct platform_device *pdev = to_platform_device(dev);
-- 
2.14.1

[PATCH] pinctrl: intel: Mask interrupts on driver probe

[Kyle Roeschley]

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 
---
 drivers/pinctrl/intel/pinctrl-intel.c | 43 +++
 1 file changed, 23 insertions(+), 20 deletions(-)

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 = >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(0x, 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 = >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(0x, base + GPI_IS + gpp * 4);
-   }
-   }
-}
-
 int intel_pinctrl_resume(struct device *dev)
 {
struct platform_device *pdev = to_platform_device(dev);
-- 
2.14.1