Re: [PATCH v3 1/3] xen/ns16550: Make ns16550 driver usable on ARM with HAS_PCI enabled.

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.11.2020 13:25, Rahul Singh wrote:
> NS16550 driver has PCI support that is under HAS_PCI flag. When HAS_PCI
> is enabled for ARM, compilation error is observed for ARM architecture
> because ARM platforms do not have full PCI support available.

While you've extended the sentence, it remains unclear to me what
compilation error it is that results here. I've requested such
clarification for v2 already.

> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -4,6 +4,10 @@ config HAS_NS16550
>       help
>         This selects the 16550-series UART support. For most systems, say Y.
>  
> +config HAS_NS16550_PCI
> +     def_bool y
> +     depends on X86 && HAS_NS16550 && HAS_PCI

Looking at this again (in particular at all the #ifdef changes in
the actual source file), I wonder whether an approach with less
code churn and without such an extra Kconfig setting (with, as
said, a bogus dependency on x86) couldn't be found. For example,
how about ...

> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -16,7 +16,7 @@
>  #include <xen/timer.h>
>  #include <xen/serial.h>
>  #include <xen/iocap.h>
> -#ifdef CONFIG_HAS_PCI
> +#ifdef CONFIG_HAS_NS16550_PCI
>  #include <xen/pci.h>
>  #include <xen/pci_regs.h>
>  #include <xen/pci_ids.h>

... #undef-ining CONFIG_HAS_PCI at a suitable position in this
file (e.g. after all #include-s, to make sure all structure
layouts remain correct)? This would then be far easier to revert
down the road, and would confine the oddity to a single file
(and there a single place) in the code base.

Jan