> On 25 Nov 2020, at 18:21, Rahul Singh <[email protected]> wrote: > > Hello Julien, > >> On 25 Nov 2020, at 6:16 pm, Rahul Singh <[email protected]> wrote: >> >> The NS16550 driver is assuming that NS16550 PCI card are usable if the >> architecture supports PCI (i.e. CONFIG_HAS_PCI=y). However, the code is >> very x86 focus and will fail to build on Arm (/!\ it is not all the >> errors): >> >> ns16550.c: In function ‘ns16550_init_irq’: >> ns16550.c:726:21: error: implicit declaration of function ‘create_irq’; >> did you mean ‘release_irq’? [-Werror=implicit-function-declaration] >> uart->irq = create_irq(0, false); >> ^~~~~~~~~~ >> release_irq >> ns16550.c:726:21: error: nested extern declaration of ‘create_irq’ >> [-Werror=nested-externs] >> ns16550.c: In function ‘ns16550_init_postirq’: >> ns16550.c:768:33: error: ‘mmio_ro_ranges’ undeclared (first use in this >> function); did you mean ‘mmio_handler’? >> rangeset_add_range(mmio_ro_ranges, uart->io_base, >> ^~~~~~~~~~~~~~ >> mmio_handler >> ns16550.c:768:33: note: each undeclared identifier is reported only once >> for each function it appears in >> ns16550.c:780:20: error: variable ‘msi’ has initializer but incomplete >> type >> struct msi_info msi = { >> ^~~~~~~~ >> >> Enabling support for NS16550 PCI card on Arm would require more plumbing >> in addition to fixing the compilation error. >> >> Arm systems tend to have platform UART available such as NS16550, PL011. >> So there are limited reasons to get NS16550 PCI support for now on Arm. >> >> Guard all remaining PCI code that is not under x86 flag with CONFIG_X86. >> >> No functional change intended. >> >> Signed-off-by: Rahul Singh <[email protected]> Reviewed-by: Bertrand Marquis <[email protected]>
> > Sorry I missed to add the signed off for the commit msg. I will send the next > version once reviewed done. > Signed-off-by: Julien Grall <[email protected]> > I guess the commiter can add this while commiting this patch ? Regards Bertrand > Regards, > Rahul >> --- >> >> Changes in v4: >> - As per the discussion guard all remaining PCI code with CONFIG_X86 >> >> --- >> xen/drivers/char/ns16550.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c >> index 9235d854fe..26e601857a 100644 >> --- 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) >> /* PCI card parameters. */ >> bool_t pb_bdf_enable; /* if =1, pb-bdf effective, port behind bridge */ >> bool_t ps_bdf_enable; /* if =1, ps_bdf effective, port on pci card */ >> @@ -66,7 +66,7 @@ static struct ns16550 { >> #endif >> } ns16550_com[2] = { { 0 } }; >> >> -#ifdef CONFIG_HAS_PCI >> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI) >> struct ns16550_config { >> u16 vendor_id; >> u16 dev_id; >> @@ -256,7 +256,7 @@ static int ns16550_getc(struct serial_port *port, char >> *pc) >> >> static void pci_serial_early_init(struct ns16550 *uart) >> { >> -#ifdef CONFIG_HAS_PCI >> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI) >> if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 ) >> return; >> >> @@ -355,7 +355,7 @@ static void __init ns16550_init_preirq(struct >> serial_port *port) >> >> static void __init ns16550_init_irq(struct serial_port *port) >> { >> -#ifdef CONFIG_HAS_PCI >> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI) >> struct ns16550 *uart = port->uart; >> >> if ( uart->msi ) >> @@ -397,7 +397,7 @@ static void __init ns16550_init_postirq(struct >> serial_port *port) >> uart->timeout_ms = max_t( >> unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud); >> >> -#ifdef CONFIG_HAS_PCI >> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI) >> if ( uart->bar || uart->ps_bdf_enable ) >> { >> if ( uart->param && uart->param->mmio && >> @@ -477,7 +477,7 @@ static void ns16550_suspend(struct serial_port *port) >> >> stop_timer(&uart->timer); >> >> -#ifdef CONFIG_HAS_PCI >> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI) >> if ( uart->bar ) >> uart->cr = pci_conf_read16(PCI_SBDF(0, uart->ps_bdf[0], >> uart->ps_bdf[1], >> uart->ps_bdf[2]), PCI_COMMAND); >> @@ -486,7 +486,7 @@ static void ns16550_suspend(struct serial_port *port) >> >> static void _ns16550_resume(struct serial_port *port) >> { >> -#ifdef CONFIG_HAS_PCI >> +#if defined(CONFIG_X86) && defined(CONFIG_HAS_PCI) >> struct ns16550 *uart = port->uart; >> >> if ( uart->bar ) >> -- >> 2.17.1
