Re: [PATCH] irqchip/sunxi-nmi: add missing flag

[Andre Przywara <andre.przywara@arm.com>]

On Thu,  9 Jan 2025 16:39:35 +0100
Philippe Simons <simons.philippe@gmail.com> wrote:

Hi Philippe,

thanks for sending that patch (and not just keeping it in your repo)!

> AXP717 has a PEK to wake up the device from sleep
> AXP717 is on the NMI irq pin
> add IRQCHIP_SKIP_SET_WAKE flag is required to enable wakeup from the suspend

That commit message might leave some people a bit puzzled, what about:
(with my embarrassingly little understanding of irqchip):
=================
Some boards with Allwinner SoCs connect the PMIC's IRQ pin to the SoC's
NMI pin instead of a normal GPIO. Since the power key is connected
to the PMIC, and people expect to wake up a suspended system via
this key, the NMI IRQ controller must stay alive when the
system goes into suspend.

Add the flag to prevent the sunxi NMI controller from going to sleep,
so that the power key can wake up those systems.
=================

Needs also your Signed-off-by:!

> ---
>  drivers/irqchip/irq-sunxi-nmi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-sunxi-nmi.c b/drivers/irqchip/irq-sunxi-nmi.c
> index bb92fd85e..3e87ca724 100644
> --- a/drivers/irqchip/irq-sunxi-nmi.c
> +++ b/drivers/irqchip/irq-sunxi-nmi.c
> @@ -186,7 +186,8 @@ static int __init sunxi_sc_nmi_irq_init(struct device_node *node,
>  	gc->chip_types[0].chip.irq_unmask	= irq_gc_mask_set_bit;
>  	gc->chip_types[0].chip.irq_eoi		= irq_gc_ack_set_bit;
>  	gc->chip_types[0].chip.irq_set_type	= sunxi_sc_nmi_set_type;
> -	gc->chip_types[0].chip.flags		= IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED;
> +	gc->chip_types[0].chip.flags		= IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED
> +		| IRQCHIP_SKIP_SET_WAKE;

I think it's customary to put the operators last on the line, to make
it more obvious that this statement continues on the next line. I'd
recommend to put each flag on a line of its own here, and align them
vertically:
	flags	= IRQCHIP_... |
		  IRQCHIP_... |
		  IRQCHIP_...;

Cheers,
Andre

>  	gc->chip_types[0].regs.ack		= reg_offs->pend;
>  	gc->chip_types[0].regs.mask		= reg_offs->enable;
>  	gc->chip_types[0].regs.type		= reg_offs->ctrl;