[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Simon Glass]

This little series adds a few checks into the code to allow better
operation when booting a build from a previous-state loader such as
coreboot.

At present we have a 'coreboot' target but this runs very different code
from the bare-metal targets, such as coral. There is very little in common
between them.

It is useful to be able to boot the same U-Boot on a device, with or
without a first-stage bootloader. For example, with chromebook_coral, it
is helpful for testing to be able to boot the same U-Boot (complete with
FSP) on bare metal and from coreboot. It allows checking of things like
CPU speed, comparing registers, ACPI tables and the like.

This series allows U-Boot to detect that it ran from coreboot and
automatically do the right thing.

This series makes the most important changes to allow the same u-boot.bin
for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL).

Changes in v3:
- Add a new patch with a gd flag for chain loading
- Add new patch to detect running from coreboot

Changes in v2:
- Drop the other check in interrupt_init() which is not needed now
- Drop patch 'dm: Avoid initing built-in devices when chain loading'

Simon Glass (7):
  x86: fsp: Allow skipping init code when chain loading
  x86: apl: Skip init code when chain loading
  x86: cpu: Skip init code when chain loading
  pci: Avoid auto-config when chain loading
  board: Add a gd flag for chain loading
  x86: Add a way to detect running from coreboot
  x86: Use the existing stack when chain-loading

 arch/x86/cpu/apollolake/fsp_s.c   |  2 ++
 arch/x86/cpu/cpu.c                |  4 +++-
 arch/x86/cpu/i386/cpu.c           | 15 +++++++++++++++
 arch/x86/cpu/i386/interrupt.c     |  6 ++++--
 arch/x86/cpu/start_from_spl.S     | 16 ++++++++++++++--
 arch/x86/include/asm/u-boot-x86.h |  7 +++++++
 arch/x86/lib/fsp/fsp_dram.c       |  8 ++++++++
 arch/x86/lib/fsp/fsp_graphics.c   |  3 +++
 arch/x86/lib/fsp2/fsp_dram.c      | 10 ++++++++++
 arch/x86/lib/fsp2/fsp_init.c      |  2 +-
 arch/x86/lib/init_helpers.c       |  3 +++
 drivers/pci/pci-uclass.c          |  4 ++--
 include/asm-generic/global_data.h |  1 +
 include/init.h                    |  2 +-
 14 files changed, 74 insertions(+), 9 deletions(-)

-- 
2.25.1.481.gfbce0eb801-goog

[PATCH v3 1/7] x86: fsp: Allow skipping init code when chain loading

[Simon Glass]

It is useful to be able to boot the same x86 image on a device with or
without a first-stage bootloader. For example, with chromebook_coral, it
is helpful for testing to be able to boot the same U-Boot (complete with
FSP) on bare metal and from coreboot. It allows checking of things like
CPU speed, comparing registers, ACPI tables and the like.

When U-Boot is not the first-stage bootloader much of this code is not
needed and can break booting. Add checks for this to the FSP code.

Rather than checking for the amount of available SDRAM, just use 1GB in
this situation, which should be safe. Using 2GB may run into a memory
hole on some SoCs.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 arch/x86/lib/fsp/fsp_dram.c     |  8 ++++++++
 arch/x86/lib/fsp/fsp_graphics.c |  3 +++
 arch/x86/lib/fsp2/fsp_dram.c    | 10 ++++++++++
 arch/x86/lib/fsp2/fsp_init.c    |  2 +-
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/x86/lib/fsp/fsp_dram.c b/arch/x86/lib/fsp/fsp_dram.c
index 9ce0ddf0d3..15e82de2fe 100644
--- a/arch/x86/lib/fsp/fsp_dram.c
+++ b/arch/x86/lib/fsp/fsp_dram.c
@@ -44,6 +44,14 @@ int dram_init_banksize(void)
 	phys_addr_t low_end;
 	uint bank;
 
+	if (!ll_boot_init()) {
+		gd->bd->bi_dram[0].start = 0;
+		gd->bd->bi_dram[0].size = gd->ram_size;
+
+		mtrr_add_request(MTRR_TYPE_WRBACK, 0, gd->ram_size);
+		return 0;
+	}
+
 	low_end = 0;
 	for (bank = 1, hdr = gd->arch.hob_list;
 	     bank < CONFIG_NR_DRAM_BANKS && !end_of_hob(hdr);
diff --git a/arch/x86/lib/fsp/fsp_graphics.c b/arch/x86/lib/fsp/fsp_graphics.c
index 226c7e66b3..98b762209f 100644
--- a/arch/x86/lib/fsp/fsp_graphics.c
+++ b/arch/x86/lib/fsp/fsp_graphics.c
@@ -78,6 +78,9 @@ static int fsp_video_probe(struct udevice *dev)
 	struct vesa_mode_info *vesa = &mode_info.vesa;
 	int ret;
 
+	if (!ll_boot_init())
+		return 0;
+
 	printf("Video: ");
 
 	/* Initialize vesa_mode_info structure */
diff --git a/arch/x86/lib/fsp2/fsp_dram.c b/arch/x86/lib/fsp2/fsp_dram.c
index 90a238a224..74835eebce 100644
--- a/arch/x86/lib/fsp2/fsp_dram.c
+++ b/arch/x86/lib/fsp2/fsp_dram.c
@@ -12,11 +12,18 @@
 #include <asm/fsp/fsp_support.h>
 #include <asm/fsp2/fsp_api.h>
 #include <asm/fsp2/fsp_internal.h>
+#include <linux/sizes.h>
 
 int dram_init(void)
 {
 	int ret;
 
+	if (!ll_boot_init()) {
+		/* Use a small and safe amount of 1GB */
+		gd->ram_size = SZ_1G;
+
+		return 0;
+	}
 	if (spl_phase() == PHASE_SPL) {
 #ifdef CONFIG_HAVE_ACPI_RESUME
 		bool s3wake = gd->arch.prev_sleep_state == ACPI_S3;
@@ -68,6 +75,9 @@ int dram_init(void)
 
 ulong board_get_usable_ram_top(ulong total_size)
 {
+	if (!ll_boot_init())
+		return gd->ram_size;
+
 #if CONFIG_IS_ENABLED(HANDOFF)
 	struct spl_handoff *ho = gd->spl_handoff;
 
diff --git a/arch/x86/lib/fsp2/fsp_init.c b/arch/x86/lib/fsp2/fsp_init.c
index da9bd6b45c..c7dc2ea257 100644
--- a/arch/x86/lib/fsp2/fsp_init.c
+++ b/arch/x86/lib/fsp2/fsp_init.c
@@ -23,7 +23,7 @@ int arch_cpu_init_dm(void)
 	int ret;
 
 	/* Make sure pads are set up early in U-Boot */
-	if (spl_phase() != PHASE_BOARD_F)
+	if (!ll_boot_init() || spl_phase() != PHASE_BOARD_F)
 		return 0;
 
 	/* Probe all pinctrl devices to set up the pads */
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH v3 2/7] x86: apl: Skip init code when chain loading

[Simon Glass]

When U-Boot is not the first-stage bootloader the FSP-S init must be
skipped. Update it to add a check.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 arch/x86/cpu/apollolake/fsp_s.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/cpu/apollolake/fsp_s.c b/arch/x86/cpu/apollolake/fsp_s.c
index 1f22c1ea3c..5d252b1111 100644
--- a/arch/x86/cpu/apollolake/fsp_s.c
+++ b/arch/x86/cpu/apollolake/fsp_s.c
@@ -566,6 +566,8 @@ int arch_fsp_init_r(void)
 	struct udevice *dev, *itss;
 	int ret;
 
+	if (!ll_boot_init())
+		return 0;
 	/*
 	 * This must be called before any devices are probed. Put any probing
 	 * into arch_fsps_preinit() above.
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH v3 3/7] x86: cpu: Skip init code when chain loading

[Simon Glass]

When U-Boot is not the first-stage bootloader the interrupt and cache init
must be skipped, as well as init for various peripherals. Update the code
to add checks for this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2:
- Drop the other check in interrupt_init() which is not needed now

 arch/x86/cpu/cpu.c            | 4 +++-
 arch/x86/cpu/i386/interrupt.c | 6 ++++--
 arch/x86/lib/init_helpers.c   | 3 +++
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index dae06949cc..3db035c2c0 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -239,8 +239,10 @@ int cpu_init_r(void)
 	struct udevice *dev;
 	int ret;
 
-	if (!ll_boot_init())
+	if (!ll_boot_init()) {
+		uclass_first_device(UCLASS_PCI, &dev);
 		return 0;
+	}
 
 	ret = x86_init_cpus();
 	if (ret)
diff --git a/arch/x86/cpu/i386/interrupt.c b/arch/x86/cpu/i386/interrupt.c
index 4c7e9ea215..e67a116ac1 100644
--- a/arch/x86/cpu/i386/interrupt.c
+++ b/arch/x86/cpu/i386/interrupt.c
@@ -264,6 +264,9 @@ int interrupt_init(void)
 	struct udevice *dev;
 	int ret;
 
+	if (!ll_boot_init())
+		return 0;
+
 	/* Try to set up the interrupt router, but don't require one */
 	ret = irq_first_device_type(X86_IRQT_BASE, &dev);
 	if (ret && ret != -ENODEV)
@@ -295,8 +298,7 @@ int interrupt_init(void)
 	 * TODO(sjg at chromium.org): But we don't handle these correctly when
 	 * booted from EFI.
 	 */
-	if (ll_boot_init())
-		enable_interrupts();
+	enable_interrupts();
 #endif
 
 	return 0;
diff --git a/arch/x86/lib/init_helpers.c b/arch/x86/lib/init_helpers.c
index 5bb55e256f..d906b528b3 100644
--- a/arch/x86/lib/init_helpers.c
+++ b/arch/x86/lib/init_helpers.c
@@ -30,6 +30,9 @@ int init_cache_f_r(void)
 			return ret;
 	}
 
+	if (!ll_boot_init())
+		return 0;
+
 	/* Initialise the CPU cache(s) */
 	return init_cache();
 }
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH v3 4/7] pci: Avoid auto-config when chain loading

[Simon Glass]

When U-Boot is not the first-stage bootloader we don't want to
re-configure the PCI devices, since this has already been done. Add a
check to avoid this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2:
- Drop patch 'dm: Avoid initing built-in devices when chain loading'

 drivers/pci/pci-uclass.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index e2882e3b63..94733662b1 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -1007,7 +1007,7 @@ static int pci_uclass_post_probe(struct udevice *bus)
 	if (ret)
 		return ret;
 
-	if (CONFIG_IS_ENABLED(PCI_PNP) &&
+	if (CONFIG_IS_ENABLED(PCI_PNP) && ll_boot_init() &&
 	    (!hose->skip_auto_config_until_reloc ||
 	     (gd->flags & GD_FLG_RELOC))) {
 		ret = pci_auto_config_devices(bus);
@@ -1029,7 +1029,7 @@ static int pci_uclass_post_probe(struct udevice *bus)
 	 * Note we only call this 1) after U-Boot is relocated, and 2)
 	 * root bus has finished probing.
 	 */
-	if ((gd->flags & GD_FLG_RELOC) && (bus->seq == 0)) {
+	if ((gd->flags & GD_FLG_RELOC) && bus->seq == 0 && ll_boot_init()) {
 		ret = fsp_init_phase_pci();
 		if (ret)
 			return ret;
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH v3 5/7] board: Add a gd flag for chain loading

[Simon Glass]

When U-Boot is run from another boot loader, much of the low-level init
needs to be skipped.

Add a flag for this and adjust ll_boot_init() to use it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Add a new patch with a gd flag for chain loading

Changes in v2: None

 include/asm-generic/global_data.h | 1 +
 include/init.h                    | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 5d027329fe..3dc51e49ef 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -166,5 +166,6 @@ typedef struct global_data {
 #define GD_FLG_SPL_EARLY_INIT	0x04000 /* Early SPL init is done	   */
 #define GD_FLG_LOG_READY	0x08000 /* Log system is ready for use	   */
 #define GD_FLG_WDT_READY	0x10000 /* Watchdog is ready for use	   */
+#define GD_FLG_NO_LL_INIT	0x20000	/* Don't perform low-level init	   */
 
 #endif /* __ASM_GENERIC_GBL_DATA_H */
diff --git a/include/init.h b/include/init.h
index 2a33a3fd1e..64fdf12491 100644
--- a/include/init.h
+++ b/include/init.h
@@ -20,7 +20,7 @@ struct global_data;
 #ifdef CONFIG_EFI_STUB
 #define ll_boot_init()	false
 #else
-#define ll_boot_init()	true
+#define ll_boot_init()	(!(gd->flags & GD_FLG_NO_LL_INIT))
 #endif
 
 /*
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH v3 6/7] x86: Add a way to detect running from coreboot

[Simon Glass]

If U-Boot is running from coreboot we need to skip low-level init. Add
an way to detect this and to set the gd flag.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Add new patch to detect running from coreboot

Changes in v2: None

 arch/x86/cpu/i386/cpu.c           | 15 +++++++++++++++
 arch/x86/include/asm/u-boot-x86.h |  7 +++++++
 2 files changed, 22 insertions(+)

diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
index c8da7f10e9..e43444c090 100644
--- a/arch/x86/cpu/i386/cpu.c
+++ b/arch/x86/cpu/i386/cpu.c
@@ -447,10 +447,25 @@ int x86_cpu_init_f(void)
 	return 0;
 }
 
+bool x86_detect_coreboot(void)
+{
+	u32 *ptr, *end;
+
+	/* We look for LBIO in the first 4K of RAM */
+	for (ptr = NULL, end = ptr + 0x400; ptr < end; ptr += 4) {
+		if (*ptr == 0x4f49424c) /* "LBIO" */
+			return true;
+	}
+
+	return false;
+}
+
 int x86_cpu_reinit_f(void)
 {
 	setup_identity();
 	setup_pci_ram_top();
+	if (x86_detect_coreboot())
+		gd->flags |= GD_FLG_NO_LL_INIT;
 
 	return 0;
 }
diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
index 3e5d56d075..654880f91c 100644
--- a/arch/x86/include/asm/u-boot-x86.h
+++ b/arch/x86/include/asm/u-boot-x86.h
@@ -43,6 +43,13 @@ int x86_cpu_reinit_f(void);
  */
 int x86_cpu_init_tpl(void);
 
+/**
+ * x86_detect_coreboot() - See if U-Boot is being started from coreboot
+ *
+ * @return true if coreboot is running, false if U-Boot is running 'bare-metal'
+ */
+bool x86_detect_coreboot(void);
+
 int cpu_init_f(void);
 void setup_gdt(struct global_data *id, u64 *gdt_addr);
 /*
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH v3 7/7] x86: Use the existing stack when chain-loading

[Simon Glass]

With chromebook_coral we normally run TPL->SPL->U-Boot. This is the
'bare metal' case.

When running from coreboot we put u-boot.bin in the RW_LEGACY portion
of the image, e.g. with:

   cbfstool image-coral.serial.bin add-flat-binary -r RW_LEGACY \
	-f /tmp/b/chromebook_coral/u-boot.bin -n altfw/u-boot \
	-c lzma -l 0x1110000 -e 0x1110000

In this case U-Boot is run from coreboot (actually Depthcharge, its
payload) so we cannot access CAR. Use the existing stack instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 arch/x86/cpu/start_from_spl.S | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/cpu/start_from_spl.S b/arch/x86/cpu/start_from_spl.S
index 22cab2dd6c..75c328fd7a 100644
--- a/arch/x86/cpu/start_from_spl.S
+++ b/arch/x86/cpu/start_from_spl.S
@@ -14,18 +14,30 @@
 .globl _start
 .type _start, @function
 _start:
-	/* Set up memory using the existing stack */
+	/*
+	 * If running from coreboot, CAR is no-longer available. Use the
+	 * existing stack, which is large enough.
+	 */
+	call	x86_detect_coreboot
+	cmp	$0, %eax
+	jne	use_existing_stack
+
 	movl	$(CONFIG_SYS_CAR_ADDR + CONFIG_SYS_CAR_SIZE - 4), %eax
 #ifdef CONFIG_DCACHE_RAM_MRC_VAR_SIZE
 	subl	$CONFIG_DCACHE_RAM_MRC_VAR_SIZE, %eax
 #endif
+	jmp	2f
 	/*
-	 * We don't subject CONFIG_DCACHE_RAM_MRC_VAR_SIZE since memory is
+	 * We don't subtract CONFIG_DCACHE_RAM_MRC_VAR_SIZE since memory is
 	 * already set up. This has the happy side-effect of putting gd in a
 	 * new place separate from SPL, so the memset() in
 	 * board_init_f_init_reserve() does not cause any problems (otherwise
 	 * it would zero out the gd and crash)
 	 */
+	/* Set up memory using the existing stack */
+use_existing_stack:
+	mov	%esp, %eax
+2:
 	call	board_init_f_alloc_reserve
 	mov	%eax, %esp
 
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Andy Shevchenko]

On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote:
> This little series adds a few checks into the code to allow better
> operation when booting a build from a previous-state loader such as
> coreboot.
> 
> At present we have a 'coreboot' target but this runs very different code
> from the bare-metal targets, such as coral. There is very little in common
> between them.
> 
> It is useful to be able to boot the same U-Boot on a device, with or
> without a first-stage bootloader. For example, with chromebook_coral, it
> is helpful for testing to be able to boot the same U-Boot (complete with
> FSP) on bare metal and from coreboot. It allows checking of things like
> CPU speed, comparing registers, ACPI tables and the like.
> 
> This series allows U-Boot to detect that it ran from coreboot and
> automatically do the right thing.
> 
> This series makes the most important changes to allow the same u-boot.bin
> for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL).

I see only this and one patch out of 7 (seven?).
Please, (re)send it correctly.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Simon Glass]

Hi Andy,

On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote:
> > This little series adds a few checks into the code to allow better
> > operation when booting a build from a previous-state loader such as
> > coreboot.
> >
> > At present we have a 'coreboot' target but this runs very different code
> > from the bare-metal targets, such as coral. There is very little in common
> > between them.
> >
> > It is useful to be able to boot the same U-Boot on a device, with or
> > without a first-stage bootloader. For example, with chromebook_coral, it
> > is helpful for testing to be able to boot the same U-Boot (complete with
> > FSP) on bare metal and from coreboot. It allows checking of things like
> > CPU speed, comparing registers, ACPI tables and the like.
> >
> > This series allows U-Boot to detect that it ran from coreboot and
> > automatically do the right thing.
> >
> > This series makes the most important changes to allow the same u-boot.bin
> > for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL).
>
> I see only this and one patch out of 7 (seven?).
> Please, (re)send it correctly.

It looks like it went to the mailing list...are you subscribed?

Regards,
Simon

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Andy Shevchenko]

On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote:
> On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote:
> > > This little series adds a few checks into the code to allow better
> > > operation when booting a build from a previous-state loader such as
> > > coreboot.
> > >
> > > At present we have a 'coreboot' target but this runs very different code
> > > from the bare-metal targets, such as coral. There is very little in common
> > > between them.
> > >
> > > It is useful to be able to boot the same U-Boot on a device, with or
> > > without a first-stage bootloader. For example, with chromebook_coral, it
> > > is helpful for testing to be able to boot the same U-Boot (complete with
> > > FSP) on bare metal and from coreboot. It allows checking of things like
> > > CPU speed, comparing registers, ACPI tables and the like.
> > >
> > > This series allows U-Boot to detect that it ran from coreboot and
> > > automatically do the right thing.
> > >
> > > This series makes the most important changes to allow the same u-boot.bin
> > > for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL).
> >
> > I see only this and one patch out of 7 (seven?).
> > Please, (re)send it correctly.
> 
> It looks like it went to the mailing list...

> are you subscribed?

It doesn't matter if you want people to pay an attention (by putting them to Cc
list), so, since it was one patch I can't see the rest and know if they affect
or not the one, you Cc'ed me to.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Simon Glass]

Hi Andy,

On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote:
> > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote:
> > > > This little series adds a few checks into the code to allow better
> > > > operation when booting a build from a previous-state loader such as
> > > > coreboot.
> > > >
> > > > At present we have a 'coreboot' target but this runs very different code
> > > > from the bare-metal targets, such as coral. There is very little in common
> > > > between them.
> > > >
> > > > It is useful to be able to boot the same U-Boot on a device, with or
> > > > without a first-stage bootloader. For example, with chromebook_coral, it
> > > > is helpful for testing to be able to boot the same U-Boot (complete with
> > > > FSP) on bare metal and from coreboot. It allows checking of things like
> > > > CPU speed, comparing registers, ACPI tables and the like.
> > > >
> > > > This series allows U-Boot to detect that it ran from coreboot and
> > > > automatically do the right thing.
> > > >
> > > > This series makes the most important changes to allow the same u-boot.bin
> > > > for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL).
> > >
> > > I see only this and one patch out of 7 (seven?).
> > > Please, (re)send it correctly.
> >
> > It looks like it went to the mailing list...
>
> > are you subscribed?
>
> It doesn't matter if you want people to pay an attention (by putting them to Cc
> list), so, since it was one patch I can't see the rest and know if they affect
> or not the one, you Cc'ed me to.

I don't fully understand this, but are you saying you did not get the
patches in your email? If you subscribe to the U-Boot mailing list you
should receive all patches even if not cc'd to you.

I did not specifically cc you on this series but I can do that if I
send it again. But patman has added you to one patch (number 3) and
therefore you are also cc'd on the cover letter.

Can you check whether you are subscribed to the mailing list?

Regards,
Simon

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Andy Shevchenko]

On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote:
> On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote:
> > > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote:

...

> > > It looks like it went to the mailing list...
> >
> > > are you subscribed?
> >
> > It doesn't matter if you want people to pay an attention (by putting them to Cc
> > list), so, since it was one patch I can't see the rest and know if they affect
> > or not the one, you Cc'ed me to.
> 
> I don't fully understand this, but are you saying you did not get the
> patches in your email? If you subscribe to the U-Boot mailing list you
> should receive all patches even if not cc'd to you.

Probably I got to another email, not this one.

> I did not specifically cc you on this series but I can do that if I
> send it again. But patman has added you to one patch (number 3) and
> therefore you are also cc'd on the cover letter.

Yes, and this one I got without seeing full picture.

> Can you check whether you are subscribed to the mailing list?

As I said, I am not subscribed by email patman uses to Cc me.


Basically it's a bug in patman. Either it shouldn't tell me, or should Cc to
the entire series. Of course there are cases, when patches are independent and
no need to spam everybody with tons of emails, but this should be clarified in
cover letter. Did I miss that?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Igor Opaniuk]

Hi Andy

On Wed, Mar 11, 2020 at 3:04 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote:
> > On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote:
> > > > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote:
>
> ...
>
> > > > It looks like it went to the mailing list...
> > >
> > > > are you subscribed?
> > >
> > > It doesn't matter if you want people to pay an attention (by putting them to Cc
> > > list), so, since it was one patch I can't see the rest and know if they affect
> > > or not the one, you Cc'ed me to.
> >
> > I don't fully understand this, but are you saying you did not get the
> > patches in your email? If you subscribe to the U-Boot mailing list you
> > should receive all patches even if not cc'd to you.
>
> Probably I got to another email, not this one.
>
> > I did not specifically cc you on this series but I can do that if I
> > send it again. But patman has added you to one patch (number 3) and
> > therefore you are also cc'd on the cover letter.
>
> Yes, and this one I got without seeing full picture.
>
> > Can you check whether you are subscribed to the mailing list?
>
> As I said, I am not subscribed by email patman uses to Cc me.
>
>
> Basically it's a bug in patman. Either it shouldn't tell me, or should Cc to
> the entire series. Of course there are cases, when patches are independent and
> no need to spam everybody with tons of emails, but this should be clarified in
> cover letter. Did I miss that?

AFAIR, last time you were not happy about "irrelevant" patches that were CC-ed
to you "by mistake" [1]. This time, as I see, is vice-versa.
The algo is simple - if the change touches a file where you're marked as one
of commiters/maintainers - it will add your email to CC, otherwise it won't
(however you can still find the full list of patches in the ML, it takes
a couple of seconds only).

Unfortunately current state of tooling in U-Boot is not too good to be able
to read thoughts of people and guess their wishes.
But if you know how to improve that, patches are always welcome!

[1] https://patchwork.ozlabs.org/cover/1225901/

>
> --
> With Best Regards,
> Andy Shevchenko
>
>


-- 
Best regards - Freundliche Gr?sse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Andy Shevchenko]

On Wed, Mar 11, 2020 at 4:54 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
> On Wed, Mar 11, 2020 at 3:04 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote:
> > > On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote:
> > > > > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote:
> >
> > ...
> >
> > > > > It looks like it went to the mailing list...
> > > >
> > > > > are you subscribed?
> > > >
> > > > It doesn't matter if you want people to pay an attention (by putting them to Cc
> > > > list), so, since it was one patch I can't see the rest and know if they affect
> > > > or not the one, you Cc'ed me to.
> > >
> > > I don't fully understand this, but are you saying you did not get the
> > > patches in your email? If you subscribe to the U-Boot mailing list you
> > > should receive all patches even if not cc'd to you.
> >
> > Probably I got to another email, not this one.
> >
> > > I did not specifically cc you on this series but I can do that if I
> > > send it again. But patman has added you to one patch (number 3) and
> > > therefore you are also cc'd on the cover letter.
> >
> > Yes, and this one I got without seeing full picture.
> >
> > > Can you check whether you are subscribed to the mailing list?
> >
> > As I said, I am not subscribed by email patman uses to Cc me.
> >
> >
> > Basically it's a bug in patman. Either it shouldn't tell me, or should Cc to
> > the entire series. Of course there are cases, when patches are independent and
> > no need to spam everybody with tons of emails, but this should be clarified in
> > cover letter. Did I miss that?
>
> AFAIR, last time you were not happy about "irrelevant" patches that were CC-ed
> to you "by mistake" [1]. This time, as I see, is vice-versa.

Nope, it's not. It maybe that I missed cover letter to tell that the
rest is irrelevant to me. Otherwise Cc list is not filled correctly.

> The algo is simple - if the change touches a file where you're marked as one
> of commiters/maintainers - it will add your email to CC, otherwise it won't
> (however you can still find the full list of patches in the ML, it takes
> a couple of seconds only).

Yes, but here is obvious flaw. If the change is done in the file
dependent to the preparatory patch(es), then I need to be Cc'ed in all
relevant.

For example, patch 1 does introduce nice helper function foo(),
patches 2-(N-1) does change some irrelevant files, patch N touches my
stuff, I have to be Cc'ed to the 1) cover letter, 2) patch 1, 3) patch
N.

> Unfortunately current state of tooling in U-Boot is not too good to be able
> to read thoughts of people and guess their wishes.

See above. *Algo is simple*.

> But if you know how to improve that, patches are always welcome!
>
> [1] https://patchwork.ozlabs.org/cover/1225901/



-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Wolfgang Denk]

Dear Andy,
dear Igor,

In message <CAHp75Venz1ZSpfkK6werib1csKXLk=tzqA25TE4iJRdSOXGbHA@mail.gmail.com> you wrote:
>
> > Unfortunately current state of tooling in U-Boot is not too good to be able
> > to read thoughts of people and guess their wishes.
>
> See above. *Algo is simple*.

Come on, guys, in theory none of the Cc:s should be needed at all as
you get everything on the mailing list in first place; so the Cc: is
already a friendly reminder.  Getting one for a patch in a series
should be sufficient to identify the whole series without problems,
right?

So please, instead of fighting over such things rather invest the
energy in writing/reviewing code...

Thanks.

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Simon Glass]

Hi Andy,

On Wed, 11 Mar 2020 at 07:04, Andy Shevchenko <
andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote:
> > On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote:
> > > > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote:
>
> ...
>
> > > > It looks like it went to the mailing list...
> > >
> > > > are you subscribed?
> > >
> > > It doesn't matter if you want people to pay an attention (by putting
them to Cc
> > > list), so, since it was one patch I can't see the rest and know if
they affect
> > > or not the one, you Cc'ed me to.
> >
> > I don't fully understand this, but are you saying you did not get the
> > patches in your email? If you subscribe to the U-Boot mailing list you
> > should receive all patches even if not cc'd to you.
>
> Probably I got to another email, not this one.
>
> > I did not specifically cc you on this series but I can do that if I
> > send it again. But patman has added you to one patch (number 3) and
> > therefore you are also cc'd on the cover letter.
>
> Yes, and this one I got without seeing full picture.
>
> > Can you check whether you are subscribed to the mailing list?
>
> As I said, I am not subscribed by email patman uses to Cc me.

Please consider subscribing to the mailing list. You'll miss a lot of
things otherwise. You can add an email filter.

>
>
> Basically it's a bug in patman. Either it shouldn't tell me, or should Cc
to
> the entire series. Of course there are cases, when patches are
independent and
> no need to spam everybody with tons of emails, but this should be
clarified in
> cover letter. Did I miss that?

It is that latter case that happened here. A series may have various
patches that are relevant to different maintainers, so they want to see
just those patches and the cover letter (for context). Perhaps this doesn't
happen so often with Linux?

Regards,
Simon

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Andy Shevchenko]

On Wed, Mar 11, 2020 at 09:22:49PM -0600, Simon Glass wrote:
> On Wed, 11 Mar 2020 at 07:04, Andy Shevchenko <
> andriy.shevchenko at linux.intel.com> wrote:
> > On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote:
> > > On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:

...

> > > I don't fully understand this, but are you saying you did not get the
> > > patches in your email? If you subscribe to the U-Boot mailing list you
> > > should receive all patches even if not cc'd to you.
> >
> > Probably I got to another email, not this one.
> >
> > > I did not specifically cc you on this series but I can do that if I
> > > send it again. But patman has added you to one patch (number 3) and
> > > therefore you are also cc'd on the cover letter.
> >
> > Yes, and this one I got without seeing full picture.
> >
> > > Can you check whether you are subscribed to the mailing list?
> >
> > As I said, I am not subscribed by email patman uses to Cc me.
> 
> Please consider subscribing to the mailing list. You'll miss a lot of
> things otherwise. You can add an email filter.

As I told above, I have two addresses in use, and patman chose one, which
I'm not using for mailing list subscription.

> > Basically it's a bug in patman. Either it shouldn't tell me, or should Cc
> to
> > the entire series. Of course there are cases, when patches are
> independent and
> > no need to spam everybody with tons of emails, but this should be
> clarified in
> > cover letter. Did I miss that?
> 
> It is that latter case that happened here. A series may have various
> patches that are relevant to different maintainers, so they want to see
> just those patches and the cover letter (for context). Perhaps this doesn't
> happen so often with Linux?

It happens, but at least I can get the idea. I re-read the cover letter and
it sounds to me (in light of Edison chain loader) that I have to see entire
series.

Unfortunately my main priorities is not a U-Boot work, so, if you think it's
not breaking existing platforms, go ahead, we will notice breakage sooner or
later.

More important is what you are doing in your ACPI series, which I (slowly)
has been reviewing.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 0/7] x86: Improve support for chain-loading U-Boot

[Simon Glass]

Hi Bin,

On Sat, 7 Mar 2020 at 16:22, Simon Glass <sjg@chromium.org> wrote:
>
> This little series adds a few checks into the code to allow better
> operation when booting a build from a previous-state loader such as
> coreboot.
>
> At present we have a 'coreboot' target but this runs very different code
> from the bare-metal targets, such as coral. There is very little in common
> between them.
>
> It is useful to be able to boot the same U-Boot on a device, with or
> without a first-stage bootloader. For example, with chromebook_coral, it
> is helpful for testing to be able to boot the same U-Boot (complete with
> FSP) on bare metal and from coreboot. It allows checking of things like
> CPU speed, comparing registers, ACPI tables and the like.
>
> This series allows U-Boot to detect that it ran from coreboot and
> automatically do the right thing.
>
> This series makes the most important changes to allow the same u-boot.bin
> for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL).
>
> Changes in v3:
> - Add a new patch with a gd flag for chain loading
> - Add new patch to detect running from coreboot
>
> Changes in v2:
> - Drop the other check in interrupt_init() which is not needed now
> - Drop patch 'dm: Avoid initing built-in devices when chain loading'
>
> Simon Glass (7):
>   x86: fsp: Allow skipping init code when chain loading
>   x86: apl: Skip init code when chain loading
>   x86: cpu: Skip init code when chain loading
>   pci: Avoid auto-config when chain loading
>   board: Add a gd flag for chain loading
>   x86: Add a way to detect running from coreboot
>   x86: Use the existing stack when chain-loading
>
>  arch/x86/cpu/apollolake/fsp_s.c   |  2 ++
>  arch/x86/cpu/cpu.c                |  4 +++-
>  arch/x86/cpu/i386/cpu.c           | 15 +++++++++++++++
>  arch/x86/cpu/i386/interrupt.c     |  6 ++++--
>  arch/x86/cpu/start_from_spl.S     | 16 ++++++++++++++--
>  arch/x86/include/asm/u-boot-x86.h |  7 +++++++
>  arch/x86/lib/fsp/fsp_dram.c       |  8 ++++++++
>  arch/x86/lib/fsp/fsp_graphics.c   |  3 +++
>  arch/x86/lib/fsp2/fsp_dram.c      | 10 ++++++++++
>  arch/x86/lib/fsp2/fsp_init.c      |  2 +-
>  arch/x86/lib/init_helpers.c       |  3 +++
>  drivers/pci/pci-uclass.c          |  4 ++--
>  include/asm-generic/global_data.h |  1 +
>  include/init.h                    |  2 +-
>  14 files changed, 74 insertions(+), 9 deletions(-)
>
> --
> 2.25.1.481.gfbce0eb801-goog
>

Any comments on this series?

Regards,
Simon