[PATCH] spi: Add spi_get_bus_and_cs() new use_dt param

Patrice Chotard patrice.chotard at foss.st.com
Wed Jan 12 11:59:08 CET 2022


Add spi_flash_probe_bus_cs() and spi_get_bus_and_cs() new "use_dt"
param which allows to select SPI speed and mode from DT or from
default value passed in parameters.

Since commit e2e95e5e2542 ("spi: Update speed/mode on change")
when calling "sf probe" or "env save" on SPI flash,
spi_set_speed_mode() is called twice.

spi_get_bus_and_cs()
      |--> spi_claim_bus()
      |       |--> spi_set_speed_mode(speed and mode from DT)
      ...
      |--> spi_set_speed_mode(default speed and mode value)

The first spi_set_speed_mode() call is done with speed and mode
values from DT, whereas the second call is done with speed
and mode set to default value (speed is set to CONFIG_SF_DEFAULT_SPEED)

This is an issue because SPI flash performance are impacted by
using default speed which can be lower than the one defined in DT.

One solution is to set CONFIG_SF_DEFAULT_SPEED to the speed defined
in DT, but we loose flexibility offered by DT.

Another issue can be encountered with 2 SPI flashes using 2 different
speeds. In this specific case usage of CONFIG_SF_DEFAULT_SPEED is not
flexible compared to get the 2 different speeds from DT.

By adding new parameter "use_dt" to spi_flash_probe_bus_cs() and to
spi_get_bus_and_cs(), this allows to force usage of either speed and
mode from DT (use_dt = true) or to use speed and mode parameter value.

Signed-off-by: Patrice Chotard <patrice.chotard at foss.st.com>
Cc: Marek Behun <marek.behun at nic.cz>
Cc: Jagan Teki <jagan at amarulasolutions.com>
Cc: Vignesh R <vigneshr at ti.com>
Cc: Joe Hershberger <joe.hershberger at ni.com>
Cc: Ramon Fried <rfried.dev at gmail.com>
Cc: Lukasz Majewski <lukma at denx.de>
Cc: Marek Vasut <marex at denx.de>
Cc: Wolfgang Denk <wd at denx.de>
Cc: Simon Glass <sjg at chromium.org>
Cc: Stefan Roese <sr at denx.de>
Cc: "Pali Rohár" <pali at kernel.org>
Cc: Konstantin Porotchkin <kostap at marvell.com>
Cc: Igal Liberman <igall at marvell.com>
Cc: Bin Meng <bmeng.cn at gmail.com>
Cc: Pratyush Yadav <p.yadav at ti.com>
Cc: Sean Anderson <seanga2 at gmail.com>
Cc: Anji J <anji.jagarlmudi at nxp.com>
Cc: Biwen Li <biwen.li at nxp.com>
Cc: Priyanka Jain <priyanka.jain at nxp.com>
Cc: Chaitanya Sakinam <chaitanya.sakinam at nxp.com>
---

 board/CZ.NIC/turris_mox/turris_mox.c |  2 +-
 cmd/sf.c                             |  5 ++++-
 cmd/spi.c                            |  2 +-
 drivers/mtd/spi/sf-uclass.c          |  8 ++++----
 drivers/net/fm/fm.c                  |  5 +++--
 drivers/net/pfe_eth/pfe_firmware.c   |  2 +-
 drivers/net/sni_netsec.c             |  3 ++-
 drivers/spi/spi-uclass.c             |  8 ++++----
 drivers/usb/gadget/max3420_udc.c     |  2 +-
 env/sf.c                             |  2 +-
 include/spi.h                        |  9 +++++----
 include/spi_flash.h                  |  2 +-
 test/dm/spi.c                        | 15 ++++++++-------
 13 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
index 2202eb8cfb..363edc115f 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -135,7 +135,7 @@ static int mox_do_spi(u8 *in, u8 *out, size_t size)
 	struct udevice *dev;
 	int ret;
 
-	ret = spi_get_bus_and_cs(0, 1, 1000000, SPI_CPHA | SPI_CPOL,
+	ret = spi_get_bus_and_cs(0, 1, 1000000, SPI_CPHA | SPI_CPOL, false,
 				 "spi_generic_drv", "moxtet at 1", &dev,
 				 &slave);
 	if (ret)
diff --git a/cmd/sf.c b/cmd/sf.c
index eac27ed2d7..0c1ddbbab7 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -91,6 +91,7 @@ static int do_spi_flash_probe(int argc, char *const argv[])
 	unsigned int speed = CONFIG_SF_DEFAULT_SPEED;
 	unsigned int mode = CONFIG_SF_DEFAULT_MODE;
 	char *endp;
+	bool use_dt = true;
 #if CONFIG_IS_ENABLED(DM_SPI_FLASH)
 	struct udevice *new, *bus_dev;
 	int ret;
@@ -117,11 +118,13 @@ static int do_spi_flash_probe(int argc, char *const argv[])
 		speed = simple_strtoul(argv[2], &endp, 0);
 		if (*argv[2] == 0 || *endp != 0)
 			return -1;
+		use_dt = false;
 	}
 	if (argc >= 4) {
 		mode = hextoul(argv[3], &endp);
 		if (*argv[3] == 0 || *endp != 0)
 			return -1;
+		use_dt = false;
 	}
 
 #if CONFIG_IS_ENABLED(DM_SPI_FLASH)
@@ -131,7 +134,7 @@ static int do_spi_flash_probe(int argc, char *const argv[])
 		device_remove(new, DM_REMOVE_NORMAL);
 	}
 	flash = NULL;
-	ret = spi_flash_probe_bus_cs(bus, cs, speed, mode, &new);
+	ret = spi_flash_probe_bus_cs(bus, cs, speed, mode, use_dt, &new);
 	if (ret) {
 		printf("Failed to initialize SPI flash at %u:%u (error %d)\n",
 		       bus, cs, ret);
diff --git a/cmd/spi.c b/cmd/spi.c
index 6dc32678da..46bd817e60 100644
--- a/cmd/spi.c
+++ b/cmd/spi.c
@@ -46,7 +46,7 @@ static int do_spi_xfer(int bus, int cs)
 	str = strdup(name);
 	if (!str)
 		return -ENOMEM;
-	ret = spi_get_bus_and_cs(bus, cs, freq, mode, "spi_generic_drv",
+	ret = spi_get_bus_and_cs(bus, cs, freq, mode, false, "spi_generic_drv",
 				 str, &dev, &slave);
 	if (ret)
 		return ret;
diff --git a/drivers/mtd/spi/sf-uclass.c b/drivers/mtd/spi/sf-uclass.c
index 63d16291ff..ef3e3bb9de 100644
--- a/drivers/mtd/spi/sf-uclass.c
+++ b/drivers/mtd/spi/sf-uclass.c
@@ -51,7 +51,7 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
 {
 	struct udevice *dev;
 
-	if (spi_flash_probe_bus_cs(bus, cs, max_hz, spi_mode, &dev))
+	if (spi_flash_probe_bus_cs(bus, cs, max_hz, spi_mode, false, &dev))
 		return NULL;
 
 	return dev_get_uclass_priv(dev);
@@ -59,7 +59,7 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
 
 int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs,
 			   unsigned int max_hz, unsigned int spi_mode,
-			   struct udevice **devp)
+			   bool use_dt, struct udevice **devp)
 {
 	struct spi_slave *slave;
 	struct udevice *bus;
@@ -74,8 +74,8 @@ int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs,
 	snprintf(name, sizeof(name), "spi_flash@%d:%d", busnum, cs);
 	str = strdup(name);
 #endif
-	ret = spi_get_bus_and_cs(busnum, cs, max_hz, spi_mode,
-				  "jedec_spi_nor", str, &bus, &slave);
+	ret = spi_get_bus_and_cs(busnum, cs, max_hz, spi_mode, use_dt,
+				 "jedec_spi_nor", str, &bus, &slave);
 	if (ret)
 		return ret;
 
diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c
index 7d51be1f72..c56afc6a47 100644
--- a/drivers/net/fm/fm.c
+++ b/drivers/net/fm/fm.c
@@ -388,7 +388,8 @@ int fm_init_common(int index, struct ccsr_fman *reg)
 
 		/* speed and mode will be read from DT */
 		ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS,
-					     CONFIG_ENV_SPI_CS, 0, 0, &new);
+					     CONFIG_ENV_SPI_CS, 0, 0, true,
+					     &new);
 
 		ucode_flash = dev_get_uclass_priv(new);
 #else
@@ -475,7 +476,7 @@ int fm_init_common(int index, struct ccsr_fman *reg)
 
 	/* speed and mode will be read from DT */
 	ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
-				     0, 0, &new);
+				     0, 0, true, &new);
 
 	ucode_flash = dev_get_uclass_priv(new);
 #else
diff --git a/drivers/net/pfe_eth/pfe_firmware.c b/drivers/net/pfe_eth/pfe_firmware.c
index ad5bc3c862..d30991571c 100644
--- a/drivers/net/pfe_eth/pfe_firmware.c
+++ b/drivers/net/pfe_eth/pfe_firmware.c
@@ -183,7 +183,7 @@ int pfe_spi_flash_init(void)
 				     CONFIG_ENV_SPI_CS,
 				     CONFIG_ENV_SPI_MAX_HZ,
 				     CONFIG_ENV_SPI_MODE,
-				     &new);
+				     true, &new);
 	if (ret) {
 		printf("SF: failed to probe spi\n");
 		free(addr);
diff --git a/drivers/net/sni_netsec.c b/drivers/net/sni_netsec.c
index 4901321d0c..6d00a3fd3a 100644
--- a/drivers/net/sni_netsec.c
+++ b/drivers/net/sni_netsec.c
@@ -625,7 +625,8 @@ static void netsec_spi_read(char *buf, loff_t len, loff_t offset)
 	struct spi_flash *flash;
 
 	spi_flash_probe_bus_cs(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
-			       CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE, &new);
+			       CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE,
+			       true, &new);
 	flash = dev_get_uclass_priv(new);
 
 	spi_flash_read(flash, offset, len, buf);
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index f8ec312d71..4366124e3c 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -340,7 +340,7 @@ int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp,
 	return ret;
 }
 
-int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
+int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, bool use_dt,
 		       const char *drv_name, const char *dev_name,
 		       struct udevice **busp, struct spi_slave **devp)
 {
@@ -419,8 +419,8 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
 	}
 
 	/* In case bus frequency or mode changed, update it. */
-	if ((speed && bus_data->speed && bus_data->speed != speed) ||
-	    (plat && plat->mode != mode)) {
+	if (((speed && bus_data->speed && bus_data->speed != speed) ||
+	     (plat && plat->mode != mode)) && !use_dt) {
 		ret = spi_set_speed_mode(bus, speed, mode);
 		if (ret)
 			goto err_speed_mode;
@@ -453,7 +453,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, unsigned int cs,
 	struct udevice *dev;
 	int ret;
 
-	ret = spi_get_bus_and_cs(busnum, cs, speed, mode, NULL, 0, &dev,
+	ret = spi_get_bus_and_cs(busnum, cs, speed, mode, false, NULL, 0, &dev,
 				 &slave);
 	if (ret)
 		return NULL;
diff --git a/drivers/usb/gadget/max3420_udc.c b/drivers/usb/gadget/max3420_udc.c
index a16095f892..ccec337f99 100644
--- a/drivers/usb/gadget/max3420_udc.c
+++ b/drivers/usb/gadget/max3420_udc.c
@@ -830,7 +830,7 @@ static int max3420_udc_probe(struct udevice *dev)
 	cs = slave_pdata->cs;
 	speed = slave_pdata->max_hz;
 	mode = slave_pdata->mode;
-	spi_get_bus_and_cs(busnum, cs, speed, mode, "spi_generic_drv",
+	spi_get_bus_and_cs(busnum, cs, speed, mode, false, "spi_generic_drv",
 			   NULL, &spid, &udc->slave);
 
 	udc->dev = dev;
diff --git a/env/sf.c b/env/sf.c
index 6a4bb756f0..ed6bfb2115 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -53,7 +53,7 @@ static int setup_flash_device(struct spi_flash **env_flash)
 	/* speed and mode will be read from DT */
 	ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
 				     CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE,
-				     &new);
+				     true, &new);
 	if (ret) {
 		env_set_default("spi_flash_probe_bus_cs() failed", 0);
 		return ret;
diff --git a/include/spi.h b/include/spi.h
index dc3b21132a..5d12f27b19 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -582,15 +582,16 @@ int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp,
  * @cs:		Chip select to look for
  * @speed:	SPI speed to use for this slave when not available in plat
  * @mode:	SPI mode to use for this slave when not available in plat
+ * @use_dt:     Force usage of SPI speed and SPI mode from DT
  * @drv_name:	Name of driver to attach to this chip select
  * @dev_name:	Name of the new device thus created
  * @busp:	Returns bus device
  * @devp:	Return slave device
- * @return 0 if found, -ve on error
+  * @return 0 if found, -ve on error
  */
-int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
-			const char *drv_name, const char *dev_name,
-			struct udevice **busp, struct spi_slave **devp);
+int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, bool use_dt,
+		       const char *drv_name, const char *dev_name,
+		       struct udevice **busp, struct spi_slave **devp);
 
 /**
  * spi_chip_select() - Get the chip select for a slave
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 4d4ae89c19..07bca1ee3b 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -117,7 +117,7 @@ int spi_flash_std_probe(struct udevice *dev);
 
 int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs,
 			   unsigned int max_hz, unsigned int spi_mode,
-			   struct udevice **devp);
+			   bool use_dt, struct udevice **devp);
 
 /* Compatibility function - this is the old U-Boot API */
 struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
diff --git a/test/dm/spi.c b/test/dm/spi.c
index ee4ad3abaa..2b1f5535c6 100644
--- a/test/dm/spi.c
+++ b/test/dm/spi.c
@@ -47,7 +47,7 @@ static int dm_test_spi_find(struct unit_test_state *uts)
 	/* This finds nothing because we removed the device */
 	ut_asserteq(-ENODEV, spi_find_bus_and_cs(busnum, cs, &bus, &dev));
 	ut_asserteq(-ENODEV, spi_get_bus_and_cs(busnum, cs, speed, mode,
-						NULL, 0, &bus, &slave));
+						false, NULL, 0, &bus, &slave));
 
 	/*
 	 * This forces the device to be re-added, but there is no emulation
@@ -56,7 +56,7 @@ static int dm_test_spi_find(struct unit_test_state *uts)
 	 * a 'partially-inited' device.
 	 */
 	ut_asserteq(-ENODEV, spi_find_bus_and_cs(busnum, cs, &bus, &dev));
-	ut_asserteq(-ENOENT, spi_get_bus_and_cs(busnum, cs, speed, mode,
+	ut_asserteq(-ENOENT, spi_get_bus_and_cs(busnum, cs, speed, mode, false,
 						"jedec_spi_nor", "name", &bus,
 						&slave));
 	sandbox_sf_unbind_emul(state_get_current(), busnum, cs);
@@ -67,7 +67,7 @@ static int dm_test_spi_find(struct unit_test_state *uts)
 	ut_assertok(sandbox_sf_bind_emul(state, busnum, cs, bus, node,
 					 "name"));
 	ut_assertok(spi_find_bus_and_cs(busnum, cs, &bus, &dev));
-	ut_assertok(spi_get_bus_and_cs(busnum, cs, speed, mode,
+	ut_assertok(spi_get_bus_and_cs(busnum, cs, speed, mode, false,
 				       "jedec_spi_nor", "name", &bus, &slave));
 
 	ut_assertok(spi_cs_info(bus, cs, &info));
@@ -77,7 +77,8 @@ static int dm_test_spi_find(struct unit_test_state *uts)
 	ut_assertok(sandbox_sf_bind_emul(state, busnum, cs_b, bus, node,
 					 "name"));
 	ut_asserteq(-EINVAL, spi_get_bus_and_cs(busnum, cs_b, speed, mode,
-				       "jedec_spi_nor", "name", &bus, &slave));
+						false, "jedec_spi_nor", "name",
+						&bus, &slave));
 	ut_asserteq(-EINVAL, spi_cs_info(bus, cs_b, &info));
 	ut_asserteq_ptr(NULL, info.dev);
 
@@ -145,10 +146,10 @@ static int dm_test_spi_claim_bus(struct unit_test_state *uts)
 	const int busnum = 0, cs_a = 0, cs_b = 1, mode = 0;
 
 	/* Get spi slave on CS0 */
-	ut_assertok(spi_get_bus_and_cs(busnum, cs_a, 1000000, mode, NULL, 0,
+	ut_assertok(spi_get_bus_and_cs(busnum, cs_a, 1000000, mode, false, NULL, 0,
 				       &bus, &slave_a));
 	/* Get spi slave on CS1 */
-	ut_assertok(spi_get_bus_and_cs(busnum, cs_b, 1000000, mode, NULL, 0,
+	ut_assertok(spi_get_bus_and_cs(busnum, cs_b, 1000000, mode, false, NULL, 0,
 				       &bus, &slave_b));
 
 	/* Different max_hz, different mode. */
@@ -182,7 +183,7 @@ static int dm_test_spi_xfer(struct unit_test_state *uts)
 	const char dout[5] = {0x9f};
 	unsigned char din[5];
 
-	ut_assertok(spi_get_bus_and_cs(busnum, cs, 1000000, mode, NULL, 0,
+	ut_assertok(spi_get_bus_and_cs(busnum, cs, 1000000, mode, false, NULL, 0,
 				       &bus, &slave));
 	ut_assertok(spi_claim_bus(slave));
 	ut_assertok(spi_xfer(slave, 40, dout, din,
-- 
2.17.1



More information about the U-Boot mailing list