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.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
