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

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 19/11/2020 09:05, Jan Beulich wrote:
> On 18.11.2020 16:50, Julien Grall wrote:
> > On 16/11/2020 12: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.
> >   >
> > > Introducing new kconfig option CONFIG_HAS_NS16550_PCI to support
> > > ns16550 PCI for X86.
> > >
> > > For X86 platforms it is enabled by default. For ARM platforms it is
> > > disabled by default, once we have proper support for NS16550 PCI for
> > > ARM we can enable it.
> > >
> > > No functional change.
> > NIT: I would say "No functional change intended" to make clear this is
> > an expectation and hopefully will be correct :).
> >
> > Regarding the commit message itself, I would suggest the following to
> > address Jan's concern:
> While indeed this is a much better description, I continue to think
> that the proposed Kconfig option is undesirable to have.
I am yet to see an argument into why we should keep the PCI code 
compiled on Arm when there will be no-use....
> Either,
> following the patch I've just sent, truly x86-specific things (at
> least as far as current state goes - if any of this was to be
> re-used by a future port, suitable further abstraction may be
> needed) should be guarded by CONFIG_X86 (or abstracted into arch
> hooks), or the HAS_PCI_MSI proposal would at least want further
> investigating as to its feasibility to address the issues at hand.
I would be happy with CONFIG_X86, despite the fact that this is only 
deferring the problem.
Regarding HAS_PCI_MSI, I don't really see the point of introducing given 
that we are not going to use NS16550 PCI on Arm in the forseeable 
future. So why do we need a finer graine Kconfig?
Cheers,

--
Julien Grall