| Message ID | 20220511135929.1823116-1-marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/2] ns16550: use poll mode if INTERRUPT_LINE is 0xff | expand |
On Wed, May 11, 2022 at 03:59:28PM +0200, 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@invisiblethingslab.com> > --- > Changes in v4: > - adjust log message, change it from WARNING to INFO > - re-add x86 reference in the commit message > Changes in v3: > - change back to checking 0xff explicitly > - adjust commit message, include spec reference > - change warning to match the above > Changes in v2: > - add log message > - extend commit message > - code style fix > --- > xen/drivers/char/ns16550.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index fb75cee4a13a..c0d65cff62fe 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -1238,6 +1238,13 @@ 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; > > + if ( uart->irq == 0xff ) > + uart->irq = 0; > + if ( !uart->irq ) > + printk(XENLOG_INFO > + "ns16550: %pp no legacy IRQ %d, using poll mode\n", > + &PCI_SBDF(0, b, d, f), uart->irq); There's no point in printing ->irq as it will be 0 or else the message won't be printed. With that fixed: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks.
On 11/05/2022 15:15, Roger Pau Monné wrote: > On Wed, May 11, 2022 at 03:59:28PM +0200, 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@invisiblethingslab.com> >> --- >> Changes in v4: >> - adjust log message, change it from WARNING to INFO >> - re-add x86 reference in the commit message >> Changes in v3: >> - change back to checking 0xff explicitly >> - adjust commit message, include spec reference >> - change warning to match the above >> Changes in v2: >> - add log message >> - extend commit message >> - code style fix >> --- >> xen/drivers/char/ns16550.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c >> index fb75cee4a13a..c0d65cff62fe 100644 >> --- a/xen/drivers/char/ns16550.c >> +++ b/xen/drivers/char/ns16550.c >> @@ -1238,6 +1238,13 @@ 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; >> >> + if ( uart->irq == 0xff ) >> + uart->irq = 0; >> + if ( !uart->irq ) >> + printk(XENLOG_INFO >> + "ns16550: %pp no legacy IRQ %d, using poll mode\n", >> + &PCI_SBDF(0, b, d, f), uart->irq); > There's no point in printing ->irq as it will be 0 or else the message > won't be printed. > > With that fixed: > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> TBH, can be fixed on commit, save another round of patching. ~Andrew
On Wed, May 11, 2022 at 02:35:05PM +0000, Andrew Cooper wrote: > On 11/05/2022 15:15, Roger Pau Monné wrote: > > On Wed, May 11, 2022 at 03:59:28PM +0200, 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@invisiblethingslab.com> > >> --- > >> Changes in v4: > >> - adjust log message, change it from WARNING to INFO > >> - re-add x86 reference in the commit message > >> Changes in v3: > >> - change back to checking 0xff explicitly > >> - adjust commit message, include spec reference > >> - change warning to match the above > >> Changes in v2: > >> - add log message > >> - extend commit message > >> - code style fix > >> --- > >> xen/drivers/char/ns16550.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > >> index fb75cee4a13a..c0d65cff62fe 100644 > >> --- a/xen/drivers/char/ns16550.c > >> +++ b/xen/drivers/char/ns16550.c > >> @@ -1238,6 +1238,13 @@ 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; > >> > >> + if ( uart->irq == 0xff ) > >> + uart->irq = 0; > >> + if ( !uart->irq ) > >> + printk(XENLOG_INFO > >> + "ns16550: %pp no legacy IRQ %d, using poll mode\n", > >> + &PCI_SBDF(0, b, d, f), uart->irq); > > There's no point in printing ->irq as it will be 0 or else the message > > won't be printed. > > > > With that fixed: > > > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > TBH, can be fixed on commit, save another round of patching. Indeed, thanks for taking care of that. Roger.
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index fb75cee4a13a..c0d65cff62fe 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -1238,6 +1238,13 @@ 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; + if ( uart->irq == 0xff ) + uart->irq = 0; + if ( !uart->irq ) + printk(XENLOG_INFO + "ns16550: %pp no legacy IRQ %d, using poll mode\n", + &PCI_SBDF(0, b, d, f), uart->irq); + return 0; } }
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@invisiblethingslab.com> --- Changes in v4: - adjust log message, change it from WARNING to INFO - re-add x86 reference in the commit message Changes in v3: - change back to checking 0xff explicitly - adjust commit message, include spec reference - change warning to match the above Changes in v2: - add log message - extend commit message - code style fix --- xen/drivers/char/ns16550.c | 7 +++++++ 1 file changed, 7 insertions(+)