Messages in this thread | | | Date | Thu, 20 Sep 2018 13:47:14 +0800 | From | Guo Ren <> | Subject | Re: [PATCH V5 1/3] irqchip: add C-SKY irqchip drivers |
| |
Hi Marc,
On Tue, Sep 18, 2018 at 04:41:22PM +0100, Marc Zyngier wrote: > > +#define IPI_IRQ 15 > > + > > It feels really bizarre that the function that maps the interrupt is > specific to the interrupt controller, and yet the interrupt number is > defined at the architecture level. I'd expect this to be just as > interrupt controller specific. > Ok, move IPI_IRQ to irq-csky-mpintc.c. See my PATCH V8
> > + irq = arch_ipi_irq_mapping(); > > How about checking the validity of the interrupt and that > arch_ipi_irq_mapping is actually non-NULL? Ok.
> > - rc = request_percpu_irq(IPI_IRQ, handle_ipi, "IPI Interrupt", &ipi_dummy_dev); > > + rc = request_percpu_irq(irq, handle_ipi, "IPI Interrupt", &ipi_dummy_dev); > > if (rc) > > panic("%s IRQ request failed\n", __func__); > > To be honest, I'd tend to question the need for this level of > abstraction, unless you actually plan for multiple SMP-capable > interrupt controllers... But at the end of the day, that's your call, > and the above code looks mostly correct. Thx for the review. I will consider your suggestion.
Best Regards GUo Ren
| |