Re: [PATCH 2/2] irqchip: nvic: Use GENERIC_IRQ_MULTI_HANDLER

From: Mark Rutland <mark.rutland@arm.com>
To: Vladimir Murzin <vladimir.murzin@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, linux@armlinux.org.uk,
	maz@kernel.org, Ard Biesheuvel <ardb@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH 2/2] irqchip: nvic: Use GENERIC_IRQ_MULTI_HANDLER
Date: Wed, 1 Dec 2021 11:58:51 +0000	[thread overview]
Message-ID: <Yadje3TcsDRMqMVS@FVFF77S0Q05N> (raw)
In-Reply-To: <20211201110259.84857-2-vladimir.murzin@arm.com>

Adding Ard and Arnd, since this looks like it might interact with their series
moving other platforms to GENERIC_IRQ_MULTI_HANDLER:

  https://lore.kernel.org/linux-arm-kernel/20211130125901.3054-1-ardb@kernel.org/

On Wed, Dec 01, 2021 at 11:02:59AM +0000, Vladimir Murzin wrote:
> Rather then restructuring the ARMv7M entrly logic per TODO, just move
> NVIC to GENERIC_IRQ_MULTI_HANDLER.
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm/include/asm/v7m.h  |  3 ++-
>  arch/arm/kernel/entry-v7m.S | 10 +++-------
>  drivers/irqchip/Kconfig     |  1 +
>  drivers/irqchip/irq-nvic.c  | 22 +++++-----------------
>  4 files changed, 11 insertions(+), 25 deletions(-)

Nice; thanks for doing this!

FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> 
> diff --git a/arch/arm/include/asm/v7m.h b/arch/arm/include/asm/v7m.h
> index 2cb00d1..4512f7e 100644
> --- a/arch/arm/include/asm/v7m.h
> +++ b/arch/arm/include/asm/v7m.h
> @@ -13,6 +13,7 @@
>  #define V7M_SCB_ICSR_PENDSVSET			(1 << 28)
>  #define V7M_SCB_ICSR_PENDSVCLR			(1 << 27)
>  #define V7M_SCB_ICSR_RETTOBASE			(1 << 11)
> +#define V7M_SCB_ICSR_VECTACTIVE			0x000001ff
>  
>  #define V7M_SCB_VTOR			0x08
>  
> @@ -38,7 +39,7 @@
>  #define V7M_SCB_SHCSR_MEMFAULTENA		(1 << 16)
>  
>  #define V7M_xPSR_FRAMEPTRALIGN			0x00000200
> -#define V7M_xPSR_EXCEPTIONNO			0x000001ff
> +#define V7M_xPSR_EXCEPTIONNO			V7M_SCB_ICSR_VECTACTIVE
>  
>  /*
>   * When branching to an address that has bits [31:28] == 0xf an exception return
> diff --git a/arch/arm/kernel/entry-v7m.S b/arch/arm/kernel/entry-v7m.S
> index 7bde93c..520dd43 100644
> --- a/arch/arm/kernel/entry-v7m.S
> +++ b/arch/arm/kernel/entry-v7m.S
> @@ -39,14 +39,10 @@ __irq_entry:
>  	@
>  	@ Invoke the IRQ handler
>  	@
> -	mrs	r0, ipsr
> -	ldr	r1, =V7M_xPSR_EXCEPTIONNO
> -	and	r0, r1
> -	sub	r0, #16
> -	mov	r1, sp
> +	mov	r0, sp
>  	stmdb	sp!, {lr}
> -	@ routine called with r0 = irq number, r1 = struct pt_regs *
> -	bl	nvic_handle_irq
> +	@ routine called with r0 = struct pt_regs *
> +	bl	generic_handle_arch_irq
>  
>  	pop	{lr}
>  	@
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 7038957..488eaa1 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -58,6 +58,7 @@ config ARM_NVIC
>  	bool
>  	select IRQ_DOMAIN_HIERARCHY
>  	select GENERIC_IRQ_CHIP
> +	select GENERIC_IRQ_MULTI_HANDLER
>  
>  config ARM_VIC
>  	bool
> diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
> index ba4759b..125f9c1 100644
> --- a/drivers/irqchip/irq-nvic.c
> +++ b/drivers/irqchip/irq-nvic.c
> @@ -37,25 +37,12 @@
>  
>  static struct irq_domain *nvic_irq_domain;
>  
> -static void __nvic_handle_irq(irq_hw_number_t hwirq)
> +static void __irq_entry nvic_handle_irq(struct pt_regs *regs)
>  {
> -	generic_handle_domain_irq(nvic_irq_domain, hwirq);
> -}
> +	unsigned long icsr = readl_relaxed(BASEADDR_V7M_SCB + V7M_SCB_ICSR);
> +	irq_hw_number_t hwirq = (icsr & V7M_SCB_ICSR_VECTACTIVE) - 16;
>  
> -/*
> - * TODO: restructure the ARMv7M entry logic so that this entry logic can live
> - * in arch code.
> - */
> -asmlinkage void __exception_irq_entry
> -nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> -{
> -	struct pt_regs *old_regs;
> -
> -	irq_enter();
> -	old_regs = set_irq_regs(regs);
> -	__nvic_handle_irq(hwirq);
> -	set_irq_regs(old_regs);
> -	irq_exit();
> +	generic_handle_domain_irq(nvic_irq_domain, hwirq);
>  }
>  
>  static int nvic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> @@ -141,6 +128,7 @@ static int __init nvic_of_init(struct device_node *node,
>  	for (i = 0; i < irqs; i += 4)
>  		writel_relaxed(0, nvic_base + NVIC_IPR + i);
>  
> +	set_handle_irq(nvic_handle_irq);
>  	return 0;
>  }
>  IRQCHIP_DECLARE(armv7m_nvic, "arm,armv7m-nvic", nvic_of_init);
> -- 
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel