Re: [PATCH v4 3/3] ns16550: Gate all PCI code with CONFIG_X86

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.11.2020 19:16, Rahul Singh wrote:
> --- 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
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)
>  #include <xen/pci.h>
>  #include <xen/pci_regs.h>
>  #include <xen/pci_ids.h>
> @@ -51,7 +51,7 @@ static struct ns16550 {
>      unsigned int timeout_ms;
>      bool_t intr_works;
>      bool_t dw_usr_bsy;
> -#ifdef CONFIG_HAS_PCI
> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI)

I'm sorry to be picky, but this being a hack wants, imo, also calling
it so, by way of a code comment. Clearly this should go at one of the
first instances, yet neither of the two above are really suitable imo.
Hence I'm coming back to my prior suggestion of introducing a
consolidated #define without this becoming a Kconfig setting:

/*
 * The PCI part of the code in this file currently is only known to
 * work on x86. Undo this hack once the logic has been suitably
 * abstracted.
 */
#if defined(CONFIG_HAS_PCI) && defined(CONFIG_X86)
# define NS16550_PCI
#endif

And then use NS16550_PCI everywhere. I'd be fine making this
adjustment while committing, if I knew that (a) you're okay with it
and (b) the R-b and A-b you've already got can be kept.

Jan