linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mfd: axp20x: fix order of pek rise and fall events
@ 2022-11-23 23:07 Aren Moynihan
  2022-11-30  6:43 ` Samuel Holland
  0 siblings, 1 reply; 3+ messages in thread
From: Aren Moynihan @ 2022-11-23 23:07 UTC (permalink / raw)
  To: Lee Jones, Chen-Yu Tsai, linux-kernel; +Cc: Ondrej Jirman, Aren Moynihan

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 <aren@peacevolution.org>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] mfd: axp20x: fix order of pek rise and fall events
  2022-11-23 23:07 [PATCH] mfd: axp20x: fix order of pek rise and fall events Aren Moynihan
@ 2022-11-30  6:43 ` Samuel Holland
  2022-12-08 23:12   ` Aren
  0 siblings, 1 reply; 3+ messages in thread
From: Samuel Holland @ 2022-11-30  6:43 UTC (permalink / raw)
  To: Aren Moynihan, Lee Jones, Chen-Yu Tsai; +Cc: Ondrej Jirman, linux-kernel

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 <aren@peacevolution.org>
> ---
>  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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mfd: axp20x: fix order of pek rise and fall events
  2022-11-30  6:43 ` Samuel Holland
@ 2022-12-08 23:12   ` Aren
  0 siblings, 0 replies; 3+ messages in thread
From: Aren @ 2022-12-08 23:12 UTC (permalink / raw)
  To: Samuel Holland; +Cc: Lee Jones, Chen-Yu Tsai, Ondrej Jirman, linux-kernel

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/20221208220225.635414-1-aren@peacevolution.org/T/

> > Signed-off-by: Aren Moynihan <aren@peacevolution.org>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-12-08 23:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-23 23:07 [PATCH] mfd: axp20x: fix order of pek rise and fall events Aren Moynihan
2022-11-30  6:43 ` Samuel Holland
2022-12-08 23:12   ` Aren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).