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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.07.2023 15:19, Oleksii wrote:
> On Thu, 2023-07-13 at 12:08 +0200, 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.
> uart->irq is used as an interrupt number for ns16550 and according to
> the code in ns16550_init_postirq() uart->irq should be > 0.
> So there is safe to use 0 as a detector of polling as it won't be used
> as an interrupt number for ns16550 itself.

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.

Jan