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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.11.2020 10:21, Julien Grall wrote:
> 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....

Well, see my patch suppressing building of quite a part of it.

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

And I continue to fail to see what would guarantee this: As soon
as you can plug in such a card into an Arm system, people will
want to be able use it. That's why we had to add support for it
on x86, after all.

> So why do we need a finer graine Kconfig?

Because most of the involved code is indeed MSI-related?

Jan