diff mbox series

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

Message ID 20220520085343.1835866-1-marmarek@invisiblethingslab.com (mailing list archive)
State New, archived
Headers show
Series [v6,1/2] ns16550: use poll mode if INTERRUPT_LINE is 0xff | expand

Commit Message

Marek Marczykowski-Górecki May 20, 2022, 8:53 a.m. UTC 
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>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes in v6:
 - wrap the check in additional CONFIG_X86, with appropriate comment
Changes in v5:
 - drop IRQ 0 from the log message
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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Jan Beulich May 20, 2022, 9:47 a.m. UTC | #1 
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@invisiblethingslab.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> 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.

Jan

> +                           &PCI_SBDF(0, b, d, f));
> +
>                  return 0;
>              }
>          }
Marek Marczykowski-Górecki May 20, 2022, 10:33 a.m. UTC | #2 
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@invisiblethingslab.com>
> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

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;
> >              }
> >          }
>
diff mbox series

Patch

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index fb75cee4a13a..b37f67dc7430 100644
--- 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 */
+                if ( uart->irq == 0xff )
+                    uart->irq = 0;
+#endif
+                if ( !uart->irq )
+                    printk(XENLOG_INFO
+                           "ns16550: %pp no legacy IRQ, using poll mode\n",
+                           &PCI_SBDF(0, b, d, f));
+
                 return 0;
             }
         }