Re: [PATCH] ns1650: refactor interrupt handling in ns16550_uart_dt_init()

[Julien Grall <julien@xxxxxxx>]

Hi,

On 14/07/2023 07:56, Jan Beulich wrote:
> On 13.07.2023 19:49, Oleksii wrote:
> > On Thu, 2023-07-13 at 16:26 +0200, Jan Beulich wrote:
> > > On 13.07.2023 15:36, Oleksii wrote:
> > > > On Thu, 2023-07-13 at 15:27 +0200, Jan Beulich wrote:
> > > > > I don't understand. My earlier comment was affecting all checks
> > > > > of
> > > > > uart->irq alike, as I'm unconvinced IRQ0 may not possibly be
> > > > > usable
> > > > > on some architecture / platform. IOW I don't see why the check in
> > > > > ns16550_init_postirq() would allow us any leeway.
> > > > It looks like I misunderstood you.
> > > >
> > > > Do you mean that on some architecture IRQ0 may be used for ns16550?
> > > Yes, I don't see why this shouldn't be possible in principle. As
> > > Julien
> > > said it can't happen on Arm, so if it also can't happen on RISC-V and
> > > PPC, we could elect to continue to ignore that aspect.
> > Then for RISC-V ( at least, for PLIC interrupt controller ) it is
> > reserved:
> > https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#interrupt-identifiers-ids
> >
> > What about to have 'define NO_IRQ_POLL 0' ( mentioned by Julien )+
> > assert(irq_from_device_tree != NO_IRQ_POLL) ?
> Such a #define may be okay as long as indeed used consistently in all
> places where it belongs (which may mean making some code less simple,
> which is a downside), but I can't judge at all the validity of the
> assertion you propose.
Is the assert() indented to check the return of platform_get_irq()? If 
so, the return value is considered as user input because it is coming 
from the device-tree. assert()s must not be used for checking external 
input. Instead, you need to add proper check (i.e. if (...)).
Cheers,

--
Julien Grall