Re: [Xen-devel] [PATCH v4] ns16550: Add ACPI support for ARM only

[Julien Grall <julien@xxxxxxx>]

On 21/02/2020 14:02, Jan Beulich wrote:
> On 21.02.2020 03:22, Wei Xu wrote:
> > --- a/xen/drivers/char/ns16550.c
> > +++ b/xen/drivers/char/ns16550.c
> > @@ -1620,6 +1620,85 @@ DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
> >   DT_DEVICE_END
> >
> >   #endif /* HAS_DEVICE_TREE */
> > +
> > +#if defined(CONFIG_ACPI) && defined(CONFIG_ARM)
> > +#include <xen/acpi.h>
> > +
> > +static int __init ns16550_acpi_uart_init(const void *data)
> > +{
> > +    struct acpi_table_header *table;
> > +    struct acpi_table_spcr *spcr;
> > +    acpi_status status;
> > +    /*
> > +     * Same as the DT part.
> > +     * Only support one UART on ARM which happen to be ns16550_com[0].
> > +     */
> > +    struct ns16550 *uart = &ns16550_com[0];
> > +
> > +    status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
> > +    if ( ACPI_FAILURE(status) )
> > +    {
> > +        printk("ns16550: Failed to get SPCR table\n");
> > +        return -EINVAL;
> > +    }
> > +
> > +    spcr = container_of(table, struct acpi_table_spcr, header);
> > +
> > +    /*
> > +     * The serial port address may be 0 for example
> > +     * if the console redirection is disabled.
> > +     */
> > +    if ( unlikely(!spcr->serial_port.address) )
> > +    {
> > +        printk("ns16550: Console redirection is disabled\n");
> > +        return -EINVAL;
> > +    }
> > +
> > +    if ( unlikely(spcr->serial_port.space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY) )
> > +    {
> > +        printk("ns16550: Address space type is not mmio\n");
> > +        return -EINVAL;
> > +    }
> The space_id field qualifies the address one, i.e. whether a value of
> zero can sensibly mean "disabled" depends on the address space. Hence
> logically the address space check should come first.
>
> This is the last thing I'd like to see changed. I won't give the
> patch my ack though, as I think it should be an Arm maintainer to ack
> it.
Acked-by: Julien Grall <julien@xxxxxxx>

Although, a reviewed-by tag from you would be nice as you did most of 
the review for this patch.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel