The fix prevents unintended wakes from second level GPIO pin interrupts.
On some Intel Kabylake platforms, it is observed that GPIO pin interrupts
can wake the platform from suspend-to-idle, even though the IRQ is not
configured as IRQF_NO_SUSPEND or enable_irq_wake().
This can cause undesired wakes on Mobile devices such as Laptops and
Chromebook devices. For example a headset jack insertion is not a desired
wake source on Chromebook devices.
The pinctrl-intel (GPIO controller) driver implements a "Shared IRQ" model.
All GPIO pin interrupts are OR'ed and mapped to a first level IRQ14 (or
IRQ15). The driver registers an irq_chip struct and maps an irq_domain for
the GPIO pin interrupts. The IRQ14 handler demuxes and calls the second
level IRQ for the respective pin.
In the suspend entry flow, at suspend_noirq stage, the kernel disables IRQs
that are not marked for wake. The pinctrl-intel driver does not implement a
irq_disable() callback (to take advantage of lazy disabling). The
pinctrl-intel GPIO interrupts are not disabled in hardware during suspend
entry, and thus are able to wake the SoC out of suspend-to-idle.
This patch sets the IRQCHIP_MASK_ON_SUSPEND flag for the GPIO irq_chip, to
disable the second level interrupts at suspend_noirq stage via the irq_mask
callbacks. The irq_mask callback disables the IRQs in hardware by
programming the corresponding GPIO pad registers. Only IRQs that are not
marked for wake are disabled.
Signed-off-by: Rushikesh S Kadam <[email protected]>
---
drivers/pinctrl/intel/pinctrl-intel.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 6dc1096..8f87215 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1035,6 +1035,7 @@ static irqreturn_t intel_gpio_irq(int irq, void *data)
.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_probe(struct intel_pinctrl *pctrl, int irq)
--
1.9.1
On Fri, Aug 11, 2017 at 01:53:44PM +0530, Rushikesh S Kadam wrote:
> The fix prevents unintended wakes from second level GPIO pin interrupts.
>
> On some Intel Kabylake platforms, it is observed that GPIO pin interrupts
> can wake the platform from suspend-to-idle, even though the IRQ is not
> configured as IRQF_NO_SUSPEND or enable_irq_wake().
>
> This can cause undesired wakes on Mobile devices such as Laptops and
> Chromebook devices. For example a headset jack insertion is not a desired
> wake source on Chromebook devices.
>
> The pinctrl-intel (GPIO controller) driver implements a "Shared IRQ" model.
> All GPIO pin interrupts are OR'ed and mapped to a first level IRQ14 (or
> IRQ15). The driver registers an irq_chip struct and maps an irq_domain for
> the GPIO pin interrupts. The IRQ14 handler demuxes and calls the second
> level IRQ for the respective pin.
>
> In the suspend entry flow, at suspend_noirq stage, the kernel disables IRQs
> that are not marked for wake. The pinctrl-intel driver does not implement a
> irq_disable() callback (to take advantage of lazy disabling). The
> pinctrl-intel GPIO interrupts are not disabled in hardware during suspend
> entry, and thus are able to wake the SoC out of suspend-to-idle.
>
> This patch sets the IRQCHIP_MASK_ON_SUSPEND flag for the GPIO irq_chip, to
> disable the second level interrupts at suspend_noirq stage via the irq_mask
> callbacks. The irq_mask callback disables the IRQs in hardware by
> programming the corresponding GPIO pad registers. Only IRQs that are not
> marked for wake are disabled.
This is really good changelog!
> Signed-off-by: Rushikesh S Kadam <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
On Fri, Aug 11, 2017 at 11:23 AM, Rushikesh S Kadam
<[email protected]> wrote:
> The fix prevents unintended wakes from second level GPIO pin interrupts.
>
> On some Intel Kabylake platforms, it is observed that GPIO pin interrupts
> can wake the platform from suspend-to-idle, even though the IRQ is not
> configured as IRQF_NO_SUSPEND or enable_irq_wake().
>
> This can cause undesired wakes on Mobile devices such as Laptops and
> Chromebook devices. For example a headset jack insertion is not a desired
> wake source on Chromebook devices.
>
> The pinctrl-intel (GPIO controller) driver implements a "Shared IRQ" model.
> All GPIO pin interrupts are OR'ed and mapped to a first level IRQ14 (or
> IRQ15). The driver registers an irq_chip struct and maps an irq_domain for
> the GPIO pin interrupts. The IRQ14 handler demuxes and calls the second
> level IRQ for the respective pin.
>
> In the suspend entry flow, at suspend_noirq stage, the kernel disables IRQs
> that are not marked for wake. The pinctrl-intel driver does not implement a
> irq_disable() callback (to take advantage of lazy disabling). The
> pinctrl-intel GPIO interrupts are not disabled in hardware during suspend
> entry, and thus are able to wake the SoC out of suspend-to-idle.
>
> This patch sets the IRQCHIP_MASK_ON_SUSPEND flag for the GPIO irq_chip, to
> disable the second level interrupts at suspend_noirq stage via the irq_mask
> callbacks. The irq_mask callback disables the IRQs in hardware by
> programming the corresponding GPIO pad registers. Only IRQs that are not
> marked for wake are disabled.
>
> Signed-off-by: Rushikesh S Kadam <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> drivers/pinctrl/intel/pinctrl-intel.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index 6dc1096..8f87215 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -1035,6 +1035,7 @@ static irqreturn_t intel_gpio_irq(int irq, void *data)
> .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_probe(struct intel_pinctrl *pctrl, int irq)
> --
> 1.9.1
>
--
With Best Regards,
Andy Shevchenko
On Fri, Aug 11, 2017 at 12:57:26PM +0300, Andy Shevchenko wrote:
> On Fri, Aug 11, 2017 at 11:23 AM, Rushikesh S Kadam
> <[email protected]> wrote:
> > The fix prevents unintended wakes from second level GPIO pin interrupts.
> >
> > On some Intel Kabylake platforms, it is observed that GPIO pin interrupts
> > can wake the platform from suspend-to-idle, even though the IRQ is not
> > configured as IRQF_NO_SUSPEND or enable_irq_wake().
> >
> > This can cause undesired wakes on Mobile devices such as Laptops and
> > Chromebook devices. For example a headset jack insertion is not a desired
> > wake source on Chromebook devices.
> >
> > The pinctrl-intel (GPIO controller) driver implements a "Shared IRQ" model.
> > All GPIO pin interrupts are OR'ed and mapped to a first level IRQ14 (or
> > IRQ15). The driver registers an irq_chip struct and maps an irq_domain for
> > the GPIO pin interrupts. The IRQ14 handler demuxes and calls the second
> > level IRQ for the respective pin.
> >
> > In the suspend entry flow, at suspend_noirq stage, the kernel disables IRQs
> > that are not marked for wake. The pinctrl-intel driver does not implement a
> > irq_disable() callback (to take advantage of lazy disabling). The
> > pinctrl-intel GPIO interrupts are not disabled in hardware during suspend
> > entry, and thus are able to wake the SoC out of suspend-to-idle.
> >
> > This patch sets the IRQCHIP_MASK_ON_SUSPEND flag for the GPIO irq_chip, to
> > disable the second level interrupts at suspend_noirq stage via the irq_mask
> > callbacks. The irq_mask callback disables the IRQs in hardware by
> > programming the corresponding GPIO pad registers. Only IRQs that are not
> > marked for wake are disabled.
> >
> > Signed-off-by: Rushikesh S Kadam <[email protected]>
>
> Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-and-tested-by: Rajneesh Bhardwaj <[email protected]>
>
> > ---
> > drivers/pinctrl/intel/pinctrl-intel.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> > index 6dc1096..8f87215 100644
> > --- a/drivers/pinctrl/intel/pinctrl-intel.c
> > +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> > @@ -1035,6 +1035,7 @@ static irqreturn_t intel_gpio_irq(int irq, void *data)
> > .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_probe(struct intel_pinctrl *pctrl, int irq)
> > --
> > 1.9.1
> >
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
--
On Fri, Aug 11, 2017 at 10:23 AM, Rushikesh S Kadam
<[email protected]> wrote:
> The fix prevents unintended wakes from second level GPIO pin interrupts.
>
> On some Intel Kabylake platforms, it is observed that GPIO pin interrupts
> can wake the platform from suspend-to-idle, even though the IRQ is not
> configured as IRQF_NO_SUSPEND or enable_irq_wake().
>
> This can cause undesired wakes on Mobile devices such as Laptops and
> Chromebook devices. For example a headset jack insertion is not a desired
> wake source on Chromebook devices.
>
> The pinctrl-intel (GPIO controller) driver implements a "Shared IRQ" model.
> All GPIO pin interrupts are OR'ed and mapped to a first level IRQ14 (or
> IRQ15). The driver registers an irq_chip struct and maps an irq_domain for
> the GPIO pin interrupts. The IRQ14 handler demuxes and calls the second
> level IRQ for the respective pin.
>
> In the suspend entry flow, at suspend_noirq stage, the kernel disables IRQs
> that are not marked for wake. The pinctrl-intel driver does not implement a
> irq_disable() callback (to take advantage of lazy disabling). The
> pinctrl-intel GPIO interrupts are not disabled in hardware during suspend
> entry, and thus are able to wake the SoC out of suspend-to-idle.
>
> This patch sets the IRQCHIP_MASK_ON_SUSPEND flag for the GPIO irq_chip, to
> disable the second level interrupts at suspend_noirq stage via the irq_mask
> callbacks. The irq_mask callback disables the IRQs in hardware by
> programming the corresponding GPIO pad registers. Only IRQs that are not
> marked for wake are disabled.
>
> Signed-off-by: Rushikesh S Kadam <[email protected]>
Excellent work, patch applied with the collected ACKs.
Yours,
Linus Walleij