[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] ns1650: refactor interrupt handling in ns16550_uart_dt_init()
On Thu, 2023-07-13 at 11:13 +0100, Julien Grall wrote: > Hi Jan, > > On 13/07/2023 11:08, Jan Beulich wrote: > > On 13.07.2023 11:30, Oleksii Kurochko wrote: > > > --- a/xen/drivers/char/ns16550.c > > > +++ b/xen/drivers/char/ns16550.c > > > @@ -1791,8 +1791,16 @@ static int __init > > > ns16550_uart_dt_init(struct dt_device_node *dev, > > > } > > > > > > res = platform_get_irq(dev, 0); > > > - if ( ! res ) > > > - return -EINVAL; > > > + if ( res == -1 ) > > > + { > > > + printk("ns1650: polling will be used\n"); > > > > Nit: Please don't omit one of the two 5-s here. > > > > > + /* > > > + * There is the check 'if ( uart->irq > 0 )' in > > > ns16550_init_postirq(). > > > + * If the check is true then interrupt mode will be used > > > otherwise > > > + * ( when irq = 0 )polling. > > > + */ > > > > I wonder in how far that's actually correct outside of x86. On x86 > > IRQ0 is > > always the timer interrupt, but I'm not convinced something similar > > can be > > used as kind of a heuristic on Arm, RISC-V, or basically any other > > architecture. > > I wondered the same. On Arm we are fine because the UART will be an > SPI > which starts at 32. > > That's part why I was suggesting to use a define. Because we don't > have > to hardcode the poll value everywhere. Probably then it would be better to introduce 'bool is_polling_mode' inside struct ns16550? The same thing ( with uart->irq = 0 ) is used for detecting if polling mode should be used in case of x86 and PCI: https://gitlab.com/xen-project/xen/-/blame/staging/xen/drivers/char/ns16550.c?page=2#L1332 ~ Oleksii
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |