diff mbox series

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

Message ID 20200518030059.80935-3-sjg@chromium.org
State Superseded
Headers show
Series x86: Correct SPI memory-mapping query | expand

Commit Message

Simon Glass May 18, 2020, 3 a.m. UTC
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 at chromium.org>
Fixes: 92842147c31 ("spi: ich: Add support for get_mmap() method")
---

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

Comments

Bin Meng May 26, 2020, 10:08 a.m. UTC | #1
Hi Simon,

On Mon, May 18, 2020 at 11:01 AM Simon Glass <sjg at 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 at 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 at 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
Simon Glass May 27, 2020, 12:59 p.m. UTC | #2
Hi Bin,

On Tue, 26 May 2020 at 04:08, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, May 18, 2020 at 11:01 AM Simon Glass <sjg at 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 at 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
diff mbox series

Patch

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.