[U-Boot] [PATCH v1 0/2] ACPI: Generate SPCR table

This is a WIP series (I have no time to continue with it, so, that's why WIP) to enable SPCR table generation in U-Boot. This table is useful to get early console in Linux for debugging purposes. The benefit of using it is not only for x86, but also for arm64 community (actually they introduced support in Linux kernel).
This PoC implementation has been tested on Intel Edison with enabled ACPI support.
Known issues: - there is no ->getconfig() callback, thus some data is hard coded - if serial parameters are changed at run time, SPCR is not regenerated, thus, might not produce what user excepts (this is more likely design issue of U-Boot itself) - implementation is not final and might not utilize best practices in U-Boot
So, if there is any interest, I would be happy to help, but as I mentioned above I have no capacity to continue this myself.
The patches would be also available through my U-Boot tree on GH.
Andy Shevchenko (2): WIP: serial: Introduce ->getinfo() callback WIP: x86: acpi: Generate SPCR table
arch/x86/include/asm/acpi_table.h | 25 ++++++++++ arch/x86/lib/acpi_table.c | 82 +++++++++++++++++++++++++++++++ drivers/serial/ns16550.c | 14 ++++++ drivers/serial/serial-uclass.c | 21 ++++++++ include/common.h | 3 ++ include/serial.h | 14 ++++++ 6 files changed, 159 insertions(+)

TBD
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/serial/ns16550.c | 14 ++++++++++++++ drivers/serial/serial-uclass.c | 21 +++++++++++++++++++++ include/common.h | 3 +++ include/serial.h | 14 ++++++++++++++ 4 files changed, 52 insertions(+)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index f9041aa626..a996446936 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -334,6 +334,19 @@ static int ns16550_serial_setbrg(struct udevice *dev, int baudrate) return 0; }
+static int ns16550_serial_getinfo(struct udevice *dev, struct serial_device_info *info) +{ + struct NS16550 *const com_port = dev_get_priv(dev); + struct ns16550_platdata *plat = com_port->plat; + + info->addr_space = 0; + info->reg_width = 8; + info->reg_shift = plat->reg_shift; + info->reg_offset = plat->reg_offset; + info->addr = plat->base; + return 0; +} + int ns16550_serial_probe(struct udevice *dev) { struct NS16550 *const com_port = dev_get_priv(dev); @@ -440,6 +453,7 @@ const struct dm_serial_ops ns16550_serial_ops = { .pending = ns16550_serial_pending, .getc = ns16550_serial_getc, .setbrg = ns16550_serial_setbrg, + .getinfo = ns16550_serial_getinfo, };
#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index ffdcae0931..4778302e8f 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -304,6 +304,25 @@ int serial_setconfig(uint config) return 0; }
+int serial_getinfo(struct serial_device_info *info) +{ + struct dm_serial_ops *ops; + + if (!gd->cur_serial_dev) + return -ENODEV; + + if (!info) + return -EINVAL; + + info->baudrate = gd->baudrate; + + ops = serial_get_ops(gd->cur_serial_dev); + if (ops->getinfo) + return ops->getinfo(gd->cur_serial_dev, info); + + return -EINVAL; +} + void serial_stdio_init(void) { } @@ -421,6 +440,8 @@ static int serial_post_probe(struct udevice *dev) if (ops->loop) ops->loop += gd->reloc_off #endif + if (ops->getinfo) + ops->getinfo += gd->reloc_off; #endif /* Set the baud rate */ if (ops->setbrg) { diff --git a/include/common.h b/include/common.h index 83b3bdc58d..0479f3eac1 100644 --- a/include/common.h +++ b/include/common.h @@ -349,6 +349,8 @@ void smp_set_core_boot_addr(unsigned long addr, int corenr); void smp_kick_all_cpus(void);
/* $(CPU)/serial.c */ +struct serial_device_info; + int serial_init (void); void serial_setbrg (void); void serial_putc (const char); @@ -357,6 +359,7 @@ void serial_puts (const char *); int serial_getc (void); int serial_tstc (void); int serial_setconfig(uint config); +int serial_getinfo(struct serial_device_info *info);
/* $(CPU)/speed.c */ int get_clocks (void); diff --git a/include/serial.h b/include/serial.h index 020cd392e8..c73477b079 100644 --- a/include/serial.h +++ b/include/serial.h @@ -111,6 +111,16 @@ enum serial_stop { SERIAL_8_BITS << SERIAL_BITS_SHIFT | \ SERIAL_ONE_STOP << SERIAL_STOP_SHIFT
+/* REVISIT: ACPI GAS specification implied */ +struct serial_device_info { + unsigned int baudrate; + u8 addr_space; /* 0 - MMIO, 1 - IO */ + u8 reg_width; + u8 reg_offset; + u8 reg_shift; + u64 addr; +}; + /** * struct struct dm_serial_ops - Driver model serial operations * @@ -201,6 +211,10 @@ struct dm_serial_ops { * @return 0 if OK, -ve on error */ int (*setconfig)(struct udevice *dev, uint serial_config); + /** + * getinfo() - Get serial device information + */ + int (*getinfo)(struct udevice *dev, struct serial_device_info *info); };
/**

Hi Andy
On 09/22/2018 03:05 PM, Andy Shevchenko wrote:
TBD
You forgot to add a commit message ;-)
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
drivers/serial/ns16550.c | 14 ++++++++++++++ drivers/serial/serial-uclass.c | 21 +++++++++++++++++++++ include/common.h | 3 +++ include/serial.h | 14 ++++++++++++++ 4 files changed, 52 insertions(+)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index f9041aa626..a996446936 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -334,6 +334,19 @@ static int ns16550_serial_setbrg(struct udevice *dev, int baudrate) return 0; }
+static int ns16550_serial_getinfo(struct udevice *dev, struct serial_device_info *info) +{
- struct NS16550 *const com_port = dev_get_priv(dev);
- struct ns16550_platdata *plat = com_port->plat;
- info->addr_space = 0;
- info->reg_width = 8;
- info->reg_shift = plat->reg_shift;
- info->reg_offset = plat->reg_offset;
- info->addr = plat->base;
- return 0;
+}
int ns16550_serial_probe(struct udevice *dev) { struct NS16550 *const com_port = dev_get_priv(dev); @@ -440,6 +453,7 @@ const struct dm_serial_ops ns16550_serial_ops = { .pending = ns16550_serial_pending, .getc = ns16550_serial_getc, .setbrg = ns16550_serial_setbrg,
- .getinfo = ns16550_serial_getinfo,
};
#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index ffdcae0931..4778302e8f 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -304,6 +304,25 @@ int serial_setconfig(uint config) return 0; }
+int serial_getinfo(struct serial_device_info *info) +{
- struct dm_serial_ops *ops;
- if (!gd->cur_serial_dev)
return -ENODEV;
- if (!info)
return -EINVAL;
- info->baudrate = gd->baudrate;
- ops = serial_get_ops(gd->cur_serial_dev);
- if (ops->getinfo)
return ops->getinfo(gd->cur_serial_dev, info);
- return -EINVAL;
+}
void serial_stdio_init(void) { } @@ -421,6 +440,8 @@ static int serial_post_probe(struct udevice *dev) if (ops->loop) ops->loop += gd->reloc_off #endif
- if (ops->getinfo)
ops->getinfo += gd->reloc_off;
#endif /* Set the baud rate */ if (ops->setbrg) { diff --git a/include/common.h b/include/common.h index 83b3bdc58d..0479f3eac1 100644 --- a/include/common.h +++ b/include/common.h @@ -349,6 +349,8 @@ void smp_set_core_boot_addr(unsigned long addr, int corenr); void smp_kick_all_cpus(void);
/* $(CPU)/serial.c */ +struct serial_device_info;
int serial_init (void); void serial_setbrg (void); void serial_putc (const char); @@ -357,6 +359,7 @@ void serial_puts (const char *); int serial_getc (void); int serial_tstc (void); int serial_setconfig(uint config); +int serial_getinfo(struct serial_device_info *info);
/* $(CPU)/speed.c */ int get_clocks (void); diff --git a/include/serial.h b/include/serial.h index 020cd392e8..c73477b079 100644 --- a/include/serial.h +++ b/include/serial.h @@ -111,6 +111,16 @@ enum serial_stop { SERIAL_8_BITS << SERIAL_BITS_SHIFT | \ SERIAL_ONE_STOP << SERIAL_STOP_SHIFT
+/* REVISIT: ACPI GAS specification implied */ +struct serial_device_info {
- unsigned int baudrate;
- u8 addr_space; /* 0 - MMIO, 1 - IO */
- u8 reg_width;
- u8 reg_offset;
- u8 reg_shift;
- u64 addr;
+};
/**
- struct struct dm_serial_ops - Driver model serial operations
@@ -201,6 +211,10 @@ struct dm_serial_ops { * @return 0 if OK, -ve on error */ int (*setconfig)(struct udevice *dev, uint serial_config);
- /**
* getinfo() - Get serial device information
*/
- int (*getinfo)(struct udevice *dev, struct serial_device_info *info);
};
/**

On Mon, Sep 24, 2018 at 10:44 AM Patrice CHOTARD patrice.chotard@st.com wrote:
Hi Andy
On 09/22/2018 03:05 PM, Andy Shevchenko wrote:
TBD
You forgot to add a commit message ;-)
Yes, cover letter explains why. It's a halfbaked stuff in case someone is interested to continue.
For your convenience: patch 1: introduces a hook to retrieve necessary data from UART driver (in particular ns16550) patch 2: adds SPCR data structures and uses hook from patch 1 to generate the table
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
drivers/serial/ns16550.c | 14 ++++++++++++++ drivers/serial/serial-uclass.c | 21 +++++++++++++++++++++ include/common.h | 3 +++ include/serial.h | 14 ++++++++++++++ 4 files changed, 52 insertions(+)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index f9041aa626..a996446936 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -334,6 +334,19 @@ static int ns16550_serial_setbrg(struct udevice *dev, int baudrate) return 0; }
+static int ns16550_serial_getinfo(struct udevice *dev, struct serial_device_info *info) +{
struct NS16550 *const com_port = dev_get_priv(dev);
struct ns16550_platdata *plat = com_port->plat;
info->addr_space = 0;
info->reg_width = 8;
info->reg_shift = plat->reg_shift;
info->reg_offset = plat->reg_offset;
info->addr = plat->base;
return 0;
+}
int ns16550_serial_probe(struct udevice *dev) { struct NS16550 *const com_port = dev_get_priv(dev); @@ -440,6 +453,7 @@ const struct dm_serial_ops ns16550_serial_ops = { .pending = ns16550_serial_pending, .getc = ns16550_serial_getc, .setbrg = ns16550_serial_setbrg,
.getinfo = ns16550_serial_getinfo,
};
#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index ffdcae0931..4778302e8f 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -304,6 +304,25 @@ int serial_setconfig(uint config) return 0; }
+int serial_getinfo(struct serial_device_info *info) +{
struct dm_serial_ops *ops;
if (!gd->cur_serial_dev)
return -ENODEV;
if (!info)
return -EINVAL;
info->baudrate = gd->baudrate;
ops = serial_get_ops(gd->cur_serial_dev);
if (ops->getinfo)
return ops->getinfo(gd->cur_serial_dev, info);
return -EINVAL;
+}
void serial_stdio_init(void) { } @@ -421,6 +440,8 @@ static int serial_post_probe(struct udevice *dev) if (ops->loop) ops->loop += gd->reloc_off #endif
if (ops->getinfo)
ops->getinfo += gd->reloc_off;
#endif /* Set the baud rate */ if (ops->setbrg) { diff --git a/include/common.h b/include/common.h index 83b3bdc58d..0479f3eac1 100644 --- a/include/common.h +++ b/include/common.h @@ -349,6 +349,8 @@ void smp_set_core_boot_addr(unsigned long addr, int corenr); void smp_kick_all_cpus(void);
/* $(CPU)/serial.c */ +struct serial_device_info;
int serial_init (void); void serial_setbrg (void); void serial_putc (const char); @@ -357,6 +359,7 @@ void serial_puts (const char *); int serial_getc (void); int serial_tstc (void); int serial_setconfig(uint config); +int serial_getinfo(struct serial_device_info *info);
/* $(CPU)/speed.c */ int get_clocks (void); diff --git a/include/serial.h b/include/serial.h index 020cd392e8..c73477b079 100644 --- a/include/serial.h +++ b/include/serial.h @@ -111,6 +111,16 @@ enum serial_stop { SERIAL_8_BITS << SERIAL_BITS_SHIFT | \ SERIAL_ONE_STOP << SERIAL_STOP_SHIFT
+/* REVISIT: ACPI GAS specification implied */ +struct serial_device_info {
unsigned int baudrate;
u8 addr_space; /* 0 - MMIO, 1 - IO */
u8 reg_width;
u8 reg_offset;
u8 reg_shift;
u64 addr;
+};
/**
- struct struct dm_serial_ops - Driver model serial operations
@@ -201,6 +211,10 @@ struct dm_serial_ops { * @return 0 if OK, -ve on error */ int (*setconfig)(struct udevice *dev, uint serial_config);
/**
* getinfo() - Get serial device information
*/
int (*getinfo)(struct udevice *dev, struct serial_device_info *info);
};
/**
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
-- With Best Regards, Andy Shevchenko

Hi Andy,
On Sat, Sep 22, 2018 at 9:05 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
TBD
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
drivers/serial/ns16550.c | 14 ++++++++++++++ drivers/serial/serial-uclass.c | 21 +++++++++++++++++++++ include/common.h | 3 +++ include/serial.h | 14 ++++++++++++++ 4 files changed, 52 insertions(+)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index f9041aa626..a996446936 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -334,6 +334,19 @@ static int ns16550_serial_setbrg(struct udevice *dev, int baudrate) return 0; }
+static int ns16550_serial_getinfo(struct udevice *dev, struct serial_device_info *info) +{
struct NS16550 *const com_port = dev_get_priv(dev);
struct ns16550_platdata *plat = com_port->plat;
info->addr_space = 0;
This one is tricky. Looks current 8250 DT binding does not have a property to describe the address space to be MMIO or PIO. I am not sure what's the best option here.
info->reg_width = 8;
For this one, I believe we can use "reg-io-width" property from device tree.
info->reg_shift = plat->reg_shift;
info->reg_offset = plat->reg_offset;
info->addr = plat->base;
return 0;
+}
int ns16550_serial_probe(struct udevice *dev) { struct NS16550 *const com_port = dev_get_priv(dev); @@ -440,6 +453,7 @@ const struct dm_serial_ops ns16550_serial_ops = { .pending = ns16550_serial_pending, .getc = ns16550_serial_getc, .setbrg = ns16550_serial_setbrg,
.getinfo = ns16550_serial_getinfo,
};
#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index ffdcae0931..4778302e8f 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -304,6 +304,25 @@ int serial_setconfig(uint config) return 0; }
+int serial_getinfo(struct serial_device_info *info) +{
struct dm_serial_ops *ops;
if (!gd->cur_serial_dev)
return -ENODEV;
if (!info)
return -EINVAL;
info->baudrate = gd->baudrate;
ops = serial_get_ops(gd->cur_serial_dev);
if (ops->getinfo)
return ops->getinfo(gd->cur_serial_dev, info);
return -EINVAL;
+}
void serial_stdio_init(void) { } @@ -421,6 +440,8 @@ static int serial_post_probe(struct udevice *dev) if (ops->loop) ops->loop += gd->reloc_off #endif
if (ops->getinfo)
ops->getinfo += gd->reloc_off;
#endif /* Set the baud rate */ if (ops->setbrg) { diff --git a/include/common.h b/include/common.h index 83b3bdc58d..0479f3eac1 100644 --- a/include/common.h +++ b/include/common.h @@ -349,6 +349,8 @@ void smp_set_core_boot_addr(unsigned long addr, int corenr); void smp_kick_all_cpus(void);
/* $(CPU)/serial.c */ +struct serial_device_info;
int serial_init (void); void serial_setbrg (void); void serial_putc (const char); @@ -357,6 +359,7 @@ void serial_puts (const char *); int serial_getc (void); int serial_tstc (void); int serial_setconfig(uint config); +int serial_getinfo(struct serial_device_info *info);
/* $(CPU)/speed.c */ int get_clocks (void); diff --git a/include/serial.h b/include/serial.h index 020cd392e8..c73477b079 100644 --- a/include/serial.h +++ b/include/serial.h @@ -111,6 +111,16 @@ enum serial_stop { SERIAL_8_BITS << SERIAL_BITS_SHIFT | \ SERIAL_ONE_STOP << SERIAL_STOP_SHIFT
+/* REVISIT: ACPI GAS specification implied */ +struct serial_device_info {
unsigned int baudrate;
u8 addr_space; /* 0 - MMIO, 1 - IO */
u8 reg_width;
u8 reg_offset;
u8 reg_shift;
u64 addr;
+};
/**
- struct struct dm_serial_ops - Driver model serial operations
@@ -201,6 +211,10 @@ struct dm_serial_ops { * @return 0 if OK, -ve on error */ int (*setconfig)(struct udevice *dev, uint serial_config);
/**
* getinfo() - Get serial device information
*/
int (*getinfo)(struct udevice *dev, struct serial_device_info *info);
};
Generally this patch looks OK to me.
BTW: why doesn't the kernel use the 'earlycon' command line?
Regards, Bin

On Tue, Sep 25, 2018 at 10:24:42AM +0800, Bin Meng wrote:
On Sat, Sep 22, 2018 at 9:05 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
I found a bit of time to look at this again.
+static int ns16550_serial_getinfo(struct udevice *dev, struct serial_device_info *info) +{
struct NS16550 *const com_port = dev_get_priv(dev);
struct ns16550_platdata *plat = com_port->plat;
info->addr_space = 0;
This one is tricky. Looks current 8250 DT binding does not have a property to describe the address space to be MMIO or PIO. I am not sure what's the best option here.
For now I suggest to leave it hard coded to MMIO.
info->reg_width = 8;
For this one, I believe we can use "reg-io-width" property from device tree.
I will try this path.
info->reg_shift = plat->reg_shift;
info->reg_offset = plat->reg_offset;
info->addr = plat->base;
return 0;
+}
Generally this patch looks OK to me.
Thanks! Still I have few amendments for the real submission of the series.
BTW: why doesn't the kernel use the 'earlycon' command line?
Not all hardware supports it or supports it in a right way.

TBD
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- arch/x86/include/asm/acpi_table.h | 25 ++++++++++ arch/x86/lib/acpi_table.c | 82 +++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+)
diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h index 95fae036f6..27435871b9 100644 --- a/arch/x86/include/asm/acpi_table.h +++ b/arch/x86/include/asm/acpi_table.h @@ -303,6 +303,31 @@ struct acpi_mcfg_mmconfig { /* ACPI global NVS structure */ struct acpi_global_nvs;
+/* SPCR (Serial Port Console Redirection table) */ +struct __packed acpi_spcr { + struct acpi_table_header header; /* Common ACPI table header */ + u8 interface_type; /* 0=full 16550, 1=subset of 16550 */ + u8 reserved[3]; + struct acpi_gen_regaddr serial_port; /* The base address of the Serial Port register set */ + u8 interrupt_type; + u8 pc_interrupt; + u32 interrupt; /* Global system interrupt */ + u8 baud_rate; + u8 parity; + u8 stop_bits; + u8 flow_control; + u8 terminal_type; + u8 reserved1; + u16 pci_device_id; /* Must be 0xffff if not PCI device */ + u16 pci_vendor_id; /* Must be 0xffff if not PCI device */ + u8 pci_bus; + u8 pci_device; + u8 pci_function; + u32 pci_flags; + u8 pci_segment; + u32 reserved2; +}; + /* These can be used by the target port */
void acpi_fill_header(struct acpi_table_header *header, char *signature); diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c index c6b2026613..551a78b11a 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,79 @@ 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) + debug("Can't get information of serial device: %d\n", 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; + 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; + 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; + + /* Hard coded values for now */ + spcr->parity = 0; + spcr->stop_bits = 1; + + /* 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 +426,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 +515,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;

On 09/22/2018 03:05 PM, Andy Shevchenko wrote:
TBD
Same remark as Patch 1
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
arch/x86/include/asm/acpi_table.h | 25 ++++++++++ arch/x86/lib/acpi_table.c | 82 +++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+)
diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h index 95fae036f6..27435871b9 100644 --- a/arch/x86/include/asm/acpi_table.h +++ b/arch/x86/include/asm/acpi_table.h @@ -303,6 +303,31 @@ struct acpi_mcfg_mmconfig { /* ACPI global NVS structure */ struct acpi_global_nvs;
+/* SPCR (Serial Port Console Redirection table) */ +struct __packed acpi_spcr {
- struct acpi_table_header header; /* Common ACPI table header */
- u8 interface_type; /* 0=full 16550, 1=subset of 16550 */
- u8 reserved[3];
- struct acpi_gen_regaddr serial_port; /* The base address of the Serial Port register set */
- u8 interrupt_type;
- u8 pc_interrupt;
- u32 interrupt; /* Global system interrupt */
- u8 baud_rate;
- u8 parity;
- u8 stop_bits;
- u8 flow_control;
- u8 terminal_type;
- u8 reserved1;
- u16 pci_device_id; /* Must be 0xffff if not PCI device */
- u16 pci_vendor_id; /* Must be 0xffff if not PCI device */
- u8 pci_bus;
- u8 pci_device;
- u8 pci_function;
- u32 pci_flags;
- u8 pci_segment;
- u32 reserved2;
+};
/* These can be used by the target port */
void acpi_fill_header(struct acpi_table_header *header, char *signature); diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c index c6b2026613..551a78b11a 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,79 @@ 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)
debug("Can't get information of serial device: %d\n", 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;
- 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;
- 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;
- /* Hard coded values for now */
- spcr->parity = 0;
- spcr->stop_bits = 1;
- /* 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 +426,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 +515,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;

Hi Andy,
On Sat, Sep 22, 2018 at 9:05 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
TBD
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
arch/x86/include/asm/acpi_table.h | 25 ++++++++++ arch/x86/lib/acpi_table.c | 82 +++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+)
diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h index 95fae036f6..27435871b9 100644 --- a/arch/x86/include/asm/acpi_table.h +++ b/arch/x86/include/asm/acpi_table.h @@ -303,6 +303,31 @@ struct acpi_mcfg_mmconfig { /* ACPI global NVS structure */ struct acpi_global_nvs;
+/* SPCR (Serial Port Console Redirection table) */ +struct __packed acpi_spcr {
struct acpi_table_header header; /* Common ACPI table header */
u8 interface_type; /* 0=full 16550, 1=subset of 16550 */
u8 reserved[3];
struct acpi_gen_regaddr serial_port; /* The base address of the Serial Port register set */
u8 interrupt_type;
u8 pc_interrupt;
u32 interrupt; /* Global system interrupt */
u8 baud_rate;
u8 parity;
u8 stop_bits;
u8 flow_control;
u8 terminal_type;
u8 reserved1;
u16 pci_device_id; /* Must be 0xffff if not PCI device */
u16 pci_vendor_id; /* Must be 0xffff if not PCI device */
u8 pci_bus;
u8 pci_device;
u8 pci_function;
u32 pci_flags;
u8 pci_segment;
u32 reserved2;
+};
/* These can be used by the target port */
void acpi_fill_header(struct acpi_table_header *header, char *signature); diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c index c6b2026613..551a78b11a 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,79 @@ 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)
debug("Can't get information of serial device: %d\n", 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;
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;
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;
/* Hard coded values for now */
spcr->parity = 0;
spcr->stop_bits = 1;
/* No PCI devices for now */
spcr->pci_device_id = 0xffff;
spcr->pci_vendor_id = 0xffff;
I see not every member of 'struct __packed acpi_spcr' is populated. Are they not used by kernel? If kernel does not use it, it is used by Windows?
/* 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 +426,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 +515,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;
--
Regards, Bin

On Tue, Sep 25, 2018 at 10:27:33AM +0800, Bin Meng wrote:
On Sat, Sep 22, 2018 at 9:05 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
+/* SPCR (Serial Port Console Redirection table) */ +struct __packed acpi_spcr {
struct acpi_table_header header; /* Common ACPI table header */
u8 interface_type; /* 0=full 16550, 1=subset of 16550 */
u8 reserved[3];
struct acpi_gen_regaddr serial_port; /* The base address of the Serial Port register set */
u8 interrupt_type;
u8 pc_interrupt;
u32 interrupt; /* Global system interrupt */
u8 baud_rate;
u8 parity;
u8 stop_bits;
u8 flow_control;
u8 terminal_type;
u8 reserved1;
u16 pci_device_id; /* Must be 0xffff if not PCI device */
u16 pci_vendor_id; /* Must be 0xffff if not PCI device */
u8 pci_bus;
u8 pci_device;
u8 pci_function;
u32 pci_flags;
u8 pci_segment;
u32 reserved2;
+};
I see not every member of 'struct __packed acpi_spcr' is populated. Are they not used by kernel? If kernel does not use it, it is used by Windows?
No idea about Windows, but there are two statements: - *not all* should be filled, otherwise it would be contradictory (PCI vs. non-PCI device, for example) - Linux doesn't use all of them, indeed
participants (4)
-
Andy Shevchenko
-
Andy Shevchenko
-
Bin Meng
-
Patrice CHOTARD