Re: [PATCH v3] ns16550: add support for polling mode when device tree is used

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.07.2023 10:15, Oleksii wrote:
> On Mon, 2023-07-24 at 16:43 +0200, Jan Beulich wrote:
>> On 24.07.2023 16:27, Oleksii Kurochko wrote:
>>> @@ -1330,9 +1341,12 @@ pci_uart_config(struct ns16550 *uart, bool_t
>>> skip_amt, unsigned int idx)
>>>                   * as special only for X86.
>>>                   */
>>>                  if ( uart->irq == 0xff )
>>> +                {
>>>                      uart->irq = 0;
>>> +                    uart->intr_works = polling;
>>> +                }
>>>  #endif
>>> -                if ( !uart->irq )
>>> +                if ( !uart->irq || (uart->intr_works == polling) )
>>>                      printk(XENLOG_INFO
>>>                             "ns16550: %pp: no legacy IRQ, using
>>> poll mode\n",
>>>                             &PCI_SBDF(0, b, d, f));
>>
>> Message and code (after your addition) continue to be out of sync.
>> As said before, any condition that leads to polling mode wants to
>> find itself expressed by uart->intr_works set to "polling".
> Well. It looks like it would be better to set 'uart->intr_works =
> polling' inside if ( !uart->irq ):
>   if ( !uart->irq || (uart->intr_works == polling) )
>   {
>       uart->intr_works = polling;
>       printk(XENLOG_INFO
>              "ns16550: %pp: using poll mode\n",
>              &PCI_SBDF(0, b, d, f));
>   }
> Then "uart->intr_works = polling;" can be removed from "if ( uart->irq
> == 0xff )" as in that case 'uart->irq = 0' means polling mode for X86.

I'm not fully convinced. Care needs to be taken that both the x86 case
and the general case continue to function as they did (unless proven to
be buggy).

Jan