On 18.07.2023 16:13, Oleksii Kurochko wrote:
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -40,6 +40,8 @@
> #include <asm/fixmap.h>
> #endif
>
> +#define NO_IRQ_POLL 0
Do you really need this? I ask because ...
> @@ -595,7 +603,9 @@ static void __init cf_check ns16550_endboot(struct
> serial_port *port)
> static int __init cf_check ns16550_irq(struct serial_port *port)
> {
> struct ns16550 *uart = port->uart;
> - return ((uart->irq > 0) ? uart->irq : -1);
> +
> + return (((uart->intr_works != polling) && (uart->irq >= 0)) ?
... you now use >= here, which includes that special value. As long
as intr_works is always set to "polling", the particular value in
uart->irq shouldn't matter (and hence you wouldn't need to store
anywhere that or any other special value).
> @@ -1330,9 +1340,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->irq = NO_IRQ_POLL;
> + uart->intr_works = polling;
> + }
> #endif
> - if ( !uart->irq )
> + if ( uart->intr_works == polling )
Careful here - we may also have read 0 from PCI_INTERRUPT_LINE, or
forced 0 because we read 0 from PCI_INTERRUPT_PIN. All these cases,
unless provably broken, need to continue to function as they were.
Further you alter parse_positional(), but you leave alone
parse_namevalue_pairs(). I think you're changing the admin (command
line) interface that way, because so far "irq=0" was the way to
request polling. While it may be unavoidable to change that interface
(which will then need noting in ./CHANGELOG.md), you still need to
offer a way to forcibly set polling mode.
Jan
