[PATCH 00/17] misc: Some more misc patches
Various issues were discovered in getting Chromium OS verified boot running on top of coreboot, booting into U-Boot.
Improvements include: - enable serial console if even coreboot doesn't - enable cache always - show the BIOS date since Chromium OS's coreboot doesn't have a version - update docs - allow SPI driver to work without a PCH - fix mtrr command when multi-processing (CONFIG_MP) is disabled - add documentation about the memory map
Simon Glass (17): pci: Use const for pci_find_device_id() etc. x86: pci: Allow binding of some devices before relocation x86: Allow coreboot serial driver to guess the UART spi: ich: Don't require the PCH tpm: cr50: Drop unnecessary coral headers x86: Don't set up MTRRs if previously done x86: Update the MP constants to avoid conflicts x86: Do cache set-up by default when booting from coreboot x86: coreboot: Show the BIOS date x86: coral: Allow booting from coreboot x86: Add function comments to cb_sysinfo.h x86: coreboot: Use vendor in the Kconfig x86: coreboot: Enable the cbsysinfo command x86: coreboot: Document the memory map x86: Check ROM exists before building vboot dtoc: Check that a parent is not missing doc: Update documentation for cros-2021.04 release
arch/x86/cpu/coreboot/Kconfig | 3 +- arch/x86/cpu/i386/cpu.c | 2 +- arch/x86/dts/chromebook_coral.dts | 2 +- arch/x86/dts/chromebook_samus.dts | 2 +- arch/x86/include/asm/cb_sysinfo.h | 16 ++++++ arch/x86/include/asm/mp.h | 12 +++-- arch/x86/lib/init_helpers.c | 5 +- board/coreboot/coreboot/Kconfig | 12 +++-- board/coreboot/coreboot/coreboot.c | 3 ++ board/google/chromebook_coral/coral.c | 28 ++++++++++ doc/board/coreboot/coreboot.rst | 19 +++++++ doc/chromium/run_vboot.rst | 15 +++--- doc/device-tree-bindings/pci/x86-pci.txt | 7 ++- drivers/pci/pci-uclass.c | 39 ++++++++++++-- drivers/serial/serial_coreboot.c | 68 +++++++++++++++++++++--- drivers/spi/ich.c | 4 +- drivers/tpm/cr50_i2c.c | 2 - include/dt-bindings/pci/pci.h | 12 +++++ include/pci.h | 5 +- include/pci_ids.h | 1 + tools/dtoc/dtb_platdata.py | 9 ++++ tools/dtoc/test/dtoc_test_noparent.dts | 32 +++++++++++ tools/dtoc/test_dtoc.py | 10 ++++ 23 files changed, 264 insertions(+), 44 deletions(-) create mode 100644 include/dt-bindings/pci/pci.h create mode 100644 tools/dtoc/test/dtoc_test_noparent.dts
These functions don't modify the device-ID struct that is passed in, so mark the argument as const, so the data structure can be declared that way. This allows it to be placed in the rodata section.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/pci/pci-uclass.c | 6 +++--- include/pci.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index dfd54b339f4..dffe5297944 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -162,7 +162,7 @@ int dm_pci_bus_find_bdf(pci_dev_t bdf, struct udevice **devp) }
static int pci_device_matches_ids(struct udevice *dev, - struct pci_device_id *ids) + const struct pci_device_id *ids) { struct pci_child_plat *pplat; int i; @@ -179,7 +179,7 @@ static int pci_device_matches_ids(struct udevice *dev, return -EINVAL; }
-int pci_bus_find_devices(struct udevice *bus, struct pci_device_id *ids, +int pci_bus_find_devices(struct udevice *bus, const struct pci_device_id *ids, int *indexp, struct udevice **devp) { struct udevice *dev; @@ -199,7 +199,7 @@ int pci_bus_find_devices(struct udevice *bus, struct pci_device_id *ids, return -ENODEV; }
-int pci_find_device_id(struct pci_device_id *ids, int index, +int pci_find_device_id(const struct pci_device_id *ids, int index, struct udevice **devp) { struct udevice *bus; diff --git a/include/pci.h b/include/pci.h index 5f36537b725..f2dfbda5b08 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1070,7 +1070,7 @@ int pci_get_ff(enum pci_size_t size); * @devp: Returns matching device if found * @return 0 if found, -ENODEV if not */ -int pci_bus_find_devices(struct udevice *bus, struct pci_device_id *ids, +int pci_bus_find_devices(struct udevice *bus, const struct pci_device_id *ids, int *indexp, struct udevice **devp);
/** @@ -1082,7 +1082,7 @@ int pci_bus_find_devices(struct udevice *bus, struct pci_device_id *ids, * @devp: Returns matching device if found * @return 0 if found, -ENODEV if not */ -int pci_find_device_id(struct pci_device_id *ids, int index, +int pci_find_device_id(const struct pci_device_id *ids, int index, struct udevice **devp);
/**
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote:
These functions don't modify the device-ID struct that is passed in, so mark the argument as const, so the data structure can be declared that way. This allows it to be placed in the rodata section.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/pci/pci-uclass.c | 6 +++--- include/pci.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
At present only bridge devices are bound before relocation, to save space in pre-relocation memory. In some cases we do actually want to bind a device, e.g. because it provides the console UART. Add a devicetree binding to support this.
Use the PCI_VENDEV() macro to encode the cell value. This is present in U-Boot but not used, so move it to the binding header-file.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/device-tree-bindings/pci/x86-pci.txt | 7 ++++- drivers/pci/pci-uclass.c | 33 +++++++++++++++++++++++- include/dt-bindings/pci/pci.h | 12 +++++++++ include/pci.h | 1 - 4 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/pci/pci.h
diff --git a/doc/device-tree-bindings/pci/x86-pci.txt b/doc/device-tree-bindings/pci/x86-pci.txt index 95e370b3e72..cf4e5ed595a 100644 --- a/doc/device-tree-bindings/pci/x86-pci.txt +++ b/doc/device-tree-bindings/pci/x86-pci.txt @@ -20,6 +20,10 @@ For PCI devices the following optional property is available: output to be lost. This should not generally be used in production code, although it is often harmless.
+- u-boot,pci-pre-reloc : List of vendor/device IDs to bind before relocation, even + if they are not bridges. This is useful if the device is needed (e.g. a + UART). The format is 0xvvvvdddd where d is the device ID and v is the + vendor ID.
Example:
@@ -32,7 +36,8 @@ pci { 0x42000000 0x0 0xb0000000 0xb0000000 0 0x10000000 0x01000000 0x0 0x1000 0x1000 0 0xefff>; u-boot,skip-auto-config-until-reloc; - + u-boot,pci-pre-reloc = < + PCI_VENDEV(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2)>;
serial: serial@18,2 { reg = <0x0200c210 0 0 0 0>; diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index dffe5297944..ec3797746ba 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -19,6 +19,7 @@ #if defined(CONFIG_X86) && defined(CONFIG_HAVE_FSP) #include <asm/fsp/fsp_support.h> #endif +#include <dt-bindings/pci/pci.h> #include <linux/delay.h> #include "pci_internal.h"
@@ -673,6 +674,34 @@ static bool pci_match_one_id(const struct pci_device_id *id, return false; }
+/** + * pci_need_device_pre_reloc() - Check if a device should be bound + * + * This checks a list of vendor/device-ID values indicating devices that should + * be bound before relocation. + * + * @bus: Bus to check + * @vendor: Vendor ID to check + * @device: Device ID to check + * @return true if the vendor/device is in the list, false if not + */ +static bool pci_need_device_pre_reloc(struct udevice *bus, uint vendor, + uint device) +{ + u32 vendev; + int index; + + for (index = 0; + !dev_read_u32_index(bus, "u-boot,pci-pre-reloc", index, + &vendev); + index++) { + if (vendev == PCI_VENDEV(vendor, device)) + return true; + } + + return false; +} + /** * pci_find_and_bind_driver() - Find and bind the right PCI driver * @@ -761,7 +790,9 @@ static int pci_find_and_bind_driver(struct udevice *parent, * precious memory space as on some platforms as that space is pretty * limited (ie: using Cache As RAM). */ - if (!(gd->flags & GD_FLG_RELOC) && !bridge) + if (!(gd->flags & GD_FLG_RELOC) && !bridge && + !pci_need_device_pre_reloc(parent, find_id->vendor, + find_id->device)) return log_msg_ret("notbr", -EPERM);
/* Bind a generic driver so that the device can be used */ diff --git a/include/dt-bindings/pci/pci.h b/include/dt-bindings/pci/pci.h new file mode 100644 index 00000000000..e7290277b90 --- /dev/null +++ b/include/dt-bindings/pci/pci.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * This header provides common constants for PCI bindings. + */ + +#ifndef _DT_BINDINGS_PCI_PCI_H +#define _DT_BINDINGS_PCI_PCI_H + +/* Encode a vendor and device ID into a single cell */ +#define PCI_VENDEV(v, d) (((v) << 16) | (d)) + +#endif /* _DT_BINDINGS_PCI_PCI_H */ diff --git a/include/pci.h b/include/pci.h index f2dfbda5b08..fe7064b2a44 100644 --- a/include/pci.h +++ b/include/pci.h @@ -578,7 +578,6 @@ typedef int pci_dev_t; #define PCI_MASK_BUS(bdf) ((bdf) & 0xffff) #define PCI_ADD_BUS(bus, devfn) (((bus) << 16) | (devfn)) #define PCI_BDF(b, d, f) ((b) << 16 | PCI_DEVFN(d, f)) -#define PCI_VENDEV(v, d) (((v) << 16) | (d)) #define PCI_ANY_ID (~0)
/* Convert from Linux format to U-Boot format */
Hi Simon,
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote:
At present only bridge devices are bound before relocation, to save space in pre-relocation memory. In some cases we do actually want to bind a device, e.g. because it provides the console UART. Add a devicetree binding to support this.
Use the PCI_VENDEV() macro to encode the cell value. This is present in U-Boot but not used, so move it to the binding header-file.
Signed-off-by: Simon Glass sjg@chromium.org
doc/device-tree-bindings/pci/x86-pci.txt | 7 ++++- drivers/pci/pci-uclass.c | 33 +++++++++++++++++++++++- include/dt-bindings/pci/pci.h | 12 +++++++++ include/pci.h | 1 - 4 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/pci/pci.h
diff --git a/doc/device-tree-bindings/pci/x86-pci.txt b/doc/device-tree-bindings/pci/x86-pci.txt index 95e370b3e72..cf4e5ed595a 100644 --- a/doc/device-tree-bindings/pci/x86-pci.txt +++ b/doc/device-tree-bindings/pci/x86-pci.txt @@ -20,6 +20,10 @@ For PCI devices the following optional property is available: output to be lost. This should not generally be used in production code, although it is often harmless.
+- u-boot,pci-pre-reloc : List of vendor/device IDs to bind before relocation, even
if they are not bridges. This is useful if the device is needed (e.g. aUART). The format is 0xvvvvdddd where d is the device ID and v is thevendor ID.
Can we reuse "u-boot,dm-pre-reloc" to do such thing?
The following is an example from arch/x86/dts/crownbay.dts
pciuart0: uart@a,1 { compatible = "pci8086,8811.00", "pci8086,8811", "pciclass,070002", "pciclass,0700", "ns16550"; u-boot,dm-pre-reloc; reg = <0x00025100 0x0 0x0 0x0 0x0 0x01025110 0x0 0x0 0x0 0x0>; reg-shift = <0>; clock-frequency = <1843200>; current-speed = <115200>; };
Regards, Bin
Hi Bin,
On Thu, 8 Apr 2021 at 14:17, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote:
At present only bridge devices are bound before relocation, to save space in pre-relocation memory. In some cases we do actually want to bind a device, e.g. because it provides the console UART. Add a devicetree binding to support this.
Use the PCI_VENDEV() macro to encode the cell value. This is present in U-Boot but not used, so move it to the binding header-file.
Signed-off-by: Simon Glass sjg@chromium.org
doc/device-tree-bindings/pci/x86-pci.txt | 7 ++++- drivers/pci/pci-uclass.c | 33 +++++++++++++++++++++++- include/dt-bindings/pci/pci.h | 12 +++++++++ include/pci.h | 1 - 4 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/pci/pci.h
diff --git a/doc/device-tree-bindings/pci/x86-pci.txt b/doc/device-tree-bindings/pci/x86-pci.txt index 95e370b3e72..cf4e5ed595a 100644 --- a/doc/device-tree-bindings/pci/x86-pci.txt +++ b/doc/device-tree-bindings/pci/x86-pci.txt @@ -20,6 +20,10 @@ For PCI devices the following optional property is available: output to be lost. This should not generally be used in production code, although it is often harmless.
+- u-boot,pci-pre-reloc : List of vendor/device IDs to bind before relocation, even
if they are not bridges. This is useful if the device is needed (e.g. aUART). The format is 0xvvvvdddd where d is the device ID and v is thevendor ID.Can we reuse "u-boot,dm-pre-reloc" to do such thing?
The following is an example from arch/x86/dts/crownbay.dts
pciuart0: uart@a,1 { compatible = "pci8086,8811.00", "pci8086,8811", "pciclass,070002", "pciclass,0700", "ns16550"; u-boot,dm-pre-reloc; reg = <0x00025100 0x0 0x0 0x0 0x0 0x01025110 0x0 0x0 0x0 0x0>; reg-shift = <0>; clock-frequency = <1843200>; current-speed = <115200>; };
Yes but only if we have the correct PCI BDF for it. But the goal of the coreboot build is to be able to run on any hardware. So if we require a devicetree (and coreboot doesn't supply it) then it cannot work. We would need to have many devicetree files, one for each board. Perhaps that will happen anyway, but for now I am trying to do things automatically.
Of course this would be a lot easier if coreboot just used devicetree, but that doesn't seem to be on the cards.
Regards, Simon
At present this driver relies on coreboot to provide information about the console UART. However if coreboot is not compiled with the UART enabled, the information is left out. This configuration is quite common, e.g. with shipping x86-based Chrome OS Chromebooks.
Add a way to determine the UART settings in this case, using a hard-coded list of PCI IDs.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- include/pci_ids.h | 1 + 2 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c index de09c8681f5..4b4619432d8 100644 --- a/drivers/serial/serial_coreboot.c +++ b/drivers/serial/serial_coreboot.c @@ -11,19 +11,71 @@ #include <serial.h> #include <asm/cb_sysinfo.h>
+static const struct pci_device_id ids[] = { + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) }, + {}, +}; + +/* + * Coreboot only sets up the UART if it uses it and doesn't bother to put the + * details in sysinfo if it doesn't. Try to guess in that case, using devices + * we know about + * + * @plat: Platform data to fill in + * @return 0 if found, -ve if no UART was found + */ +static int guess_uart(struct ns16550_plat *plat) +{ + struct udevice *bus, *dev; + ulong addr; + int index; + int ret; + + ret = uclass_first_device_err(UCLASS_PCI, &bus); + if (ret) + return ret; + index = 0; + ret = pci_bus_find_devices(bus, ids, &index, &dev); + if (ret) + return ret; + addr = dm_pci_read_bar32(dev, 0); + plat->base = addr; + plat->reg_shift = 2; + plat->reg_width = 4; + plat->clock = 1843200; + plat->fcr = UART_FCR_DEFVAL; + plat->flags = 0; + + return 0; +} + static int coreboot_of_to_plat(struct udevice *dev) { struct ns16550_plat *plat = dev_get_plat(dev); struct cb_serial *cb_info = lib_sysinfo.serial;
- plat->base = cb_info->baseaddr; - plat->reg_shift = cb_info->regwidth == 4 ? 2 : 0; - plat->reg_width = cb_info->regwidth; - plat->clock = cb_info->input_hertz; - plat->fcr = UART_FCR_DEFVAL; - plat->flags = 0; - if (cb_info->type == CB_SERIAL_TYPE_IO_MAPPED) - plat->flags |= NS16550_FLAG_IO; + if (cb_info) { + plat->base = cb_info->baseaddr; + plat->reg_shift = cb_info->regwidth == 4 ? 2 : 0; + plat->reg_width = cb_info->regwidth; + plat->clock = cb_info->input_hertz; + plat->fcr = UART_FCR_DEFVAL; + plat->flags = 0; + if (cb_info->type == CB_SERIAL_TYPE_IO_MAPPED) + plat->flags |= NS16550_FLAG_IO; + } else if (CONFIG_IS_ENABLED(PCI)) { + int ret; + + ret = guess_uart(plat); + if (ret) { + /* + * Returning an error will cause U-Boot to complain that + * there is no UART, which may panic. So stay silent and + * pray that the video console will work. + */ + log_debug("Cannot detect UART\n"); + } + }
return 0; } diff --git a/include/pci_ids.h b/include/pci_ids.h index 7ecedc7f04c..d91c1d08f1a 100644 --- a/include/pci_ids.h +++ b/include/pci_ids.h @@ -2987,6 +2987,7 @@ #define PCI_DEVICE_ID_INTEL_UNC_R3QPI1 0x3c45 #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX 0x3ce0 #define PCI_DEVICE_ID_INTEL_IOAT_SNB 0x402f +#define PCI_DEVICE_ID_INTEL_APL_UART2 0x5ac0 #define PCI_DEVICE_ID_INTEL_5100_16 0x65f0 #define PCI_DEVICE_ID_INTEL_5100_19 0x65f3 #define PCI_DEVICE_ID_INTEL_5100_21 0x65f5
Hi Simon,
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote:
At present this driver relies on coreboot to provide information about the console UART. However if coreboot is not compiled with the UART enabled, the information is left out. This configuration is quite common, e.g. with shipping x86-based Chrome OS Chromebooks.
Add a way to determine the UART settings in this case, using a hard-coded list of PCI IDs.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- include/pci_ids.h | 1 + 2 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c index de09c8681f5..4b4619432d8 100644 --- a/drivers/serial/serial_coreboot.c +++ b/drivers/serial/serial_coreboot.c @@ -11,19 +11,71 @@ #include <serial.h> #include <asm/cb_sysinfo.h>
+static const struct pci_device_id ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) },{},+};
+/*
- Coreboot only sets up the UART if it uses it and doesn't bother to put the
- details in sysinfo if it doesn't. Try to guess in that case, using devices
- we know about
- @plat: Platform data to fill in
- @return 0 if found, -ve if no UART was found
- */
+static int guess_uart(struct ns16550_plat *plat)
This is really not a guess, but use a pre-configured platform data. Also this only work for Apollo Lake board, and will break other boards if they don't have cbinfo available.
Why not just simply put a serial node in the device tree and we are all done?
+{
struct udevice *bus, *dev;ulong addr;int index;int ret;ret = uclass_first_device_err(UCLASS_PCI, &bus);if (ret)return ret;index = 0;ret = pci_bus_find_devices(bus, ids, &index, &dev);if (ret)return ret;addr = dm_pci_read_bar32(dev, 0);plat->base = addr;plat->reg_shift = 2;plat->reg_width = 4;plat->clock = 1843200;plat->fcr = UART_FCR_DEFVAL;plat->flags = 0;return 0;+}
static int coreboot_of_to_plat(struct udevice *dev) { struct ns16550_plat *plat = dev_get_plat(dev); struct cb_serial *cb_info = lib_sysinfo.serial;
plat->base = cb_info->baseaddr;plat->reg_shift = cb_info->regwidth == 4 ? 2 : 0;plat->reg_width = cb_info->regwidth;plat->clock = cb_info->input_hertz;plat->fcr = UART_FCR_DEFVAL;plat->flags = 0;if (cb_info->type == CB_SERIAL_TYPE_IO_MAPPED)plat->flags |= NS16550_FLAG_IO;
if (cb_info) {plat->base = cb_info->baseaddr;plat->reg_shift = cb_info->regwidth == 4 ? 2 : 0;plat->reg_width = cb_info->regwidth;plat->clock = cb_info->input_hertz;plat->fcr = UART_FCR_DEFVAL;plat->flags = 0;if (cb_info->type == CB_SERIAL_TYPE_IO_MAPPED)plat->flags |= NS16550_FLAG_IO;} else if (CONFIG_IS_ENABLED(PCI)) {int ret;ret = guess_uart(plat);if (ret) {/** Returning an error will cause U-Boot to complain that* there is no UART, which may panic. So stay silent and* pray that the video console will work.*/log_debug("Cannot detect UART\n");}} return 0;} diff --git a/include/pci_ids.h b/include/pci_ids.h index 7ecedc7f04c..d91c1d08f1a 100644 --- a/include/pci_ids.h +++ b/include/pci_ids.h @@ -2987,6 +2987,7 @@ #define PCI_DEVICE_ID_INTEL_UNC_R3QPI1 0x3c45 #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX 0x3ce0 #define PCI_DEVICE_ID_INTEL_IOAT_SNB 0x402f +#define PCI_DEVICE_ID_INTEL_APL_UART2 0x5ac0 #define PCI_DEVICE_ID_INTEL_5100_16 0x65f0 #define PCI_DEVICE_ID_INTEL_5100_19 0x65f3
#define PCI_DEVICE_ID_INTEL_5100_21 0x65f5
Regards, Bin
Hi Bin,
On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote:
At present this driver relies on coreboot to provide information about the console UART. However if coreboot is not compiled with the UART enabled, the information is left out. This configuration is quite common, e.g. with shipping x86-based Chrome OS Chromebooks.
Add a way to determine the UART settings in this case, using a hard-coded list of PCI IDs.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- include/pci_ids.h | 1 + 2 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c index de09c8681f5..4b4619432d8 100644 --- a/drivers/serial/serial_coreboot.c +++ b/drivers/serial/serial_coreboot.c @@ -11,19 +11,71 @@ #include <serial.h> #include <asm/cb_sysinfo.h>
+static const struct pci_device_id ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) },{},+};
+/*
- Coreboot only sets up the UART if it uses it and doesn't bother to put the
- details in sysinfo if it doesn't. Try to guess in that case, using devices
- we know about
- @plat: Platform data to fill in
- @return 0 if found, -ve if no UART was found
- */
+static int guess_uart(struct ns16550_plat *plat)
This is really not a guess, but use a pre-configured platform data. Also this only work for Apollo Lake board, and will break other boards if they don't have cbinfo available.
Which bit of it breaks other boards?
Why not just simply put a serial node in the device tree and we are all done?
See my other email...I am trying to make this boot on any board that coreboot supports.
[..]
Regards, Simon
Hi Simon,
On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote:
At present this driver relies on coreboot to provide information about the console UART. However if coreboot is not compiled with the UART enabled, the information is left out. This configuration is quite common, e.g. with shipping x86-based Chrome OS Chromebooks.
Add a way to determine the UART settings in this case, using a hard-coded list of PCI IDs.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- include/pci_ids.h | 1 + 2 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c index de09c8681f5..4b4619432d8 100644 --- a/drivers/serial/serial_coreboot.c +++ b/drivers/serial/serial_coreboot.c @@ -11,19 +11,71 @@ #include <serial.h> #include <asm/cb_sysinfo.h>
+static const struct pci_device_id ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) },{},+};
+/*
- Coreboot only sets up the UART if it uses it and doesn't bother to put the
- details in sysinfo if it doesn't. Try to guess in that case, using devices
- we know about
- @plat: Platform data to fill in
- @return 0 if found, -ve if no UART was found
- */
+static int guess_uart(struct ns16550_plat *plat)
This is really not a guess, but use a pre-configured platform data. Also this only work for Apollo Lake board, and will break other boards if they don't have cbinfo available.
Which bit of it breaks other boards?
I see it does not return the error code to the caller, that's okay ...
Why not just simply put a serial node in the device tree and we are all done?
See my other email...I am trying to make this boot on any board that coreboot supports.
But this solution does not scale. One has to put all known UARTs into serial_coreboot.c.
Why not patch coreboot instead? Why coreboot does not provide a cbinfo with serial?
Regards, Bin
Hi Bin,
On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote:
At present this driver relies on coreboot to provide information about the console UART. However if coreboot is not compiled with the UART enabled, the information is left out. This configuration is quite common, e.g. with shipping x86-based Chrome OS Chromebooks.
Add a way to determine the UART settings in this case, using a hard-coded list of PCI IDs.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- include/pci_ids.h | 1 + 2 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c index de09c8681f5..4b4619432d8 100644 --- a/drivers/serial/serial_coreboot.c +++ b/drivers/serial/serial_coreboot.c @@ -11,19 +11,71 @@ #include <serial.h> #include <asm/cb_sysinfo.h>
+static const struct pci_device_id ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) },{},+};
+/*
- Coreboot only sets up the UART if it uses it and doesn't bother to put the
- details in sysinfo if it doesn't. Try to guess in that case, using devices
- we know about
- @plat: Platform data to fill in
- @return 0 if found, -ve if no UART was found
- */
+static int guess_uart(struct ns16550_plat *plat)
This is really not a guess, but use a pre-configured platform data. Also this only work for Apollo Lake board, and will break other boards if they don't have cbinfo available.
Which bit of it breaks other boards?
I see it does not return the error code to the caller, that's okay ...
Why not just simply put a serial node in the device tree and we are all done?
See my other email...I am trying to make this boot on any board that coreboot supports.
But this solution does not scale. One has to put all known UARTs into serial_coreboot.c.
Yes that's right...but this is only for when coreboot does not enable serial. Also the number of new platforms is not that great.
Why not patch coreboot instead? Why coreboot does not provide a cbinfo with serial?
Because it does not even set up the serial device in that case, so doesn't know the details. The driver is completely missing.
Regards, Simon
Hi Simon,
On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote:
At present this driver relies on coreboot to provide information about the console UART. However if coreboot is not compiled with the UART enabled, the information is left out. This configuration is quite common, e.g. with shipping x86-based Chrome OS Chromebooks.
Add a way to determine the UART settings in this case, using a hard-coded list of PCI IDs.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- include/pci_ids.h | 1 + 2 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c index de09c8681f5..4b4619432d8 100644 --- a/drivers/serial/serial_coreboot.c +++ b/drivers/serial/serial_coreboot.c @@ -11,19 +11,71 @@ #include <serial.h> #include <asm/cb_sysinfo.h>
+static const struct pci_device_id ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) },{},+};
+/*
- Coreboot only sets up the UART if it uses it and doesn't bother to put the
- details in sysinfo if it doesn't. Try to guess in that case, using devices
- we know about
- @plat: Platform data to fill in
- @return 0 if found, -ve if no UART was found
- */
+static int guess_uart(struct ns16550_plat *plat)
This is really not a guess, but use a pre-configured platform data. Also this only work for Apollo Lake board, and will break other boards if they don't have cbinfo available.
Which bit of it breaks other boards?
I see it does not return the error code to the caller, that's okay ...
Why not just simply put a serial node in the device tree and we are all done?
See my other email...I am trying to make this boot on any board that coreboot supports.
But this solution does not scale. One has to put all known UARTs into serial_coreboot.c.
Yes that's right...but this is only for when coreboot does not enable serial. Also the number of new platforms is not that great.
Why not patch coreboot instead? Why coreboot does not provide a cbinfo with serial?
Because it does not even set up the serial device in that case, so doesn't know the details. The driver is completely missing.
Sigh. Is it possible to upstream a patch to coreboot to enable that?
I don't like the current approach because it ends up duplicating all UART IDs/info in C.
Regards, Bin
Hi Bin,
On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote:
At present this driver relies on coreboot to provide information about the console UART. However if coreboot is not compiled with the UART enabled, the information is left out. This configuration is quite common, e.g. with shipping x86-based Chrome OS Chromebooks.
Add a way to determine the UART settings in this case, using a hard-coded list of PCI IDs.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- include/pci_ids.h | 1 + 2 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c index de09c8681f5..4b4619432d8 100644 --- a/drivers/serial/serial_coreboot.c +++ b/drivers/serial/serial_coreboot.c @@ -11,19 +11,71 @@ #include <serial.h> #include <asm/cb_sysinfo.h>
+static const struct pci_device_id ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) },{},+};
+/*
- Coreboot only sets up the UART if it uses it and doesn't bother to put the
- details in sysinfo if it doesn't. Try to guess in that case, using devices
- we know about
- @plat: Platform data to fill in
- @return 0 if found, -ve if no UART was found
- */
+static int guess_uart(struct ns16550_plat *plat)
This is really not a guess, but use a pre-configured platform data. Also this only work for Apollo Lake board, and will break other boards if they don't have cbinfo available.
Which bit of it breaks other boards?
I see it does not return the error code to the caller, that's okay ...
Why not just simply put a serial node in the device tree and we are all done?
See my other email...I am trying to make this boot on any board that coreboot supports.
But this solution does not scale. One has to put all known UARTs into serial_coreboot.c.
Yes that's right...but this is only for when coreboot does not enable serial. Also the number of new platforms is not that great.
Why not patch coreboot instead? Why coreboot does not provide a cbinfo with serial?
Because it does not even set up the serial device in that case, so doesn't know the details. The driver is completely missing.
Sigh. Is it possible to upstream a patch to coreboot to enable that?
Well I'm not even sure upstream coreboot boots on the various Chromebooks I am targetting. If it does, then serial is probablt enabled. But certainly for chromebooks, it is not. My goal is to have U-Boot boot on a chromebook in altfw mode with serial enabled.
I don't like the current approach because it ends up duplicating all UART IDs/info in C.
Yes. Do you think it would be better to put it in the devicetree? I suppose we could add some more stuff to the compatible string, although U-Boot does not support the PCI compatible strings at present.
What do you suggest?
Regards, Simon
Hi Simon,
On Fri, Apr 30, 2021 at 12:10 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote: > > At present this driver relies on coreboot to provide information about > the console UART. However if coreboot is not compiled with the UART > enabled, the information is left out. This configuration is quite > common, e.g. with shipping x86-based Chrome OS Chromebooks. > > Add a way to determine the UART settings in this case, using a > hard-coded list of PCI IDs. > > Signed-off-by: Simon Glass sjg@chromium.org > --- > > drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- > include/pci_ids.h | 1 + > 2 files changed, 61 insertions(+), 8 deletions(-) > > diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c > index de09c8681f5..4b4619432d8 100644 > --- a/drivers/serial/serial_coreboot.c > +++ b/drivers/serial/serial_coreboot.c > @@ -11,19 +11,71 @@ > #include <serial.h> > #include <asm/cb_sysinfo.h> > > +static const struct pci_device_id ids[] = { > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) }, > + {}, > +}; > + > +/* > + * Coreboot only sets up the UART if it uses it and doesn't bother to put the > + * details in sysinfo if it doesn't. Try to guess in that case, using devices > + * we know about > + * > + * @plat: Platform data to fill in > + * @return 0 if found, -ve if no UART was found > + */ > +static int guess_uart(struct ns16550_plat *plat)
This is really not a guess, but use a pre-configured platform data. Also this only work for Apollo Lake board, and will break other boards if they don't have cbinfo available.
Which bit of it breaks other boards?
I see it does not return the error code to the caller, that's okay ...
Why not just simply put a serial node in the device tree and we are all done?
See my other email...I am trying to make this boot on any board that coreboot supports.
But this solution does not scale. One has to put all known UARTs into serial_coreboot.c.
Yes that's right...but this is only for when coreboot does not enable serial. Also the number of new platforms is not that great.
Why not patch coreboot instead? Why coreboot does not provide a cbinfo with serial?
Because it does not even set up the serial device in that case, so doesn't know the details. The driver is completely missing.
Sigh. Is it possible to upstream a patch to coreboot to enable that?
Well I'm not even sure upstream coreboot boots on the various Chromebooks I am targetting. If it does, then serial is probablt enabled. But certainly for chromebooks, it is not. My goal is to have U-Boot boot on a chromebook in altfw mode with serial enabled.
I don't like the current approach because it ends up duplicating all UART IDs/info in C.
Yes. Do you think it would be better to put it in the devicetree? I suppose we could add some more stuff to the compatible string, although U-Boot does not support the PCI compatible strings at present.
Putting it in the device tree also looks odd, because it only matters on a dedicated board.
What do you suggest?
Or parse the ACPI table coreboot has set up? But that might be another huge monster :(
Regards, Bin
Hi Bin,
On Thu, 29 Apr 2021 at 17:01, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Apr 30, 2021 at 12:10 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote: > > Hi Simon, > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote: > > > > At present this driver relies on coreboot to provide information about > > the console UART. However if coreboot is not compiled with the UART > > enabled, the information is left out. This configuration is quite > > common, e.g. with shipping x86-based Chrome OS Chromebooks. > > > > Add a way to determine the UART settings in this case, using a > > hard-coded list of PCI IDs. > > > > Signed-off-by: Simon Glass sjg@chromium.org > > --- > > > > drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- > > include/pci_ids.h | 1 + > > 2 files changed, 61 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c > > index de09c8681f5..4b4619432d8 100644 > > --- a/drivers/serial/serial_coreboot.c > > +++ b/drivers/serial/serial_coreboot.c > > @@ -11,19 +11,71 @@ > > #include <serial.h> > > #include <asm/cb_sysinfo.h> > > > > +static const struct pci_device_id ids[] = { > > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) }, > > + {}, > > +}; > > + > > +/* > > + * Coreboot only sets up the UART if it uses it and doesn't bother to put the > > + * details in sysinfo if it doesn't. Try to guess in that case, using devices > > + * we know about > > + * > > + * @plat: Platform data to fill in > > + * @return 0 if found, -ve if no UART was found > > + */ > > +static int guess_uart(struct ns16550_plat *plat) > > This is really not a guess, but use a pre-configured platform data. > Also this only work for Apollo Lake board, and will break other boards > if they don't have cbinfo available.
Which bit of it breaks other boards?
I see it does not return the error code to the caller, that's okay ...
> > Why not just simply put a serial node in the device tree and we are all done?
See my other email...I am trying to make this boot on any board that coreboot supports.
But this solution does not scale. One has to put all known UARTs into serial_coreboot.c.
Yes that's right...but this is only for when coreboot does not enable serial. Also the number of new platforms is not that great.
Why not patch coreboot instead? Why coreboot does not provide a cbinfo with serial?
Because it does not even set up the serial device in that case, so doesn't know the details. The driver is completely missing.
Sigh. Is it possible to upstream a patch to coreboot to enable that?
Well I'm not even sure upstream coreboot boots on the various Chromebooks I am targetting. If it does, then serial is probablt enabled. But certainly for chromebooks, it is not. My goal is to have U-Boot boot on a chromebook in altfw mode with serial enabled.
I don't like the current approach because it ends up duplicating all UART IDs/info in C.
Yes. Do you think it would be better to put it in the devicetree? I suppose we could add some more stuff to the compatible string, although U-Boot does not support the PCI compatible strings at present.
Putting it in the device tree also looks odd, because it only matters on a dedicated board.
Right, but that is the nature of trying to run the same image on different hardware.
What do you suggest?
Or parse the ACPI table coreboot has set up? But that might be another huge monster :(
Yes, even worse...
Regards, Simon
On Fri, Apr 30, 2021 at 9:14 PM Simon Glass sjg@chromium.org wrote:
On Thu, 29 Apr 2021 at 17:01, Bin Meng bmeng.cn@gmail.com wrote:
On Fri, Apr 30, 2021 at 12:10 AM Simon Glass sjg@chromium.org wrote:
On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote:
On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote:
On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote:
On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote: > On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote: > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote: > > > > > > At present this driver relies on coreboot to provide information about > > > the console UART. However if coreboot is not compiled with the UART > > > enabled, the information is left out. This configuration is quite > > > common, e.g. with shipping x86-based Chrome OS Chromebooks. > > > > > > Add a way to determine the UART settings in this case, using a > > > hard-coded list of PCI IDs.
I don't like this either, so -1 from me.
What coreboot should do is either provide serial information or SPCR ACPI table. Otherwise if it does not provide it, I think it's on purpose, and we have to respect this.
The patch is ugly hack in my opinion. Sorry.
You may make it less ugly by checking PCI class rather than IDs.
> > Why not just simply put a serial node in the device tree and we are all done? > > See my other email...I am trying to make this boot on any board that > coreboot supports.
But this solution does not scale. One has to put all known UARTs into serial_coreboot.c.
Yes that's right...but this is only for when coreboot does not enable serial. Also the number of new platforms is not that great.
Why not patch coreboot instead? Why coreboot does not provide a cbinfo with serial?
Because it does not even set up the serial device in that case, so doesn't know the details. The driver is completely missing.
Sigh. Is it possible to upstream a patch to coreboot to enable that?
Well I'm not even sure upstream coreboot boots on the various Chromebooks I am targetting. If it does, then serial is probablt enabled. But certainly for chromebooks, it is not. My goal is to have U-Boot boot on a chromebook in altfw mode with serial enabled.
I don't like the current approach because it ends up duplicating all UART IDs/info in C.
Yes. Do you think it would be better to put it in the devicetree? I suppose we could add some more stuff to the compatible string, although U-Boot does not support the PCI compatible strings at present.
Putting it in the device tree also looks odd, because it only matters on a dedicated board.
Right, but that is the nature of trying to run the same image on different hardware.
What do you suggest?
Or parse the ACPI table coreboot has set up? But that might be another huge monster :(
Yes, even worse...
Hi Bin and Andy,
On Fri, 30 Apr 2021 at 12:41, Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Apr 30, 2021 at 9:14 PM Simon Glass sjg@chromium.org wrote:
On Thu, 29 Apr 2021 at 17:01, Bin Meng bmeng.cn@gmail.com wrote:
On Fri, Apr 30, 2021 at 12:10 AM Simon Glass sjg@chromium.org wrote:
On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote:
On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote:
On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote: > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote: > > On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote: > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote: > > > > > > > > At present this driver relies on coreboot to provide information about > > > > the console UART. However if coreboot is not compiled with the UART > > > > enabled, the information is left out. This configuration is quite > > > > common, e.g. with shipping x86-based Chrome OS Chromebooks. > > > > > > > > Add a way to determine the UART settings in this case, using a > > > > hard-coded list of PCI IDs.
I don't like this either, so -1 from me.
What coreboot should do is either provide serial information or SPCR ACPI table. Otherwise if it does not provide it, I think it's on purpose, and we have to respect this.
The patch is ugly hack in my opinion. Sorry.
You may make it less ugly by checking PCI class rather than IDs.
I have not been able to distinguish a pattern on Intel SoCs yet. Perhaps you can help with that as there must be a way...
> > > Why not just simply put a serial node in the device tree and we are all done? > > > > See my other email...I am trying to make this boot on any board that > > coreboot supports. > > But this solution does not scale. One has to put all known UARTs into > serial_coreboot.c.
Yes that's right...but this is only for when coreboot does not enable serial. Also the number of new platforms is not that great.
> > Why not patch coreboot instead? Why coreboot does not provide a cbinfo > with serial?
Because it does not even set up the serial device in that case, so doesn't know the details. The driver is completely missing.
Sigh. Is it possible to upstream a patch to coreboot to enable that?
Well I'm not even sure upstream coreboot boots on the various Chromebooks I am targetting. If it does, then serial is probablt enabled. But certainly for chromebooks, it is not. My goal is to have U-Boot boot on a chromebook in altfw mode with serial enabled.
I don't like the current approach because it ends up duplicating all UART IDs/info in C.
Yes. Do you think it would be better to put it in the devicetree? I suppose we could add some more stuff to the compatible string, although U-Boot does not support the PCI compatible strings at present.
Putting it in the device tree also looks odd, because it only matters on a dedicated board.
Right, but that is the nature of trying to run the same image on different hardware.
What do you suggest?
Or parse the ACPI table coreboot has set up? But that might be another huge monster :(
Yes, even worse...
All of the suggestions so far do not solve the problem, so we are left without serial output, or something else...?
Regards, Simon
On Tue, May 04, 2021 at 09:26:19AM -0600, Simon Glass wrote:
On Fri, 30 Apr 2021 at 12:41, Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Apr 30, 2021 at 9:14 PM Simon Glass sjg@chromium.org wrote:
On Thu, 29 Apr 2021 at 17:01, Bin Meng bmeng.cn@gmail.com wrote:
On Fri, Apr 30, 2021 at 12:10 AM Simon Glass sjg@chromium.org wrote:
On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote:
On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote: > On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote: > > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote: > > > On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote: > > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote: > > > > > > > > > > At present this driver relies on coreboot to provide information about > > > > > the console UART. However if coreboot is not compiled with the UART > > > > > enabled, the information is left out. This configuration is quite > > > > > common, e.g. with shipping x86-based Chrome OS Chromebooks. > > > > > > > > > > Add a way to determine the UART settings in this case, using a > > > > > hard-coded list of PCI IDs.
I don't like this either, so -1 from me.
What coreboot should do is either provide serial information or SPCR ACPI table. Otherwise if it does not provide it, I think it's on purpose, and we have to respect this.
The patch is ugly hack in my opinion. Sorry.
You may make it less ugly by checking PCI class rather than IDs.
I have not been able to distinguish a pattern on Intel SoCs yet. Perhaps you can help with that as there must be a way...
> > > > Why not just simply put a serial node in the device tree and we are all done? > > > > > > See my other email...I am trying to make this boot on any board that > > > coreboot supports. > > > > But this solution does not scale. One has to put all known UARTs into > > serial_coreboot.c. > > Yes that's right...but this is only for when coreboot does not enable > serial. Also the number of new platforms is not that great. > > > > > Why not patch coreboot instead? Why coreboot does not provide a cbinfo > > with serial? > > Because it does not even set up the serial device in that case, so > doesn't know the details. The driver is completely missing.
Sigh. Is it possible to upstream a patch to coreboot to enable that?
Well I'm not even sure upstream coreboot boots on the various Chromebooks I am targetting. If it does, then serial is probablt enabled. But certainly for chromebooks, it is not. My goal is to have U-Boot boot on a chromebook in altfw mode with serial enabled.
I don't like the current approach because it ends up duplicating all UART IDs/info in C.
Yes. Do you think it would be better to put it in the devicetree? I suppose we could add some more stuff to the compatible string, although U-Boot does not support the PCI compatible strings at present.
Putting it in the device tree also looks odd, because it only matters on a dedicated board.
Right, but that is the nature of trying to run the same image on different hardware.
What do you suggest?
Or parse the ACPI table coreboot has set up? But that might be another huge monster :(
Yes, even worse...
All of the suggestions so far do not solve the problem, so we are left without serial output, or something else...?
You are truing to solve Coreboot issue in U-Boot, doesn't seem to be quite right.
Yes, I prefer no serial than some "smart" hack. It will encourage people who want to have serial out of Coreboot to whine to coreboot project and see what they will do about it.
Hi Bin,
On Tue, 4 May 2021 at 08:26, Simon Glass sjg@chromium.org wrote:
Hi Bin and Andy,
On Fri, 30 Apr 2021 at 12:41, Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Apr 30, 2021 at 9:14 PM Simon Glass sjg@chromium.org wrote:
On Thu, 29 Apr 2021 at 17:01, Bin Meng bmeng.cn@gmail.com wrote:
On Fri, Apr 30, 2021 at 12:10 AM Simon Glass sjg@chromium.org wrote:
On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote:
On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote: > On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote: > > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote: > > > On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote: > > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote: > > > > > > > > > > At present this driver relies on coreboot to provide information about > > > > > the console UART. However if coreboot is not compiled with the UART > > > > > enabled, the information is left out. This configuration is quite > > > > > common, e.g. with shipping x86-based Chrome OS Chromebooks. > > > > > > > > > > Add a way to determine the UART settings in this case, using a > > > > > hard-coded list of PCI IDs.
I don't like this either, so -1 from me.
What coreboot should do is either provide serial information or SPCR ACPI table. Otherwise if it does not provide it, I think it's on purpose, and we have to respect this.
The patch is ugly hack in my opinion. Sorry.
You may make it less ugly by checking PCI class rather than IDs.
I have not been able to distinguish a pattern on Intel SoCs yet. Perhaps you can help with that as there must be a way...
> > > > Why not just simply put a serial node in the device tree and we are all done? > > > > > > See my other email...I am trying to make this boot on any board that > > > coreboot supports. > > > > But this solution does not scale. One has to put all known UARTs into > > serial_coreboot.c. > > Yes that's right...but this is only for when coreboot does not enable > serial. Also the number of new platforms is not that great. > > > > > Why not patch coreboot instead? Why coreboot does not provide a cbinfo > > with serial? > > Because it does not even set up the serial device in that case, so > doesn't know the details. The driver is completely missing.
Sigh. Is it possible to upstream a patch to coreboot to enable that?
Well I'm not even sure upstream coreboot boots on the various Chromebooks I am targetting. If it does, then serial is probablt enabled. But certainly for chromebooks, it is not. My goal is to have U-Boot boot on a chromebook in altfw mode with serial enabled.
I don't like the current approach because it ends up duplicating all UART IDs/info in C.
Yes. Do you think it would be better to put it in the devicetree? I suppose we could add some more stuff to the compatible string, although U-Boot does not support the PCI compatible strings at present.
Putting it in the device tree also looks odd, because it only matters on a dedicated board.
Right, but that is the nature of trying to run the same image on different hardware.
What do you suggest?
Or parse the ACPI table coreboot has set up? But that might be another huge monster :(
Yes, even worse...
All of the suggestions so far do not solve the problem, so we are left without serial output, or something else...?
In the interested of getting the patches applied, shall we just drop this one? I will revisit it when I try the next platform and see if I can find a different pattern.
Regards, Simon
Hi Simon,
On Sat, May 8, 2021 at 9:42 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Tue, 4 May 2021 at 08:26, Simon Glass sjg@chromium.org wrote:
Hi Bin and Andy,
On Fri, 30 Apr 2021 at 12:41, Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Apr 30, 2021 at 9:14 PM Simon Glass sjg@chromium.org wrote:
On Thu, 29 Apr 2021 at 17:01, Bin Meng bmeng.cn@gmail.com wrote:
On Fri, Apr 30, 2021 at 12:10 AM Simon Glass sjg@chromium.org wrote:
On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote: > On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote: > > On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote: > > > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote: > > > > On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote: > > > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote: > > > > > > > > > > > > At present this driver relies on coreboot to provide information about > > > > > > the console UART. However if coreboot is not compiled with the UART > > > > > > enabled, the information is left out. This configuration is quite > > > > > > common, e.g. with shipping x86-based Chrome OS Chromebooks. > > > > > > > > > > > > Add a way to determine the UART settings in this case, using a > > > > > > hard-coded list of PCI IDs.
I don't like this either, so -1 from me.
What coreboot should do is either provide serial information or SPCR ACPI table. Otherwise if it does not provide it, I think it's on purpose, and we have to respect this.
The patch is ugly hack in my opinion. Sorry.
You may make it less ugly by checking PCI class rather than IDs.
I have not been able to distinguish a pattern on Intel SoCs yet. Perhaps you can help with that as there must be a way...
> > > > > Why not just simply put a serial node in the device tree and we are all done? > > > > > > > > See my other email...I am trying to make this boot on any board that > > > > coreboot supports. > > > > > > But this solution does not scale. One has to put all known UARTs into > > > serial_coreboot.c. > > > > Yes that's right...but this is only for when coreboot does not enable > > serial. Also the number of new platforms is not that great. > > > > > > > > Why not patch coreboot instead? Why coreboot does not provide a cbinfo > > > with serial? > > > > Because it does not even set up the serial device in that case, so > > doesn't know the details. The driver is completely missing. > > Sigh. Is it possible to upstream a patch to coreboot to enable that?
Well I'm not even sure upstream coreboot boots on the various Chromebooks I am targetting. If it does, then serial is probablt enabled. But certainly for chromebooks, it is not. My goal is to have U-Boot boot on a chromebook in altfw mode with serial enabled.
> > I don't like the current approach because it ends up duplicating all > UART IDs/info in C.
Yes. Do you think it would be better to put it in the devicetree? I suppose we could add some more stuff to the compatible string, although U-Boot does not support the PCI compatible strings at present.
Putting it in the device tree also looks odd, because it only matters on a dedicated board.
Right, but that is the nature of trying to run the same image on different hardware.
What do you suggest?
Or parse the ACPI table coreboot has set up? But that might be another huge monster :(
Yes, even worse...
All of the suggestions so far do not solve the problem, so we are left without serial output, or something else...?
In the interested of getting the patches applied, shall we just drop this one? I will revisit it when I try the next platform and see if I can find a different pattern.
I think we should drop this patch for now, and apply other patches in this series. But I suspect the rest patches cannot be applied cleanly if dropping this patch.
Regards, Bin
Hi Bin,
On Fri, 7 May 2021 at 18:48, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, May 8, 2021 at 9:42 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Tue, 4 May 2021 at 08:26, Simon Glass sjg@chromium.org wrote:
Hi Bin and Andy,
On Fri, 30 Apr 2021 at 12:41, Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Apr 30, 2021 at 9:14 PM Simon Glass sjg@chromium.org wrote:
On Thu, 29 Apr 2021 at 17:01, Bin Meng bmeng.cn@gmail.com wrote:
On Fri, Apr 30, 2021 at 12:10 AM Simon Glass sjg@chromium.org wrote: > On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote: > > On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote: > > > On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote: > > > > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote: > > > > > On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote: > > > > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote: > > > > > > > > > > > > > > At present this driver relies on coreboot to provide information about > > > > > > > the console UART. However if coreboot is not compiled with the UART > > > > > > > enabled, the information is left out. This configuration is quite > > > > > > > common, e.g. with shipping x86-based Chrome OS Chromebooks. > > > > > > > > > > > > > > Add a way to determine the UART settings in this case, using a > > > > > > > hard-coded list of PCI IDs.
I don't like this either, so -1 from me.
What coreboot should do is either provide serial information or SPCR ACPI table. Otherwise if it does not provide it, I think it's on purpose, and we have to respect this.
The patch is ugly hack in my opinion. Sorry.
You may make it less ugly by checking PCI class rather than IDs.
I have not been able to distinguish a pattern on Intel SoCs yet. Perhaps you can help with that as there must be a way...
> > > > > > Why not just simply put a serial node in the device tree and we are all done? > > > > > > > > > > See my other email...I am trying to make this boot on any board that > > > > > coreboot supports. > > > > > > > > But this solution does not scale. One has to put all known UARTs into > > > > serial_coreboot.c. > > > > > > Yes that's right...but this is only for when coreboot does not enable > > > serial. Also the number of new platforms is not that great. > > > > > > > > > > > Why not patch coreboot instead? Why coreboot does not provide a cbinfo > > > > with serial? > > > > > > Because it does not even set up the serial device in that case, so > > > doesn't know the details. The driver is completely missing. > > > > Sigh. Is it possible to upstream a patch to coreboot to enable that? > > Well I'm not even sure upstream coreboot boots on the various > Chromebooks I am targetting. If it does, then serial is probablt > enabled. But certainly for chromebooks, it is not. My goal is to have > U-Boot boot on a chromebook in altfw mode with serial enabled. > > > > > I don't like the current approach because it ends up duplicating all > > UART IDs/info in C. > > Yes. Do you think it would be better to put it in the devicetree? I > suppose we could add some more stuff to the compatible string, > although U-Boot does not support the PCI compatible strings at > present.
Putting it in the device tree also looks odd, because it only matters on a dedicated board.
Right, but that is the nature of trying to run the same image on different hardware.
> What do you suggest?
Or parse the ACPI table coreboot has set up? But that might be another huge monster :(
Yes, even worse...
All of the suggestions so far do not solve the problem, so we are left without serial output, or something else...?
In the interested of getting the patches applied, shall we just drop this one? I will revisit it when I try the next platform and see if I can find a different pattern.
I think we should drop this patch for now, and apply other patches in this series. But I suspect the rest patches cannot be applied cleanly if dropping this patch.
That's OK with me. I did a little test and it applies cleanly without this patch. If you have trouble I can resend it.
Regards, Simon
Hi Simon,
On Sat, May 8, 2021 at 10:12 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Fri, 7 May 2021 at 18:48, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, May 8, 2021 at 9:42 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Tue, 4 May 2021 at 08:26, Simon Glass sjg@chromium.org wrote:
Hi Bin and Andy,
On Fri, 30 Apr 2021 at 12:41, Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Apr 30, 2021 at 9:14 PM Simon Glass sjg@chromium.org wrote:
On Thu, 29 Apr 2021 at 17:01, Bin Meng bmeng.cn@gmail.com wrote: > On Fri, Apr 30, 2021 at 12:10 AM Simon Glass sjg@chromium.org wrote: > > On Sun, 25 Apr 2021 at 18:21, Bin Meng bmeng.cn@gmail.com wrote: > > > On Sun, Apr 25, 2021 at 10:10 AM Simon Glass sjg@chromium.org wrote: > > > > On Sun, 25 Apr 2021 at 13:49, Bin Meng bmeng.cn@gmail.com wrote: > > > > > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass sjg@chromium.org wrote: > > > > > > On Thu, 8 Apr 2021 at 14:23, Bin Meng bmeng.cn@gmail.com wrote: > > > > > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass sjg@chromium.org wrote: > > > > > > > > > > > > > > > > At present this driver relies on coreboot to provide information about > > > > > > > > the console UART. However if coreboot is not compiled with the UART > > > > > > > > enabled, the information is left out. This configuration is quite > > > > > > > > common, e.g. with shipping x86-based Chrome OS Chromebooks. > > > > > > > > > > > > > > > > Add a way to determine the UART settings in this case, using a > > > > > > > > hard-coded list of PCI IDs.
I don't like this either, so -1 from me.
What coreboot should do is either provide serial information or SPCR ACPI table. Otherwise if it does not provide it, I think it's on purpose, and we have to respect this.
The patch is ugly hack in my opinion. Sorry.
You may make it less ugly by checking PCI class rather than IDs.
I have not been able to distinguish a pattern on Intel SoCs yet. Perhaps you can help with that as there must be a way...
> > > > > > > Why not just simply put a serial node in the device tree and we are all done? > > > > > > > > > > > > See my other email...I am trying to make this boot on any board that > > > > > > coreboot supports. > > > > > > > > > > But this solution does not scale. One has to put all known UARTs into > > > > > serial_coreboot.c. > > > > > > > > Yes that's right...but this is only for when coreboot does not enable > > > > serial. Also the number of new platforms is not that great. > > > > > > > > > > > > > > Why not patch coreboot instead? Why coreboot does not provide a cbinfo > > > > > with serial? > > > > > > > > Because it does not even set up the serial device in that case, so > > > > doesn't know the details. The driver is completely missing. > > > > > > Sigh. Is it possible to upstream a patch to coreboot to enable that? > > > > Well I'm not even sure upstream coreboot boots on the various > > Chromebooks I am targetting. If it does, then serial is probablt > > enabled. But certainly for chromebooks, it is not. My goal is to have > > U-Boot boot on a chromebook in altfw mode with serial enabled. > > > > > > > > I don't like the current approach because it ends up duplicating all > > > UART IDs/info in C. > > > > Yes. Do you think it would be better to put it in the devicetree? I > > suppose we could add some more stuff to the compatible string, > > although U-Boot does not support the PCI compatible strings at > > present. > > Putting it in the device tree also looks odd, because it only matters > on a dedicated board.
Right, but that is the nature of trying to run the same image on different hardware.
> > > What do you suggest? > > Or parse the ACPI table coreboot has set up? But that might be another > huge monster :(
Yes, even worse...
All of the suggestions so far do not solve the problem, so we are left without serial output, or something else...?
In the interested of getting the patches applied, shall we just drop this one? I will revisit it when I try the next platform and see if I can find a different pattern.
I think we should drop this patch for now, and apply other patches in this series. But I suspect the rest patches cannot be applied cleanly if dropping this patch.
That's OK with me. I did a little test and it applies cleanly without this patch. If you have trouble I can resend it.
Sure, will do soon.
Regards, Bin
When booting from coreboot we may not have a PCH driver available. The SPI driver can operate without the PCH but currently complains in this case. Update it to continue to work normally. The only missing feature is memory-mapping of SPI-flash contents, which is not essential.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/spi/ich.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c index 1cd410493b0..3d49c22a9da 100644 --- a/drivers/spi/ich.c +++ b/drivers/spi/ich.c @@ -114,7 +114,7 @@ static bool ich9_can_do_33mhz(struct udevice *dev) struct ich_spi_priv *priv = dev_get_priv(dev); u32 fdod, speed;
- if (!CONFIG_IS_ENABLED(PCI)) + if (!CONFIG_IS_ENABLED(PCI) || !priv->pch) return false; /* Observe SPI Descriptor Component Section 0 */ dm_pci_write_config32(priv->pch, 0xb0, 0x1000); @@ -632,7 +632,7 @@ static int ich_spi_get_basics(struct udevice *bus, bool can_probe, if (device_get_uclass_id(pch) != UCLASS_PCH) { uclass_first_device(UCLASS_PCH, &pch); if (!pch) - return log_msg_ret("uclass", -EPROTOTYPE); + ; /* ignore this error since we don't need it */ } }
On Wed, Apr 7, 2021 at 12:34 PM Simon Glass sjg@chromium.org wrote:
When booting from coreboot we may not have a PCH driver available. The SPI driver can operate without the PCH but currently complains in this case. Update it to continue to work normally. The only missing feature is memory-mapping of SPI-flash contents, which is not essential.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/spi/ich.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
But I feel this driver is slightly becoming unmaintainable as there are many combinations we need to consider ...
Regards, Bin
Hi Bin,
On Thu, 8 Apr 2021 at 14:28, Bin Meng bmeng.cn@gmail.com wrote:
On Wed, Apr 7, 2021 at 12:34 PM Simon Glass sjg@chromium.org wrote:
When booting from coreboot we may not have a PCH driver available. The SPI driver can operate without the PCH but currently complains in this case. Update it to continue to work normally. The only missing feature is memory-mapping of SPI-flash contents, which is not essential.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/spi/ich.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
But I feel this driver is slightly becoming unmaintainable as there are many combinations we need to consider ...
Yes it is a bit of a pain. The main one is that in some cases the SPI bus appears on PCI and in others it is part of the PCH or LPC. This is a hardware thing though, so I'm not sure what we can do about it. Perhaps split up the driver?
Regards, Simon
These headers are not actually used. Drop them so that this driver can be used by other boards, e.g. coreboot.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/cr50_i2c.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/tpm/cr50_i2c.c b/drivers/tpm/cr50_i2c.c index 76432bdec1f..7a2b5a4faa5 100644 --- a/drivers/tpm/cr50_i2c.c +++ b/drivers/tpm/cr50_i2c.c @@ -18,8 +18,6 @@ #include <acpi/acpi_device.h> #include <asm/gpio.h> #include <asm/io.h> -#include <asm/arch/iomap.h> -#include <asm/arch/pm.h> #include <linux/delay.h> #include <dm/acpi.h>
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
These headers are not actually used. Drop them so that this driver can be used by other boards, e.g. coreboot.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/tpm/cr50_i2c.c | 2 -- 1 file changed, 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
When starting U-Boot from a previous-stage bootloader we presumably don't need to set up the variable MTRRs. In fact this could be harmful if the existing settings are not what U-Boot uses.
Skip that step in this case.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/i386/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index e59215cc20e..c7f6c5a013e 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -423,7 +423,7 @@ static void setup_mtrr(void) u64 mtrr_cap;
/* Configure fixed range MTRRs for some legacy regions */ - if (!gd->arch.has_mtrr) + if (!gd->arch.has_mtrr || !ll_boot_init()) return;
mtrr_cap = native_read_msr(MTRR_CAP_MSR);
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
When starting U-Boot from a previous-stage bootloader we presumably don't need to set up the variable MTRRs. In fact this could be harmful if the existing settings are not what U-Boot uses.
Skip that step in this case.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/i386/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
These constants conflict with error codes returned by the MP implementation when something is wrong. In particular, mp_first_cpu() returns MP_SELECT_BSP when running without multiprocessing enabled. Since this is -2, it is interpreted as an error by callers, which expect a positive CPU number for the first CPU.
Correct this by using a different range for the pre-defined CPU numbers, above zero and out of the range of possible CPU values. For now it is safe to assume there are no more than 64K CPUs.
This fixes the 'mtrr' command when CONFIG_SMP is not enabled.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/include/asm/mp.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h index 1a3ae8e3950..e48ba051d92 100644 --- a/arch/x86/include/asm/mp.h +++ b/arch/x86/include/asm/mp.h @@ -10,18 +10,22 @@
#include <asm/atomic.h> #include <asm/cache.h> +#include <linux/bitops.h>
struct udevice;
enum { - /* Indicates that the function should run on all CPUs */ - MP_SELECT_ALL = -1, + /* + * Indicates that the function should run on all CPUs. We use a large + * number, above the number of real CPUs we expect to find. + */ + MP_SELECT_ALL = BIT(16),
/* Run on boot CPUs */ - MP_SELECT_BSP = -2, + MP_SELECT_BSP,
/* Run on non-boot CPUs */ - MP_SELECT_APS = -3, + MP_SELECT_APS, };
typedef int (*mp_callback_t)(struct udevice *cpu, void *arg);
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
These constants conflict with error codes returned by the MP implementation when something is wrong. In particular, mp_first_cpu() returns MP_SELECT_BSP when running without multiprocessing enabled. Since this is -2, it is interpreted as an error by callers, which expect a positive CPU number for the first CPU.
Correct this by using a different range for the pre-defined CPU numbers, above zero and out of the range of possible CPU values. For now it is safe to assume there are no more than 64K CPUs.
This fixes the 'mtrr' command when CONFIG_SMP is not enabled.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/mp.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
A recent change to disable cache setup when booting from coreboot assumed that this has been done by SPL. The result is that for the coreboot board, the cache is disabled (in start.S) and never re-enabled.
If the cache was turned off, as it is on boards without SPL, we should turn it back on. Add this new condition.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/lib/init_helpers.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/x86/lib/init_helpers.c b/arch/x86/lib/init_helpers.c index 67401b9ba79..0c55544a670 100644 --- a/arch/x86/lib/init_helpers.c +++ b/arch/x86/lib/init_helpers.c @@ -18,10 +18,7 @@ int init_cache_f_r(void) IS_ENABLED(CONFIG_FSP_VERSION2); int ret;
- if (!ll_boot_init()) - return 0; - - do_mtrr &= !IS_ENABLED(CONFIG_FSP_VERSION1) && + do_mtrr &= !IS_ENABLED(CONFIG_SPL) && !IS_ENABLED(CONFIG_FSP_VERSION1) && !IS_ENABLED(CONFIG_SYS_SLIMBOOTLOADER);
if (do_mtrr) {
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
A recent change to disable cache setup when booting from coreboot assumed that this has been done by SPL. The result is that for the coreboot board, the cache is disabled (in start.S) and never re-enabled.
If the cache was turned off, as it is on boards without SPL, we should turn it back on. Add this new condition.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/lib/init_helpers.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/x86/lib/init_helpers.c b/arch/x86/lib/init_helpers.c index 67401b9ba79..0c55544a670 100644 --- a/arch/x86/lib/init_helpers.c +++ b/arch/x86/lib/init_helpers.c @@ -18,10 +18,7 @@ int init_cache_f_r(void) IS_ENABLED(CONFIG_FSP_VERSION2); int ret;
if (!ll_boot_init())return 0;do_mtrr &= !IS_ENABLED(CONFIG_FSP_VERSION1) &&
Can we add a comment block here for every configuration we support?
do_mtrr &= !IS_ENABLED(CONFIG_SPL) && !IS_ENABLED(CONFIG_FSP_VERSION1) && !IS_ENABLED(CONFIG_SYS_SLIMBOOTLOADER); if (do_mtrr) {--
Otherwise, Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
The BIOS version may not be present, e.g. on a Chrome OS build. Add the BIOS date as well, so we get some sort of indication of coreboot's vintage.
Signed-off-by: Simon Glass sjg@chromium.org ---
board/coreboot/coreboot/coreboot.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/board/coreboot/coreboot/coreboot.c b/board/coreboot/coreboot/coreboot.c index 175d3ce691a..93f897893eb 100644 --- a/board/coreboot/coreboot/coreboot.c +++ b/board/coreboot/coreboot/coreboot.c @@ -39,6 +39,7 @@ int show_board_info(void) const char *bios_ver = smbios_string(bios, t0->bios_ver); const char *model = smbios_string(system, t1->product_name); const char *manufacturer = smbios_string(system, t1->manufacturer); + const char *date = smbios_string(bios, t0->bios_release_date);
if (!model || !manufacturer || !bios_ver) goto fallback; @@ -46,6 +47,8 @@ int show_board_info(void) printf("Vendor: %s\n", manufacturer); printf("Model: %s\n", model); printf("BIOS Version: %s\n", bios_ver); + if (date) + printf("BIOS date: %s\n", date);
return 0;
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
The BIOS version may not be present, e.g. on a Chrome OS build. Add the BIOS date as well, so we get some sort of indication of coreboot's vintage.
Signed-off-by: Simon Glass sjg@chromium.org
board/coreboot/coreboot/coreboot.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/board/coreboot/coreboot/coreboot.c b/board/coreboot/coreboot/coreboot.c index 175d3ce691a..93f897893eb 100644 --- a/board/coreboot/coreboot/coreboot.c +++ b/board/coreboot/coreboot/coreboot.c @@ -39,6 +39,7 @@ int show_board_info(void) const char *bios_ver = smbios_string(bios, t0->bios_ver);
I would insert the line here and name the variable as bios_date:
const char *bios_date = smbios_string(bios, t0->bios_release_date);
const char *model = smbios_string(system, t1->product_name); const char *manufacturer = smbios_string(system, t1->manufacturer);
const char *date = smbios_string(bios, t0->bios_release_date); if (!model || !manufacturer || !bios_ver) goto fallback;@@ -46,6 +47,8 @@ int show_board_info(void) printf("Vendor: %s\n", manufacturer); printf("Model: %s\n", model); printf("BIOS Version: %s\n", bios_ver);
if (date)printf("BIOS date: %s\n", date); return 0;
Regards, Bin
Set up coral so that it can boot from coreboot, even though it is a bare-metal build. This helps with testing since the same image can be used in both cases.
Signed-off-by: Simon Glass sjg@chromium.org ---
board/google/chromebook_coral/coral.c | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/board/google/chromebook_coral/coral.c b/board/google/chromebook_coral/coral.c index 3f9235c903b..b16252d2a8b 100644 --- a/board/google/chromebook_coral/coral.c +++ b/board/google/chromebook_coral/coral.c @@ -10,17 +10,21 @@ #include <command.h> #include <cros_ec.h> #include <dm.h> +#include <init.h> #include <log.h> #include <sysinfo.h> #include <acpi/acpigen.h> #include <asm-generic/gpio.h> #include <asm/acpi_nhlt.h> +#include <asm/cb_sysinfo.h> #include <asm/intel_gnvs.h> #include <asm/intel_pinctrl.h> #include <dm/acpi.h> #include <linux/delay.h> #include "variant_gpio.h"
+DECLARE_GLOBAL_DATA_PTR; + struct cros_gpio_info { const char *linux_name; enum cros_gpio_t type; @@ -28,6 +32,30 @@ struct cros_gpio_info { int flags; };
+int misc_init_f(void) +{ + if (!ll_boot_init()) { + printf("Running as secondary loader"); + if (gd->arch.coreboot_table) { + int ret; + + printf(" (found coreboot table at %lx)", + gd->arch.coreboot_table); + + ret = get_coreboot_info(&lib_sysinfo); + if (ret != 0) { + printf("Failed to parse coreboot tables (err=%d)\n", + ret); + return ret; + } + } + + printf("\n"); + } + + return 0; +} + int arch_misc_init(void) { return 0;
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
Set up coral so that it can boot from coreboot, even though it is a bare-metal build. This helps with testing since the same image can be used in both cases.
Signed-off-by: Simon Glass sjg@chromium.org
board/google/chromebook_coral/coral.c | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/board/google/chromebook_coral/coral.c b/board/google/chromebook_coral/coral.c index 3f9235c903b..b16252d2a8b 100644 --- a/board/google/chromebook_coral/coral.c +++ b/board/google/chromebook_coral/coral.c @@ -10,17 +10,21 @@ #include <command.h> #include <cros_ec.h> #include <dm.h> +#include <init.h> #include <log.h> #include <sysinfo.h> #include <acpi/acpigen.h> #include <asm-generic/gpio.h> #include <asm/acpi_nhlt.h> +#include <asm/cb_sysinfo.h> #include <asm/intel_gnvs.h> #include <asm/intel_pinctrl.h> #include <dm/acpi.h> #include <linux/delay.h> #include "variant_gpio.h"
+DECLARE_GLOBAL_DATA_PTR;
struct cros_gpio_info { const char *linux_name; enum cros_gpio_t type; @@ -28,6 +32,30 @@ struct cros_gpio_info { int flags; };
+int misc_init_f(void) +{
if (!ll_boot_init()) {printf("Running as secondary loader");if (gd->arch.coreboot_table) {int ret;printf(" (found coreboot table at %lx)",gd->arch.coreboot_table);
Missing an ending '\n', if the following get_coreboot_info() fails
ret = get_coreboot_info(&lib_sysinfo);if (ret != 0) {printf("Failed to parse coreboot tables (err=%d)\n",ret);return ret;}}printf("\n");}return 0;+}
int arch_misc_init(void) { return 0; --
Otherwise, Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
Add a function comment for get_coreboot_info() and a declaration for cb_get_sysinfo(), since this may be called from elsewhere.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/include/asm/cb_sysinfo.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/arch/x86/include/asm/cb_sysinfo.h b/arch/x86/include/asm/cb_sysinfo.h index 675eef6f2c9..75901359f98 100644 --- a/arch/x86/include/asm/cb_sysinfo.h +++ b/arch/x86/include/asm/cb_sysinfo.h @@ -215,6 +215,22 @@ struct sysinfo_t {
extern struct sysinfo_t lib_sysinfo;
+/** + * get_coreboot_info() - parse the coreboot sysinfo table + * + * Parses the coreboot table if found, setting the GD_FLG_SKIP_LL_INIT flag if + * so. + * + * @info: Place to put the parsed information + * @return 0 if OK, -ENOENT if no table found + */ int get_coreboot_info(struct sysinfo_t *info);
+/** + * cb_get_sysinfo() - get a pointer to the parsed coreboot sysinfo + * + * @return pointer to sysinfo, or NULL if not available + */ +const struct sysinfo_t *cb_get_sysinfo(void); + #endif
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
Add a function comment for get_coreboot_info() and a declaration for cb_get_sysinfo(), since this may be called from elsewhere.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/cb_sysinfo.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Use VENDOR_COREBOOT instead of TARGET_COREBOOT so we can have multiple coreboot boards, sharing options. Only SYS_CONFIG_NAME needs to be defined TARGET_COREBOOT.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/coreboot/Kconfig | 2 +- board/coreboot/coreboot/Kconfig | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/x86/cpu/coreboot/Kconfig b/arch/x86/cpu/coreboot/Kconfig index 497d6284ac1..b97c2779041 100644 --- a/arch/x86/cpu/coreboot/Kconfig +++ b/arch/x86/cpu/coreboot/Kconfig @@ -1,4 +1,4 @@ -if TARGET_COREBOOT +if VENDOR_COREBOOT
config SYS_COREBOOT bool diff --git a/board/coreboot/coreboot/Kconfig b/board/coreboot/coreboot/Kconfig index 5bd6465d989..05e9b3b6f75 100644 --- a/board/coreboot/coreboot/Kconfig +++ b/board/coreboot/coreboot/Kconfig @@ -1,4 +1,4 @@ -if TARGET_COREBOOT +if VENDOR_COREBOOT
config SYS_BOARD default "coreboot" @@ -9,9 +9,6 @@ config SYS_VENDOR config SYS_SOC default "coreboot"
-config SYS_CONFIG_NAME - default "coreboot" - config SYS_TEXT_BASE default 0x01110000
@@ -31,4 +28,11 @@ config SYS_CAR_SIZE help This option specifies the board specific Cache-As-RAM (CAR) size.
+endif # CONFIG_VENDOR_COREBOOT + +if TARGET_COREBOOT + +config SYS_CONFIG_NAME + default "coreboot" + endif
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
Use VENDOR_COREBOOT instead of TARGET_COREBOOT so we can have multiple coreboot boards, sharing options. Only SYS_CONFIG_NAME needs to be defined TARGET_COREBOOT.
I am not sure what use case this is? This change makes no difference when U-Boot is built as a coreboot payload.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/coreboot/Kconfig | 2 +- board/coreboot/coreboot/Kconfig | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-)
Regards, Bin
Hi Bin,
On Thu, 8 Apr 2021 at 14:59, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
Use VENDOR_COREBOOT instead of TARGET_COREBOOT so we can have multiple coreboot boards, sharing options. Only SYS_CONFIG_NAME needs to be defined TARGET_COREBOOT.
I am not sure what use case this is? This change makes no difference when U-Boot is built as a coreboot payload.
This is because I want to add another coreboot board, called 'chromeos_coreboot'. So in that case it will have VENDOR_COREBOOT enabled, but not TARGET_COREBOOT.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/coreboot/Kconfig | 2 +- board/coreboot/coreboot/Kconfig | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-)
Regards, Bin
Depending on a particular TARGET Kconfig is not a good idea, IMO.
Regards, Simon
Enable this by default on coreboot, since it is quite useful there.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/coreboot/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/x86/cpu/coreboot/Kconfig b/arch/x86/cpu/coreboot/Kconfig index b97c2779041..11385918d83 100644 --- a/arch/x86/cpu/coreboot/Kconfig +++ b/arch/x86/cpu/coreboot/Kconfig @@ -26,5 +26,6 @@ config SYS_COREBOOT imply CBMEM_CONSOLE imply X86_TSC_READ_BASE select BINMAN if X86_64 + imply CMD_CBSYSINFO
endif
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
Enable this by default on coreboot, since it is quite useful there.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/coreboot/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/x86/cpu/coreboot/Kconfig b/arch/x86/cpu/coreboot/Kconfig index b97c2779041..11385918d83 100644 --- a/arch/x86/cpu/coreboot/Kconfig +++ b/arch/x86/cpu/coreboot/Kconfig @@ -26,5 +26,6 @@ config SYS_COREBOOT imply CBMEM_CONSOLE imply X86_TSC_READ_BASE select BINMAN if X86_64
imply CMD_CBSYSINFO
This is not needed as in cmd/Kconfig, we have:
config CMD_CBSYSINFO default y if SYS_COREBOOT
Regards, Bin
Add information about memory usage when U-Boot is started from coreboot. This is useful when debugging. Also, since coreboot takes a chunk of memory in the middle of SDRAM for use by PCI devices, it can help avoid overwriting this with a loaded kernel by accident.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/board/coreboot/coreboot.rst | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/doc/board/coreboot/coreboot.rst b/doc/board/coreboot/coreboot.rst index 9c44c025a48..e791b7e39f0 100644 --- a/doc/board/coreboot/coreboot.rst +++ b/doc/board/coreboot/coreboot.rst @@ -50,3 +50,22 @@ works by using a 32-bit SPL binary to switch to 64-bit for running U-Boot. It can be useful for running UEFI applications, for example.
This has only been lightly tested. + + +Memory map +---------- + +:: + + ffffffff Top of ROM (and last byte of 32-bit address space) + 7a9fd000 Typical top of memory available to U-Boot + (use cbsysinfo to see where memory range 'table' starts) + 10000000 Memory reserved by coreboot for mapping PCI devices + (typical size 2151000, includes framebuffer) + 1920000 CONFIG_SYS_CAR_ADDR, fake Cache-as-RAM memory, used during startup + 1110000 CONFIG_SYS_TEXT_BASE (start address of U-Boot code, before reloc) + 110000 CONFIG_BLOBLIST_ADDR (before being relocated) + 100000 CONFIG_PRE_CON_BUF_ADDR + f0000 ACPI tables set up by U-Boot + (typically redirects to 7ab10030 or similar) + 500 Location of coreboot sysinfo table, used during startup
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
Add information about memory usage when U-Boot is started from coreboot. This is useful when debugging. Also, since coreboot takes a chunk of memory in the middle of SDRAM for use by PCI devices, it can help avoid overwriting this with a loaded kernel by accident.
Signed-off-by: Simon Glass sjg@chromium.org
doc/board/coreboot/coreboot.rst | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/doc/board/coreboot/coreboot.rst b/doc/board/coreboot/coreboot.rst index 9c44c025a48..e791b7e39f0 100644 --- a/doc/board/coreboot/coreboot.rst +++ b/doc/board/coreboot/coreboot.rst @@ -50,3 +50,22 @@ works by using a 32-bit SPL binary to switch to 64-bit for running U-Boot. It can be useful for running UEFI applications, for example.
This has only been lightly tested.
+Memory map +----------
+::
Can we use the reST table syntax for the following table?
- ffffffff Top of ROM (and last byte of 32-bit address space)
- 7a9fd000 Typical top of memory available to U-Boot
(use cbsysinfo to see where memory range 'table' starts)- 10000000 Memory reserved by coreboot for mapping PCI devices
(typical size 2151000, includes framebuffer)1920000 CONFIG_SYS_CAR_ADDR, fake Cache-as-RAM memory, used during startup1110000 CONFIG_SYS_TEXT_BASE (start address of U-Boot code, before reloc)110000 CONFIG_BLOBLIST_ADDR (before being relocated)100000 CONFIG_PRE_CON_BUF_ADDRf0000 ACPI tables set up by U-Boot(typically redirects to 7ab10030 or similar)500 Location of coreboot sysinfo table, used during startup--
Regards, Bin
All the x86 devicetree files are built at once, whichever board is actually being built. If coreboot is the target build, CONFIG_ROM_SIZE is not defined and samus cannot build Chromium OS verified boot. Add this condition to avoid errors about CONFIG_ROM_SIZE being missing.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/dts/chromebook_coral.dts | 2 +- arch/x86/dts/chromebook_samus.dts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/dts/chromebook_coral.dts b/arch/x86/dts/chromebook_coral.dts index c8cb4e21c6d..66c31efb6cd 100644 --- a/arch/x86/dts/chromebook_coral.dts +++ b/arch/x86/dts/chromebook_coral.dts @@ -10,7 +10,7 @@ /include/ "rtc.dtsi" /include/ "tsc_timer.dtsi"
-#ifdef CONFIG_CHROMEOS_VBOOT +#if defined(CONFIG_CHROMEOS_VBOOT) && defined(CONFIG_ROM_SIZE) #include "chromeos-x86.dtsi" #include "flashmap-x86-ro.dtsi" #include "flashmap-16mb-rw.dtsi" diff --git a/arch/x86/dts/chromebook_samus.dts b/arch/x86/dts/chromebook_samus.dts index adaeb1ea355..ad35ab2e3fd 100644 --- a/arch/x86/dts/chromebook_samus.dts +++ b/arch/x86/dts/chromebook_samus.dts @@ -11,7 +11,7 @@
#include "smbios.dtsi"
-#ifdef CONFIG_CHROMEOS_VBOOT +#if defined(CONFIG_CHROMEOS_VBOOT) && defined(CONFIG_ROM_SIZE) #include "chromeos-x86.dtsi" #include "flashmap-x86-ro.dtsi" #include "flashmap-8mb-rw.dtsi"
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
All the x86 devicetree files are built at once, whichever board is actually being built. If coreboot is the target build, CONFIG_ROM_SIZE is not defined and samus cannot build Chromium OS verified boot. Add this condition to avoid errors about CONFIG_ROM_SIZE being missing.
The commit title should use "x86: chromebook" as the tags, since it's not x86 generic thing.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/dts/chromebook_coral.dts | 2 +- arch/x86/dts/chromebook_samus.dts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Otherwise, Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
On 4/7/21 1:32 PM, Simon Glass wrote:
All the x86 devicetree files are built at once, whichever board is actually being built. If coreboot is the target build, CONFIG_ROM_SIZE is not defined and samus cannot build Chromium OS verified boot. Add this condition to avoid errors about CONFIG_ROM_SIZE being missing.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Best Regards, Jaehoon Chung
arch/x86/dts/chromebook_coral.dts | 2 +- arch/x86/dts/chromebook_samus.dts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/dts/chromebook_coral.dts b/arch/x86/dts/chromebook_coral.dts index c8cb4e21c6d..66c31efb6cd 100644 --- a/arch/x86/dts/chromebook_coral.dts +++ b/arch/x86/dts/chromebook_coral.dts @@ -10,7 +10,7 @@ /include/ "rtc.dtsi" /include/ "tsc_timer.dtsi"
-#ifdef CONFIG_CHROMEOS_VBOOT +#if defined(CONFIG_CHROMEOS_VBOOT) && defined(CONFIG_ROM_SIZE) #include "chromeos-x86.dtsi" #include "flashmap-x86-ro.dtsi" #include "flashmap-16mb-rw.dtsi" diff --git a/arch/x86/dts/chromebook_samus.dts b/arch/x86/dts/chromebook_samus.dts index adaeb1ea355..ad35ab2e3fd 100644 --- a/arch/x86/dts/chromebook_samus.dts +++ b/arch/x86/dts/chromebook_samus.dts @@ -11,7 +11,7 @@
#include "smbios.dtsi"
-#ifdef CONFIG_CHROMEOS_VBOOT +#if defined(CONFIG_CHROMEOS_VBOOT) && defined(CONFIG_ROM_SIZE) #include "chromeos-x86.dtsi" #include "flashmap-x86-ro.dtsi" #include "flashmap-8mb-rw.dtsi"
With of-platdata-inst we want to set up a reference to each devices' parent device, if there is one. If we find that the device has a parent (i.e. is not a root node) but it is not in the list of devices being written, then we cannot create the reference.
Report an error in this case, since it indicates that the parent node is either missing a compatible string, is disabled, or perhaps does not have any properties because it was not tagged for SPL.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/dtoc/dtb_platdata.py | 9 ++++++++ tools/dtoc/test/dtoc_test_noparent.dts | 32 ++++++++++++++++++++++++++ tools/dtoc/test_dtoc.py | 10 ++++++++ 3 files changed, 51 insertions(+) create mode 100644 tools/dtoc/test/dtoc_test_noparent.dts
diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 1374f01c707..c303224e295 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -749,6 +749,15 @@ class DtbPlatdata(): break
if node.parent and node.parent.parent: + if node.parent not in self._valid_nodes: + # This might indicate that the parent node is not in the + # SPL/TPL devicetree but the child is. For example if we are + # dealing with of-platdata in TPL, the parent has a + # u-boot,dm-spl tag but the child has u-boot,dm-pre-reloc. In + # this case the child node exists in TPL but the parent does + # not. + raise ValueError("Node '%s' requires parent node '%s' but it is not in the valid list" % + (node.path, node.parent.path)) self.buf('\t.parent\t\t= DM_DEVICE_REF(%s),\n' % node.parent.var_name) if priv_name: diff --git a/tools/dtoc/test/dtoc_test_noparent.dts b/tools/dtoc/test/dtoc_test_noparent.dts new file mode 100644 index 00000000000..5a820301e72 --- /dev/null +++ b/tools/dtoc/test/dtoc_test_noparent.dts @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2017 Google, Inc + */ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + i2c@0 { + compatible = "sandbox,i2c"; + u-boot,dm-pre-tpl; + #address-cells = <1>; + #size-cells = <0>; + spl-test { + u-boot,dm-pre-reloc; + compatible = "sandbox,spl-test"; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + pmic@9 { + compatible = "sandbox,pmic"; + u-boot,dm-pre-reloc; + reg = <9>; + low-power; + }; + }; + }; +}; diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index a05e3d9ed65..e7397ae554b 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -1839,3 +1839,13 @@ U_BOOT_DRVINFO(spl_test2) = { dtb_file = get_dtb_file('dtoc_test_single_reg.dts') output = tools.GetOutputFilename('output') self.run_test(['struct'], dtb_file, output) + + def test_missing_parent(self): + """Test detection of a parent node with no properties""" + dtb_file = get_dtb_file('dtoc_test_noparent.dts', capture_stderr=True) + output = tools.GetOutputFilename('output') + with self.assertRaises(ValueError) as exc: + self.run_test(['device'], dtb_file, output, instantiate=True) + self.assertIn("Node '/i2c@0/spl-test/pmic@9' requires parent node " + "'/i2c@0/spl-test' but it is not in the valid list", + str(exc.exception))
With the new 2021.04 we have a new version of Chromium OS boot, which supports sandbox, coral and coral-on-coreboot. Add documentation for this.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/chromium/run_vboot.rst | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/doc/chromium/run_vboot.rst b/doc/chromium/run_vboot.rst index 41b4f631835..a9e4408d55f 100644 --- a/doc/chromium/run_vboot.rst +++ b/doc/chromium/run_vboot.rst @@ -6,11 +6,15 @@ Running U-Boot with Chromium OS verified boot =============================================
+Note: Once you use the source below you can obtain extra documentation with +'make htmldocs'. See the 'Internal Documentation' link, under +'Chromium OS-specific doc'. + To obtain::
git clone https://github.com/sjg20/u-boot.git cd u-boot - git checkout cros-master + git checkout cros-2021.04
cd .. git clone https://chromium.googlesource.com/chromiumos/platform/vboot_reference @@ -169,7 +173,8 @@ detect problems that affect the flow or particular vboot features. U-Boot without Chromium OS verified boot ----------------------------------------
-The following script can be used to boot a Chrome OS image on coral:: +The following script can be used to boot a Chrome OS image on coral. It is +defined as the boot command in mainline::
# Read the image header and obtain the address of the kernel # The offset 4f0 is defined by verified boot and may change for other @@ -195,10 +200,4 @@ The following script can be used to boot a Chrome OS image on coral:: zboot go
-TO DO ------ - -Get the full ACPI tables working with Coral - - 7 October 2018
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
With the new 2021.04 we have a new version of Chromium OS boot, which supports sandbox, coral and coral-on-coreboot. Add documentation for
Chrominum OS can run on Sandbox?
this.
Signed-off-by: Simon Glass sjg@chromium.org
doc/chromium/run_vboot.rst | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
Hi Bin,
On Thu, 8 Apr 2021 at 15:02, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Apr 7, 2021 at 12:33 PM Simon Glass sjg@chromium.org wrote:
With the new 2021.04 we have a new version of Chromium OS boot, which supports sandbox, coral and coral-on-coreboot. Add documentation for
Chrominum OS can run on Sandbox?
Well the U-Boot part, yes. Once it boots a kernel sandbox just exits. This is useful for testing.
this.
Signed-off-by: Simon Glass sjg@chromium.org
doc/chromium/run_vboot.rst | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Simon
participants (4)
-
Andy Shevchenko -
Bin Meng -
Jaehoon Chung -
Simon Glass