[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 1/3] xen/ns16550: Make ns16550 driver usable on ARM with HAS_PCI enabled.
On 19/11/2020 09:53, Jan Beulich wrote: 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. I will let Rahul figuring out whether your patch series is sufficient to fix compilation issues (this is what matters right now). 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. Well, plug-in PCI cards on Arm has been available for quite a while... Yet I haven't heard anyone asking for NS16550 PCI support. This is probably because SBSA compliant server should always provide an SBSA UART (a cut-down version of the PL011). So why would bother to lose a PCI slot for yet another UART? >> So why do we need a finer graine Kconfig? Because most of the involved code is indeed MSI-related? Possibly, yet it would not be necessary if we don't want NS16550 PCI support... Cheers, -- Julien Grall
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |