Re: [PATCH v6 1/2] ns16550: use poll mode if INTERRUPT_LINE is 0xff

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, May 20, 2022 at 11:47:02AM +0200, Jan Beulich wrote:
> On 20.05.2022 10:53, Marek Marczykowski-Górecki wrote:
> > Intel LPSS has INTERRUPT_LINE set to 0xff by default, that is declared
> > by the PCI Local Bus Specification Revision 3.0 (from 2004) as
> > "unknown"/"no connection". Fallback to poll mode in this case.
> > The 0xff handling is x86-specific, the surrounding code is guarded with
> > CONFIG_X86 anyway.
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

> > Changes in v6:
> >  - wrap the check in additional CONFIG_X86, with appropriate comment
> 
> Thanks, albeit ...
> 
> > --- a/xen/drivers/char/ns16550.c
> > +++ b/xen/drivers/char/ns16550.c
> > @@ -1238,6 +1238,17 @@ pci_uart_config(struct ns16550 *uart, bool_t 
> > skip_amt, unsigned int idx)
> >                              pci_conf_read8(PCI_SBDF(0, b, d, f),
> >                                             PCI_INTERRUPT_LINE) : 0;
> >  
> > +#ifdef CONFIG_X86
> > +                /* PCI Local Bus Specification Revision 3.0 defines 0xff 
> > value
> > +                 * as special only for X86 */
> 
> ... I'll have to remember to correct the style of the comment while
> committing ...
> 
> > +                if ( uart->irq == 0xff )
> > +                    uart->irq = 0;
> > +#endif
> > +                if ( !uart->irq )
> > +                    printk(XENLOG_INFO
> > +                           "ns16550: %pp no legacy IRQ, using poll mode\n",
> 
> ... and perhaps insert another colon after %pp here.

Both fine with me, thanks!

> Jan
> 
> > +                           &PCI_SBDF(0, b, d, f));
> > +
> >                  return 0;
> >              }
> >          }
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature