[PATCH 0/3] x86: Correct SPI memory-mapping query

[PATCH 0/3] x86: Correct SPI memory-mapping query

[Simon Glass]

This little series makes the SPI memory-mapping query work on all current
x86 boards where possible, returning an error code (rather than hanging)
when it fails.

It replaces the previous patch at [1]. Unfortunately it is quite a bit
more complicated.

This fixes booting on link and samus.

[1] http://patchwork.ozlabs.org/project/uboot/patch/20200324074524.1.Ibc9c511db58caa8a1e4c56d7e7824d7690718aeb at changeid/


Simon Glass (3):
  x86: spi: Add a way to access the SPI mapping via registers
  x86: spi: Rewrite logic for obtaining the SPI memory map
  x86: spl: Print the error on SPL failure

 arch/x86/cpu/intel_common/fast_spi.c |  19 +++--
 arch/x86/include/asm/fast_spi.h      |  19 +++++
 arch/x86/lib/spl.c                   |   4 +-
 drivers/spi/ich.c                    | 103 +++++++++++++++++++++++----
 4 files changed, 123 insertions(+), 22 deletions(-)

-- 
2.26.2.761.g0e0b3e54be-goog

[PATCH 1/3] x86: spi: Add a way to access the SPI mapping via registers

[Simon Glass]

At present the PCI BDF (bus/device/function) is needed to access the SPI
mapping, since the registers are at BAR0. This doesn't work when PCI
auto-config has not been done yet, since BARs are unassigned.

Add another way to find the mapping, using the MMIO base, if the caller
knows this.

Also add a missing function comment.

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

 arch/x86/cpu/intel_common/fast_spi.c | 19 ++++++++++++++-----
 arch/x86/include/asm/fast_spi.h      | 19 +++++++++++++++++++
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/arch/x86/cpu/intel_common/fast_spi.c b/arch/x86/cpu/intel_common/fast_spi.c
index a6e3d0a5bf..5d3944dee2 100644
--- a/arch/x86/cpu/intel_common/fast_spi.c
+++ b/arch/x86/cpu/intel_common/fast_spi.c
@@ -31,21 +31,30 @@ static ulong fast_spi_get_bios_region(struct fast_spi_regs *regs,
 	return bios_start;
 }
 
+int fast_spi_get_bios_mmap_regs(struct fast_spi_regs *regs, ulong *map_basep,
+				uint *map_sizep, uint *offsetp)
+{
+	ulong base;
+
+	base = fast_spi_get_bios_region(regs, map_sizep);
+	*map_basep = (u32)-*map_sizep - base;
+	*offsetp = base;
+
+	return 0;
+}
+
 int fast_spi_get_bios_mmap(pci_dev_t pdev, ulong *map_basep, uint *map_sizep,
 			   uint *offsetp)
 {
 	struct fast_spi_regs *regs;
-	ulong bar, base, mmio_base;
+	ulong bar, mmio_base;
 
 	/* Special case to find mapping without probing the device */
 	pci_x86_read_config(pdev, PCI_BASE_ADDRESS_0, &bar, PCI_SIZE_32);
 	mmio_base = bar & PCI_BASE_ADDRESS_MEM_MASK;
 	regs = (struct fast_spi_regs *)mmio_base;
-	base = fast_spi_get_bios_region(regs, map_sizep);
-	*map_basep = (u32)-*map_sizep - base;
-	*offsetp = base;
 
-	return 0;
+	return fast_spi_get_bios_mmap_regs(regs, map_basep, map_sizep, offsetp);
 }
 
 int fast_spi_early_init(pci_dev_t pdev, ulong mmio_base)
diff --git a/arch/x86/include/asm/fast_spi.h b/arch/x86/include/asm/fast_spi.h
index 6894298526..56c680d800 100644
--- a/arch/x86/include/asm/fast_spi.h
+++ b/arch/x86/include/asm/fast_spi.h
@@ -63,6 +63,25 @@ check_member(fast_spi_regs, ptdata, 0xd0);
 int fast_spi_get_bios_mmap(pci_dev_t pdev, ulong *map_basep, uint *map_sizep,
 			   uint *offsetp);
 
+/**
+ * fast_spi_get_bios_mmap_regs() - Get memory map for SPI flash given regs
+ *
+ * @regs:	SPI registers to use
+ * @map_basep:	Returns base memory address for mapped SPI
+ * @map_sizep:	Returns size of mapped SPI
+ * @offsetp:	Returns start offset of SPI flash where the map works
+ *	correctly (offsets before this are not visible)
+ * @return 0 (always)
+ */
+int fast_spi_get_bios_mmap_regs(struct fast_spi_regs *regs, ulong *map_basep,
+				uint *map_sizep, uint *offsetp);
+
+/**
+ * fast_spi_early_init() - Set up a BAR to use SPI early in U-Boot
+ *
+ * @pdev:	PCI device to use (this is the Fast SPI device)
+ * @mmio_base:	MMIO base to use to access registers
+ */
 int fast_spi_early_init(pci_dev_t pdev, ulong mmio_base);
 
 #endif	/* ASM_FAST_SPI_H */
-- 
2.26.2.761.g0e0b3e54be-goog

[PATCH 2/3] x86: spi: Rewrite logic for obtaining the SPI memory map

[Simon Glass]

At present this logic does not work on link and samus, since their SPI
controller is not a PCI device, but a child of the PCH.

Unfortunately, fixing this involves a lot of extra logic. Still, this was
requested in the review of the fix-up patch, so here it is.

Signed-off-by: Simon Glass <sjg@chromium.org>
Fixes: 92842147c31 ("spi: ich: Add support for get_mmap() method")
---

 drivers/spi/ich.c | 103 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 88 insertions(+), 15 deletions(-)

diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
index a9d7715a55..195200c4ca 100644
--- a/drivers/spi/ich.c
+++ b/drivers/spi/ich.c
@@ -23,6 +23,7 @@
 #include <asm/fast_spi.h>
 #include <asm/io.h>
 #include <asm/mtrr.h>
+#include <dm/uclass-internal.h>
 #include <linux/sizes.h>
 
 #include "ich.h"
@@ -610,15 +611,94 @@ static int ich_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	return ret;
 }
 
+/**
+ * ich_spi_get_basics() - Get basic information about the ICH device
+ *
+ * This works without probing any devices if requested.
+ *
+ * @bus: SPI controller to use
+ * @can_probe: true if this function is allowed to probe the PCH
+ * @pchp: Returns a pointer to the pch, or NULL if not found
+ * @ich_versionp: Returns ICH version detected on success
+ * @mmio_basep: Returns the address of the SPI registers on success
+ * @return 0 if OK, -EPROTOTYPE if the PCH could not be found, -EAGAIN if
+ *	the function cannot success without probing, possible another error if
+ *	pch_get_spi_base() fails
+ */
+static int ich_spi_get_basics(struct udevice *bus, bool can_probe,
+			      struct udevice **pchp,
+			      enum ich_version *ich_versionp, ulong *mmio_basep)
+{
+	struct udevice *pch = NULL;
+	int ret = 0;
+
+	/* Find a PCH if there is one */
+	if (can_probe) {
+		pch = dev_get_parent(bus);
+		if (device_get_uclass_id(pch) != UCLASS_PCH) {
+			uclass_first_device(UCLASS_PCH, &pch);
+			if (!pch)
+				return log_msg_ret("uclass", -EPROTOTYPE);
+		}
+	}
+
+	*ich_versionp = dev_get_driver_data(bus);
+	if (*ich_versionp == ICHV_APL)
+		*mmio_basep = dm_pci_read_bar32(bus, 0);
+	else if (pch)
+		ret = pch_get_spi_base(pch, mmio_basep);
+	else
+		return -EAGAIN;
+	*pchp = pch;
+
+	return ret;
+}
+
+/**
+ * ich_get_mmap_bus() - Handle the get_mmap() method for a bus
+ *
+ * There are several cases to consider:
+ * 1. Using of-platdata, in which case we have the BDF and can access the
+ *	registers by reading the BAR
+ * 2. Not using of-platdata, but still with a SPI controller that is on its own
+ * PCI PDF. In this case we read the BFF from the parent platdata and again get
+ *	the registers by reading the BAR
+ * 3. Using a SPI controller that is a child of the PCH, in which case we try
+ *	to find the registers by asking the PCH. This only works if the PCH has
+ *	been probed (which it will be if the bus is probed since parents are
+ *	probed before children), since the PCH may not have a PCI address until
+ *	its parent (the PCI bus itself) has been probed. If you are using this
+ *	method then you should make sure the SPI bus is probed.
+ *
+ * The first two cases are useful in early init. The last one is more useful
+ * afterwards.
+ */
 static int ich_get_mmap_bus(struct udevice *bus, ulong *map_basep,
 			    uint *map_sizep, uint *offsetp)
 {
 	pci_dev_t spi_bdf;
-
 #if !CONFIG_IS_ENABLED(OF_PLATDATA)
-	struct pci_child_platdata *pplat = dev_get_parent_platdata(bus);
+	if (device_is_on_pci_bus(bus)) {
+		struct pci_child_platdata *pplat;
+
+		pplat = dev_get_parent_platdata(bus);
+		spi_bdf = pplat->devfn;
+	} else {
+		enum ich_version ich_version;
+		struct fast_spi_regs *regs;
+		struct udevice *pch;
+		ulong mmio_base;
+		int ret;
 
-	spi_bdf = pplat->devfn;
+		ret = ich_spi_get_basics(bus, device_active(bus), &pch,
+					 &ich_version, &mmio_base);
+		if (ret)
+			return log_msg_ret("basics", ret);
+		regs = (struct fast_spi_regs *)mmio_base;
+
+		return fast_spi_get_bios_mmap_regs(regs, map_basep, map_sizep,
+						   offsetp);
+	}
 #else
 	struct ich_spi_platdata *plat = dev_get_platdata(bus);
 
@@ -862,23 +942,16 @@ static int ich_spi_child_pre_probe(struct udevice *dev)
 static int ich_spi_ofdata_to_platdata(struct udevice *dev)
 {
 	struct ich_spi_platdata *plat = dev_get_platdata(dev);
+	int ret;
 
 #if !CONFIG_IS_ENABLED(OF_PLATDATA)
 	struct ich_spi_priv *priv = dev_get_priv(dev);
 
-	/* Find a PCH if there is one */
-	uclass_first_device(UCLASS_PCH, &priv->pch);
-	if (!priv->pch)
-		priv->pch = dev_get_parent(dev);
-
-	plat->ich_version = dev_get_driver_data(dev);
+	ret = ich_spi_get_basics(dev, true, &priv->pch, &plat->ich_version,
+				 &plat->mmio_base);
+	if (ret)
+		return log_msg_ret("basics", ret);
 	plat->lockdown = dev_read_bool(dev, "intel,spi-lock-down");
-	if (plat->ich_version == ICHV_APL) {
-		plat->mmio_base = dm_pci_read_bar32(dev, 0);
-	} else  {
-		/* SBASE is similar */
-		pch_get_spi_base(priv->pch, &plat->mmio_base);
-	}
 	/*
 	 * Use an int so that the property is present in of-platdata even
 	 * when false.
-- 
2.26.2.761.g0e0b3e54be-goog

[PATCH 3/3] x86: spl: Print the error on SPL failure

[Simon Glass]

The error code is often useful to figure out what is going on. Printing it
does not increase code size much, so print out the error and then hang.

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

 arch/x86/lib/spl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index 212b4d596d..a533dfa8b3 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -161,8 +161,8 @@ void board_init_f(ulong flags)
 
 	ret = x86_spl_init();
 	if (ret) {
-		debug("Error %d\n", ret);
-		panic("x86_spl_init fail");
+		printf("x86_spl_init: error %d\n", ret);
+		hang();
 	}
 #if IS_ENABLED(CONFIG_TPL) || IS_ENABLED(CONFIG_SYS_COREBOOT)
 	gd->bd = malloc(sizeof(*gd->bd));
-- 
2.26.2.761.g0e0b3e54be-goog

[PATCH 2/3] x86: spi: Rewrite logic for obtaining the SPI memory map

[Bin Meng]

Hi Simon,

On Mon, May 18, 2020 at 11:01 AM Simon Glass <sjg@chromium.org> wrote:
>
> At present this logic does not work on link and samus, since their SPI
> controller is not a PCI device, but a child of the PCH.
>
> Unfortunately, fixing this involves a lot of extra logic. Still, this was
> requested in the review of the fix-up patch, so here it is.

This patch still does not fix the issue completely.

In mrccache_get_region() the call to
uclass_find_first_device(UCLASS_SPI_FLASH, &dev) finds nothing. The
commit changed the behavior is this one:

commit 87f1084a630e6dbd5ba9a9747ce185d98ed40658
Author: Simon Glass <sjg@chromium.org>
Date:   Fri Dec 6 21:42:03 2019 -0700

    x86: Adjust mrccache_get_region() to use livetree

    Change the algorithm to first find the flash device then read the
    properties using the livetree API. With this change the device is not
    probed so this needs to be done in mrccache_save().

Previously fdtdec APIs are used to get the MRC cache range, but now
since it switches to live tree, a dev node has to be passed hence the
call to uclass_find_first_device(UCLASS_SPI_FLASH, &dev) but at this
point nothing is probed.

>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Fixes: 92842147c31 ("spi: ich: Add support for get_mmap() method")
> ---
>
>  drivers/spi/ich.c | 103 +++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 88 insertions(+), 15 deletions(-)
>

Regards,
Bin

[PATCH 2/3] x86: spi: Rewrite logic for obtaining the SPI memory map

[Simon Glass]

Hi Bin,

On Tue, 26 May 2020 at 04:08, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, May 18, 2020 at 11:01 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > At present this logic does not work on link and samus, since their SPI
> > controller is not a PCI device, but a child of the PCH.
> >
> > Unfortunately, fixing this involves a lot of extra logic. Still, this was
> > requested in the review of the fix-up patch, so here it is.
>
> This patch still does not fix the issue completely.
>
> In mrccache_get_region() the call to
> uclass_find_first_device(UCLASS_SPI_FLASH, &dev) finds nothing. The
> commit changed the behavior is this one:
>
> commit 87f1084a630e6dbd5ba9a9747ce185d98ed40658
> Author: Simon Glass <sjg@chromium.org>
> Date:   Fri Dec 6 21:42:03 2019 -0700
>
>     x86: Adjust mrccache_get_region() to use livetree
>
>     Change the algorithm to first find the flash device then read the
>     properties using the livetree API. With this change the device is not
>     probed so this needs to be done in mrccache_save().
>
> Previously fdtdec APIs are used to get the MRC cache range, but now
> since it switches to live tree, a dev node has to be passed hence the
> call to uclass_find_first_device(UCLASS_SPI_FLASH, &dev) but at this
> point nothing is probed.

OK I see. I didn't notice that due to SPI-flash problems on minnowmax.
I sent a patch for that as well.

Have sent a v2 series with an additional patch to restore the old behaviour.

Regards,
Simon