The power button can get "stuck" if the rising edge and falling edge irq
are read in the same pass. This can often be triggered when resuming
from suspend if the power button is released before the kernel handles
the interrupt.
Swapping the order of the rise and fall events makes sure that the press
event is handled first, which prevents this situation.
Signed-off-by: Aren Moynihan <[email protected]>
---
include/linux/mfd/axp20x.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 9ab0e2fca7ea..a13ba2bdd01f 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -432,8 +432,8 @@ enum {
AXP152_IRQ_PEK_SHORT,
AXP152_IRQ_PEK_LONG,
AXP152_IRQ_TIMER,
- AXP152_IRQ_PEK_RIS_EDGE,
AXP152_IRQ_PEK_FAL_EDGE,
+ AXP152_IRQ_PEK_RIS_EDGE,
AXP152_IRQ_GPIO3_INPUT,
AXP152_IRQ_GPIO2_INPUT,
AXP152_IRQ_GPIO1_INPUT,
@@ -472,8 +472,8 @@ enum {
AXP20X_IRQ_LOW_PWR_LVL1,
AXP20X_IRQ_LOW_PWR_LVL2,
AXP20X_IRQ_TIMER,
- AXP20X_IRQ_PEK_RIS_EDGE,
AXP20X_IRQ_PEK_FAL_EDGE,
+ AXP20X_IRQ_PEK_RIS_EDGE,
AXP20X_IRQ_GPIO3_INPUT,
AXP20X_IRQ_GPIO2_INPUT,
AXP20X_IRQ_GPIO1_INPUT,
@@ -502,8 +502,8 @@ enum axp22x_irqs {
AXP22X_IRQ_LOW_PWR_LVL1,
AXP22X_IRQ_LOW_PWR_LVL2,
AXP22X_IRQ_TIMER,
- AXP22X_IRQ_PEK_RIS_EDGE,
AXP22X_IRQ_PEK_FAL_EDGE,
+ AXP22X_IRQ_PEK_RIS_EDGE,
AXP22X_IRQ_GPIO1_INPUT,
AXP22X_IRQ_GPIO0_INPUT,
};
@@ -571,8 +571,8 @@ enum axp803_irqs {
AXP803_IRQ_LOW_PWR_LVL1,
AXP803_IRQ_LOW_PWR_LVL2,
AXP803_IRQ_TIMER,
- AXP803_IRQ_PEK_RIS_EDGE,
AXP803_IRQ_PEK_FAL_EDGE,
+ AXP803_IRQ_PEK_RIS_EDGE,
AXP803_IRQ_PEK_SHORT,
AXP803_IRQ_PEK_LONG,
AXP803_IRQ_PEK_OVER_OFF,
@@ -623,8 +623,8 @@ enum axp809_irqs {
AXP809_IRQ_LOW_PWR_LVL1,
AXP809_IRQ_LOW_PWR_LVL2,
AXP809_IRQ_TIMER,
- AXP809_IRQ_PEK_RIS_EDGE,
AXP809_IRQ_PEK_FAL_EDGE,
+ AXP809_IRQ_PEK_RIS_EDGE,
AXP809_IRQ_PEK_SHORT,
AXP809_IRQ_PEK_LONG,
AXP809_IRQ_PEK_OVER_OFF,
--
2.38.1
On 11/23/22 17:07, Aren Moynihan wrote:
> The power button can get "stuck" if the rising edge and falling edge irq
> are read in the same pass. This can often be triggered when resuming
> from suspend if the power button is released before the kernel handles
> the interrupt.
>
> Swapping the order of the rise and fall events makes sure that the press
> event is handled first, which prevents this situation.
Indeed. This is probably the simplest solution, but I think it deserves
a comment in at least one place that the order intentionally mismatches
the order of the bits in the register.
> Signed-off-by: Aren Moynihan <[email protected]>
> ---
> include/linux/mfd/axp20x.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 9ab0e2fca7ea..a13ba2bdd01f 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -432,8 +432,8 @@ enum {
> AXP152_IRQ_PEK_SHORT,
> AXP152_IRQ_PEK_LONG,
> AXP152_IRQ_TIMER,
> - AXP152_IRQ_PEK_RIS_EDGE,
> AXP152_IRQ_PEK_FAL_EDGE,
> + AXP152_IRQ_PEK_RIS_EDGE,
> AXP152_IRQ_GPIO3_INPUT,
> AXP152_IRQ_GPIO2_INPUT,
> AXP152_IRQ_GPIO1_INPUT,
> @@ -472,8 +472,8 @@ enum {
> AXP20X_IRQ_LOW_PWR_LVL1,
> AXP20X_IRQ_LOW_PWR_LVL2,
> AXP20X_IRQ_TIMER,
> - AXP20X_IRQ_PEK_RIS_EDGE,
> AXP20X_IRQ_PEK_FAL_EDGE,
> + AXP20X_IRQ_PEK_RIS_EDGE,
> AXP20X_IRQ_GPIO3_INPUT,
> AXP20X_IRQ_GPIO2_INPUT,
> AXP20X_IRQ_GPIO1_INPUT,
> @@ -502,8 +502,8 @@ enum axp22x_irqs {
> AXP22X_IRQ_LOW_PWR_LVL1,
> AXP22X_IRQ_LOW_PWR_LVL2,
> AXP22X_IRQ_TIMER,
> - AXP22X_IRQ_PEK_RIS_EDGE,
> AXP22X_IRQ_PEK_FAL_EDGE,
> + AXP22X_IRQ_PEK_RIS_EDGE,
> AXP22X_IRQ_GPIO1_INPUT,
> AXP22X_IRQ_GPIO0_INPUT,
> };
> @@ -571,8 +571,8 @@ enum axp803_irqs {
> AXP803_IRQ_LOW_PWR_LVL1,
> AXP803_IRQ_LOW_PWR_LVL2,
> AXP803_IRQ_TIMER,
> - AXP803_IRQ_PEK_RIS_EDGE,
> AXP803_IRQ_PEK_FAL_EDGE,
> + AXP803_IRQ_PEK_RIS_EDGE,
> AXP803_IRQ_PEK_SHORT,
> AXP803_IRQ_PEK_LONG,
> AXP803_IRQ_PEK_OVER_OFF,
> @@ -623,8 +623,8 @@ enum axp809_irqs {
> AXP809_IRQ_LOW_PWR_LVL1,
> AXP809_IRQ_LOW_PWR_LVL2,
> AXP809_IRQ_TIMER,
> - AXP809_IRQ_PEK_RIS_EDGE,
> AXP809_IRQ_PEK_FAL_EDGE,
> + AXP809_IRQ_PEK_RIS_EDGE,
> AXP809_IRQ_PEK_SHORT,
> AXP809_IRQ_PEK_LONG,
> AXP809_IRQ_PEK_OVER_OFF,
You should also update APX288 and APX806, which name the IRQs "POK"
instead of "PEK" for some reason.
Regards,
Samuel
Hi, thanks for taking a look at this
On Wed, Nov 30, 2022 at 12:43:15AM -0600, Samuel Holland wrote:
> On 11/23/22 17:07, Aren Moynihan wrote:
> > The power button can get "stuck" if the rising edge and falling edge irq
> > are read in the same pass. This can often be triggered when resuming
> > from suspend if the power button is released before the kernel handles
> > the interrupt.
> >
> > Swapping the order of the rise and fall events makes sure that the press
> > event is handled first, which prevents this situation.
>
> Indeed. This is probably the simplest solution, but I think it deserves
> a comment in at least one place that the order intentionally mismatches
> the order of the bits in the register.
Alright, I've sent a v2 with comments explaining the order
https://lore.kernel.org/lkml/[email protected]/T/
> > Signed-off-by: Aren Moynihan <[email protected]>
> > ---
> > include/linux/mfd/axp20x.h | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> > index 9ab0e2fca7ea..a13ba2bdd01f 100644
> > --- a/include/linux/mfd/axp20x.h
> > +++ b/include/linux/mfd/axp20x.h
> > @@ -432,8 +432,8 @@ enum {
> > AXP152_IRQ_PEK_SHORT,
> > AXP152_IRQ_PEK_LONG,
> > AXP152_IRQ_TIMER,
> > - AXP152_IRQ_PEK_RIS_EDGE,
> > AXP152_IRQ_PEK_FAL_EDGE,
> > + AXP152_IRQ_PEK_RIS_EDGE,
> > AXP152_IRQ_GPIO3_INPUT,
> > AXP152_IRQ_GPIO2_INPUT,
> > AXP152_IRQ_GPIO1_INPUT,
> > @@ -472,8 +472,8 @@ enum {
> > AXP20X_IRQ_LOW_PWR_LVL1,
> > AXP20X_IRQ_LOW_PWR_LVL2,
> > AXP20X_IRQ_TIMER,
> > - AXP20X_IRQ_PEK_RIS_EDGE,
> > AXP20X_IRQ_PEK_FAL_EDGE,
> > + AXP20X_IRQ_PEK_RIS_EDGE,
> > AXP20X_IRQ_GPIO3_INPUT,
> > AXP20X_IRQ_GPIO2_INPUT,
> > AXP20X_IRQ_GPIO1_INPUT,
> > @@ -502,8 +502,8 @@ enum axp22x_irqs {
> > AXP22X_IRQ_LOW_PWR_LVL1,
> > AXP22X_IRQ_LOW_PWR_LVL2,
> > AXP22X_IRQ_TIMER,
> > - AXP22X_IRQ_PEK_RIS_EDGE,
> > AXP22X_IRQ_PEK_FAL_EDGE,
> > + AXP22X_IRQ_PEK_RIS_EDGE,
> > AXP22X_IRQ_GPIO1_INPUT,
> > AXP22X_IRQ_GPIO0_INPUT,
> > };
> > @@ -571,8 +571,8 @@ enum axp803_irqs {
> > AXP803_IRQ_LOW_PWR_LVL1,
> > AXP803_IRQ_LOW_PWR_LVL2,
> > AXP803_IRQ_TIMER,
> > - AXP803_IRQ_PEK_RIS_EDGE,
> > AXP803_IRQ_PEK_FAL_EDGE,
> > + AXP803_IRQ_PEK_RIS_EDGE,
> > AXP803_IRQ_PEK_SHORT,
> > AXP803_IRQ_PEK_LONG,
> > AXP803_IRQ_PEK_OVER_OFF,
> > @@ -623,8 +623,8 @@ enum axp809_irqs {
> > AXP809_IRQ_LOW_PWR_LVL1,
> > AXP809_IRQ_LOW_PWR_LVL2,
> > AXP809_IRQ_TIMER,
> > - AXP809_IRQ_PEK_RIS_EDGE,
> > AXP809_IRQ_PEK_FAL_EDGE,
> > + AXP809_IRQ_PEK_RIS_EDGE,
> > AXP809_IRQ_PEK_SHORT,
> > AXP809_IRQ_PEK_LONG,
> > AXP809_IRQ_PEK_OVER_OFF,
>
> You should also update APX288 and APX806, which name the IRQs "POK"
> instead of "PEK" for some reason.
Thanks for point this out, it looks like these are already in the
correct order.
- Aren