On 15.11.18 18:58, Andy Shevchenko wrote:
> Microsoft specifies a SPCR (Serial Port Console Redirection Table) [1].
> Let's provide it in U-Boot.
>
> [1]
> https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
>
> Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> ---
> arch/x86/include/asm/acpi_table.h | 2 +
> arch/x86/lib/acpi_table.c | 85 +++++++++++++++++++++++++++++++
> 2 files changed, 87 insertions(+)
>
> diff --git a/arch/x86/include/asm/acpi_table.h
> b/arch/x86/include/asm/acpi_table.h
> index 51cc806673..e3b65cff66 100644
> --- a/arch/x86/include/asm/acpi_table.h
> +++ b/arch/x86/include/asm/acpi_table.h
> @@ -327,6 +327,8 @@ struct acpi_global_nvs;
> #define ACPI_DBG2_USB_XHCI 0x0000
> #define ACPI_DBG2_USB_EHCI 0x0001
>
> +#define ACPI_DBG2_UNKNOWN 0x00FF
> +
> /* SPCR (Serial Port Console Redirection table) */
> struct __packed acpi_spcr {
> struct acpi_table_header header;
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index c6b2026613..d8b44aea02 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -12,6 +12,7 @@
> #include <cpu.h>
> #include <dm.h>
> #include <dm/uclass-internal.h>
> +#include <serial.h>
> #include <version.h>
> #include <asm/acpi/global_nvs.h>
> #include <asm/acpi_table.h>
> @@ -338,6 +339,82 @@ static void acpi_create_mcfg(struct acpi_mcfg *mcfg)
> header->checksum = table_compute_checksum((void *)mcfg, header->length);
> }
>
> +static void acpi_create_spcr(struct acpi_spcr *spcr)
> +{
> + struct acpi_table_header *header = &(spcr->header);
> + struct serial_device_info info = {0};
> + int access_size;
> + int ret;
> +
> + /* Fill out header fields */
> + acpi_fill_header(header, "SPCR");
> + header->length = sizeof(struct acpi_spcr);
> + header->revision = 2;
> +
> + ret = serial_getinfo(&info);
> + if (ret)
> + spcr->interface_type = ACPI_DBG2_UNKNOWN;
> + else
> + spcr->interface_type = ACPI_DBG2_16550_COMPATIBLE;
This sounds like pretty subtle semantics. Could you just include the
interface type in &info?
The main problem I'm seeing is that your UART might be a PL011
compatible which could still implement getinfo() but then wouldn't be
16660 compatible.
> + debug("UART type %u (serial_getinfo() rc=%d)\n", spcr->interface_type,
> ret);
> +
> + /* Encode baud rate */
> + switch (info.baudrate) {
> + case 9600:
> + spcr->baud_rate = 3;
> + break;
> + case 19200:
> + spcr->baud_rate = 4;
> + break;
> + case 57600:
> + spcr->baud_rate = 6;
> + break;
> + case 115200:
> + spcr->baud_rate = 7;
> + break;
Are any higher (and lower) ones specified too? If so, you may want to
add them as well.
> + default:
> + spcr->baud_rate = 0;
> + break;
> + }
> +
> + /* Encode register access size */
> + switch (info.reg_shift) {
> + case 0:
> + access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS;
> + break;
> + case 1:
> + access_size = ACPI_ACCESS_SIZE_WORD_ACCESS;
> + break;
> + case 2:
> + access_size = ACPI_ACCESS_SIZE_DWORD_ACCESS;
> + break;
> + case 3:
> + access_size = ACPI_ACCESS_SIZE_QWORD_ACCESS;
> + break;
> + default:
> + access_size = ACPI_ACCESS_SIZE_UNDEFINED;
> + break;
> + }
> +
> + spcr->serial_port.space_id = ACPI_ADDRESS_SPACE_MEMORY;
Can you guarantee that? Should probably be part of &info as well if addr is.
> + spcr->serial_port.bit_width = info.reg_width;
> + spcr->serial_port.bit_offset = info.reg_offset;
> + spcr->serial_port.access_size = access_size;
> + spcr->serial_port.addrl = info.addr >> 0;
> + spcr->serial_port.addrh = info.addr >> 32;
> +
> + /* REVISIT: Hard coded values for now */
> + spcr->parity = 0;
> + spcr->stop_bits = 1;
IMHO those should be part of &info as well. If you have to hard code
them, better hard code them in the driver rather than the generic ACPI
generation code.
Alex
> +
> + /* REVISIT: No PCI devices for now */
> + spcr->pci_device_id = 0xffff;
> + spcr->pci_vendor_id = 0xffff;
> +
> + /* Fix checksum */
> + header->checksum = table_compute_checksum((void *)spcr, header->length);
> +}
> +
> /*
> * QEMU's version of write_acpi_tables is defined in drivers/misc/qfw.c
> */
> @@ -352,6 +429,7 @@ ulong write_acpi_tables(ulong start)
> struct acpi_fadt *fadt;
> struct acpi_mcfg *mcfg;
> struct acpi_madt *madt;
> + struct acpi_spcr *spcr;
> int i;
>
> current = start;
> @@ -440,6 +518,13 @@ ulong write_acpi_tables(ulong start)
> acpi_add_table(rsdp, mcfg);
> current = ALIGN(current, 16);
>
> + debug("ACPI: * SPCR\n");
> + spcr = (struct acpi_spcr *)current;
> + acpi_create_spcr(spcr);
> + current += spcr->header.length;
> + acpi_add_table(rsdp, spcr);
> + current = ALIGN(current, 16);
> +
> debug("current = %x\n", current);
>
> acpi_rsdp_addr = (unsigned long)rsdp;
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
