Re: [kvm-unit-tests PATCH v4 03/13] arm/arm64: gic: Introduce setup_irq() helper

[Andrew Jones]

On Tue, Mar 10, 2020 at 12:00:19PM +0100, Auger Eric wrote:
> Hi Drew,
> On 3/9/20 11:56 AM, Andrew Jones wrote:
> > On Mon, Mar 09, 2020 at 11:24:10AM +0100, Eric Auger wrote:
> >> ipi_enable() code would be reusable for other interrupts
> >> than IPI. Let's rename it setup_irq() and pass an interrupt
> >> handler pointer.
> >>
> >> Signed-off-by: Eric Auger <address@hidden>
> >>
> >> ---
> >>
> >> v2 -> v3:
> >> - do not export setup_irq anymore
> >> ---
> >>  arm/gic.c | 20 +++++++-------------
> >>  1 file changed, 7 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/arm/gic.c b/arm/gic.c
> >> index fcf4c1f..abf08c7 100644
> >> --- a/arm/gic.c
> >> +++ b/arm/gic.c
> >> @@ -34,6 +34,7 @@ static struct gic *gic;
> >>  static int acked[NR_CPUS], spurious[NR_CPUS];
> >>  static int bad_sender[NR_CPUS], bad_irq[NR_CPUS];
> >>  static cpumask_t ready;
> >> +typedef void (*handler_t)(struct pt_regs *regs __unused);
> > 
> > This is just irq_handler_fn, which is already defined in processor.h.
> > We don't need the __unused, not since 6b07148d06b1 ("Replace -Wextra
> > with a saner list of warning flags").
> Shall I duplicate it into ./lib/arm/asm/processor.h as well?

Yes, please do

Thanks,
drew
> 
> Thanks
> 
> Eric
> > 
> >>  
> >>  static void nr_cpu_check(int nr)
> >>  {
> >> @@ -215,20 +216,20 @@ static void ipi_test_smp(void)
> >>    report_prefix_pop();
> >>  }
> >>  
> >> -static void ipi_enable(void)
> >> +static void setup_irq(handler_t handler)
> >>  {
> >>    gic_enable_defaults();
> >>  #ifdef __arm__
> >> -  install_exception_handler(EXCPTN_IRQ, ipi_handler);
> >> +  install_exception_handler(EXCPTN_IRQ, handler);
> >>  #else
> >> -  install_irq_handler(EL1H_IRQ, ipi_handler);
> >> +  install_irq_handler(EL1H_IRQ, handler);
> >>  #endif
> >>    local_irq_enable();
> >>  }
> >>  
> >>  static void ipi_send(void)
> >>  {
> >> -  ipi_enable();
> >> +  setup_irq(ipi_handler);
> >>    wait_on_ready();
> >>    ipi_test_self();
> >>    ipi_test_smp();
> >> @@ -238,7 +239,7 @@ static void ipi_send(void)
> >>  
> >>  static void ipi_recv(void)
> >>  {
> >> -  ipi_enable();
> >> +  setup_irq(ipi_handler);
> >>    cpumask_set_cpu(smp_processor_id(), &ready);
> >>    while (1)
> >>            wfi();
> >> @@ -295,14 +296,7 @@ static void ipi_clear_active_handler(struct pt_regs 
> >> *regs __unused)
> >>  static void run_active_clear_test(void)
> >>  {
> >>    report_prefix_push("active");
> >> -  gic_enable_defaults();
> >> -#ifdef __arm__
> >> -  install_exception_handler(EXCPTN_IRQ, ipi_clear_active_handler);
> >> -#else
> >> -  install_irq_handler(EL1H_IRQ, ipi_clear_active_handler);
> >> -#endif
> >> -  local_irq_enable();
> >> -
> >> +  setup_irq(ipi_clear_active_handler);
> >>    ipi_test_self();
> >>    report_prefix_pop();
> >>  }
> >> -- 
> >> 2.20.1
> >>
> 
>