All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH v4 0/3] spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode
@ 2022-05-18  6:46 Patrice Chotard
  2022-05-18  6:46 ` [RESEND PATCH v4 1/3] spi: spi-uclass: Add new spi_get_bus_and_cs() implementation Patrice Chotard
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Patrice Chotard @ 2022-05-18  6:46 UTC (permalink / raw)
  To: u-boot
  Cc: Patrice CHOTARD, Patrick DELAUNAY, U-Boot STM32, Anji J,
	Biwen Li, Chaitanya Sakinam, Heinrich Schuchardt, Jagan Teki,
	Jassi Brar, Joe Hershberger, Lukasz Majewski, Marek Behún,
	Marek Vasut, Pratyush Yadav, Priyanka Jain, Ramon Fried,
	Simon Glass, Vignesh R, Wolfgang Denk


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.


Changes in v4:
  - Split previous series in 3 patches
  - Update commit message with additionnal information

Changes in v3:
  - Update commit header to reflect what the patch really do.
  - Rename legacy spi_get_bus_and_cs() to _spi_get_bus_and_cs().
  - New spi_get_bus_and_cs() rely on DT for spi speed and mode values.
  - spi_flash_probe_bus_cs() rely also on DT for spi and mode values.

Changes in v2:
  - add spi_flash_probe_bus_cs_default() which calls spi_get_bus_and_cs()
    with "use_dt" param set to true, whereas spi_flash_probe_bus_cs() calls
    spi_get_bus_and_cs() with "use_dt" param set to true.

Patrice Chotard (3):
  spi: spi-uclass: Add new spi_get_bus_and_cs() implementation
  spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode
  test: dm: spi: Replace _spi_get_bus_and_cs() by spi_get_bus_and_cs()
    in some case

 board/CZ.NIC/turris_mox/turris_mox.c |  6 +--
 cmd/sf.c                             | 15 +++++--
 cmd/spi.c                            |  4 +-
 drivers/mtd/spi/sf-uclass.c          | 33 +++++++-------
 drivers/net/fm/fm.c                  |  4 +-
 drivers/net/pfe_eth/pfe_firmware.c   | 19 ++------
 drivers/net/sni_netsec.c             |  6 +--
 drivers/spi/spi-uclass.c             | 66 +++++++++++++++++++++++++---
 drivers/usb/gadget/max3420_udc.c     |  4 +-
 env/sf.c                             |  1 -
 include/spi.h                        | 19 +++++++-
 include/spi_flash.h                  |  1 -
 test/dm/spi.c                        | 33 +++++++-------
 13 files changed, 137 insertions(+), 74 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [RESEND PATCH v4 1/3] spi: spi-uclass: Add new spi_get_bus_and_cs() implementation
  2022-05-18  6:46 [RESEND PATCH v4 0/3] spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode Patrice Chotard
@ 2022-05-18  6:46 ` Patrice Chotard
  2022-05-18  6:46 ` [RESEND PATCH v4 2/3] spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode Patrice Chotard
  2022-05-18  6:46 ` [RESEND PATCH v4 3/3] test: dm: spi: Replace _spi_get_bus_and_cs() by spi_get_bus_and_cs() in some case Patrice Chotard
  2 siblings, 0 replies; 4+ messages in thread
From: Patrice Chotard @ 2022-05-18  6:46 UTC (permalink / raw)
  To: u-boot
  Cc: Patrice CHOTARD, Patrick DELAUNAY, U-Boot STM32, Marek Behun,
	Jagan Teki, Vignesh R, Joe Hershberger, Ramon Fried,
	Lukasz Majewski, Marek Vasut, Wolfgang Denk, Simon Glass,
	Stefan Roese, Pali Rohár, Konstantin Porotchkin,
	Igal Liberman, Bin Meng, Pratyush Yadav, Sean Anderson, Anji J,
	Biwen Li, Priyanka Jain, Chaitanya Sakinam

Move legacy spi_get_bus_and_cs() code to _spi_get_bus_and_cs().

Add new spi_get_bus_and_cs() implementation which rely on DT
for speed and mode and don't need any drv_name nor dev_name
parameters. This will prepare the ground for next patch.

Update all callers to use _spi_get_bus_and_cs() to keep the
same behavior.

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>

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

(no changes since v1)

 board/CZ.NIC/turris_mox/turris_mox.c |  6 +--
 cmd/spi.c                            |  4 +-
 drivers/mtd/spi/sf-uclass.c          |  2 +-
 drivers/spi/spi-uclass.c             | 66 +++++++++++++++++++++++++---
 drivers/usb/gadget/max3420_udc.c     |  4 +-
 include/spi.h                        | 19 +++++++-
 test/dm/spi.c                        | 33 +++++++-------
 7 files changed, 104 insertions(+), 30 deletions(-)

diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
index a4738b3a3c..68bc315a9c 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -149,9 +149,9 @@ 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,
-				 "spi_generic_drv", "moxtet@1", &dev,
-				 &slave);
+	ret = _spi_get_bus_and_cs(0, 1, 1000000, SPI_CPHA | SPI_CPOL,
+				  "spi_generic_drv", "moxtet@1", &dev,
+				  &slave);
 	if (ret)
 		goto fail;
 
diff --git a/cmd/spi.c b/cmd/spi.c
index 6dc32678da..454ebe37d7 100644
--- a/cmd/spi.c
+++ b/cmd/spi.c
@@ -46,8 +46,8 @@ 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",
-				 str, &dev, &slave);
+	ret = _spi_get_bus_and_cs(bus, cs, freq, mode, "spi_generic_drv",
+				  str, &dev, &slave);
 	if (ret)
 		return ret;
 #else
diff --git a/drivers/mtd/spi/sf-uclass.c b/drivers/mtd/spi/sf-uclass.c
index 63d16291ff..b45ba54ebf 100644
--- a/drivers/mtd/spi/sf-uclass.c
+++ b/drivers/mtd/spi/sf-uclass.c
@@ -74,7 +74,7 @@ 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,
+	ret = _spi_get_bus_and_cs(busnum, cs, max_hz, spi_mode,
 				  "jedec_spi_nor", str, &bus, &slave);
 	if (ret)
 		return ret;
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index f8ec312d71..f2791c4b88 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -340,9 +340,65 @@ 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,
-		       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, struct udevice **busp,
+		       struct spi_slave **devp)
+{
+	struct udevice *bus, *dev;
+	struct dm_spi_bus *bus_data;
+	struct spi_slave *slave;
+	int ret;
+
+#if CONFIG_IS_ENABLED(OF_PLATDATA)
+	ret = uclass_first_device_err(UCLASS_SPI, &bus);
+#else
+	ret = uclass_get_device_by_seq(UCLASS_SPI, busnum, &bus);
+#endif
+	if (ret) {
+		log_err("Invalid bus %d (err=%d)\n", busnum, ret);
+		return ret;
+	}
+	ret = spi_find_chip_select(bus, cs, &dev);
+	if (ret) {
+		dev_err(bus, "Invalid chip select %d:%d (err=%d)\n", busnum, cs, ret);
+		return ret;
+	}
+
+	if (!device_active(dev)) {
+		struct spi_slave *slave;
+
+		ret = device_probe(dev);
+		if (ret)
+			goto err;
+		slave = dev_get_parent_priv(dev);
+		slave->dev = dev;
+	}
+
+	slave = dev_get_parent_priv(dev);
+	bus_data = dev_get_uclass_priv(bus);
+
+	/*
+	 * In case the operation speed is not yet established by
+	 * dm_spi_claim_bus() ensure the bus is configured properly.
+	 */
+	if (!bus_data->speed) {
+		ret = spi_claim_bus(slave);
+		if (ret)
+			goto err;
+	}
+	*busp = bus;
+	*devp = slave;
+
+	return 0;
+
+err:
+	log_debug("%s: Error path, device '%s'\n", __func__, dev->name);
+
+	return ret;
+}
+
+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)
 {
 	struct udevice *bus, *dev;
 	struct dm_spi_slave_plat *plat;
@@ -453,8 +509,8 @@ 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,
-				 &slave);
+	ret = _spi_get_bus_and_cs(busnum, cs, speed, mode, 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..fa655c98dc 100644
--- a/drivers/usb/gadget/max3420_udc.c
+++ b/drivers/usb/gadget/max3420_udc.c
@@ -830,8 +830,8 @@ 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",
-			   NULL, &spid, &udc->slave);
+	_spi_get_bus_and_cs(busnum, cs, speed, mode, false, "spi_generic_drv",
+			    NULL, &spid, &udc->slave);
 
 	udc->dev = dev;
 	udc->gadget.ep0 = &udc->ep[0].ep_usb;
diff --git a/include/spi.h b/include/spi.h
index fa9ab12dbe..9a8c1fb260 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -572,6 +572,23 @@ int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp,
  * Given a bus number and chip select, this finds the corresponding bus
  * device and slave device.
  *
+ * @busnum:	SPI bus number
+ * @cs:		Chip select to look for
+ * @busp:	Returns bus device
+ * @devp:	Return slave device
+ * @return 0 if found, -ve on error
+ */
+int spi_get_bus_and_cs(int busnum, int cs,
+		       struct udevice **busp, struct spi_slave **devp);
+
+/**
+ * _spi_get_bus_and_cs() - Find and activate bus and slave devices by number
+ * As spi_flash_probe(), This is an old-style function. We should remove
+ * it when all SPI flash drivers use dm
+ *
+ * Given a bus number and chip select, this finds the corresponding bus
+ * device and slave device.
+ *
  * If no such slave exists, and drv_name is not NULL, then a new slave device
  * is automatically bound on this chip select with requested speed and mode.
  *
@@ -588,7 +605,7 @@ int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp,
  * @devp:	Return slave device
  * Return: 0 if found, -ve on error
  */
-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,
 			const char *drv_name, const char *dev_name,
 			struct udevice **busp, struct spi_slave **devp);
 
diff --git a/test/dm/spi.c b/test/dm/spi.c
index ee4ad3abaa..7ab0820abb 100644
--- a/test/dm/spi.c
+++ b/test/dm/spi.c
@@ -46,19 +46,19 @@ 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));
+	ut_asserteq(-ENODEV, _spi_get_bus_and_cs(busnum, cs, speed, mode,
+						 NULL, 0, &bus, &slave));
 
 	/*
 	 * This forces the device to be re-added, but there is no emulation
 	 * connected so the probe will fail. We require that bus is left
-	 * alone on failure, and that the spi_get_bus_and_cs() does not add
+	 * alone on failure, and that the _spi_get_bus_and_cs() does not add
 	 * 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,
-						"jedec_spi_nor", "name", &bus,
-						&slave));
+	ut_asserteq(-ENOENT, _spi_get_bus_and_cs(busnum, cs, speed, mode,
+						 "jedec_spi_nor", "name", &bus,
+						 &slave));
 	sandbox_sf_unbind_emul(state_get_current(), busnum, cs);
 	ut_assertok(spi_cs_info(bus, cs, &info));
 	ut_asserteq_ptr(NULL, info.dev);
@@ -67,8 +67,8 @@ 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,
-				       "jedec_spi_nor", "name", &bus, &slave));
+	ut_assertok(_spi_get_bus_and_cs(busnum, cs, speed, mode,
+					"jedec_spi_nor", "name", &bus, &slave));
 
 	ut_assertok(spi_cs_info(bus, cs, &info));
 	ut_asserteq_ptr(info.dev, slave->dev);
@@ -76,8 +76,9 @@ static int dm_test_spi_find(struct unit_test_state *uts)
 	/* We should be able to add something to another chip select */
 	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));
+	ut_asserteq(-EINVAL, _spi_get_bus_and_cs(busnum, cs_b, speed, mode,
+						 "jedec_spi_nor", "name", &bus,
+						 &slave));
 	ut_asserteq(-EINVAL, spi_cs_info(bus, cs_b, &info));
 	ut_asserteq_ptr(NULL, info.dev);
 
@@ -145,11 +146,11 @@ 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,
-				       &bus, &slave_a));
+	ut_assertok(_spi_get_bus_and_cs(busnum, cs_a, 1000000, mode, NULL, 0,
+					&bus, &slave_a));
 	/* Get spi slave on CS1 */
-	ut_assertok(spi_get_bus_and_cs(busnum, cs_b, 1000000, mode, NULL, 0,
-				       &bus, &slave_b));
+	ut_assertok(_spi_get_bus_and_cs(busnum, cs_b, 1000000, mode, NULL, 0,
+					&bus, &slave_b));
 
 	/* Different max_hz, different mode. */
 	ut_assert(slave_a->max_hz != slave_b->max_hz);
@@ -182,8 +183,8 @@ 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,
-				       &bus, &slave));
+	ut_assertok(_spi_get_bus_and_cs(busnum, cs, 1000000, mode, NULL, 0,
+					&bus, &slave));
 	ut_assertok(spi_claim_bus(slave));
 	ut_assertok(spi_xfer(slave, 40, dout, din,
 			     SPI_XFER_BEGIN | SPI_XFER_END));
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [RESEND PATCH v4 2/3] spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode
  2022-05-18  6:46 [RESEND PATCH v4 0/3] spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode Patrice Chotard
  2022-05-18  6:46 ` [RESEND PATCH v4 1/3] spi: spi-uclass: Add new spi_get_bus_and_cs() implementation Patrice Chotard
@ 2022-05-18  6:46 ` Patrice Chotard
  2022-05-18  6:46 ` [RESEND PATCH v4 3/3] test: dm: spi: Replace _spi_get_bus_and_cs() by spi_get_bus_and_cs() in some case Patrice Chotard
  2 siblings, 0 replies; 4+ messages in thread
From: Patrice Chotard @ 2022-05-18  6:46 UTC (permalink / raw)
  To: u-boot
  Cc: Patrice CHOTARD, Patrick DELAUNAY, U-Boot STM32, Marek Behun,
	Jagan Teki, Vignesh R, Joe Hershberger, Ramon Fried,
	Lukasz Majewski, Marek Vasut, Wolfgang Denk, Simon Glass,
	Stefan Roese, Pali Rohár, Konstantin Porotchkin,
	Igal Liberman, Bin Meng, Pratyush Yadav, Sean Anderson, Anji J,
	Biwen Li, Priyanka Jain, Chaitanya Sakinam, Heinrich Schuchardt,
	Jassi Brar

Now, spi_flash_probe_bus_cs() relies on DT for spi speed and mode
and logically calls spi_get_bus_and_cs(). In case spi mode and speed are
not read from DT, make usage of spi_flash_probe() instead.

To sum-up:
 - Previous call tree was:
    spi_flash_probe() -> spi_flash_probe_bus_cs() -> spi_get_bus_and_cs()

 - Current call tree is:
    spi_flash_probe() -> _spi_get_bus_and_cs()
    spi_flash_probe_bus_cs() -> spi_get_bus_and_cs()

This patch impacts the following :
  - cmd/sf.c: if spi mode and/or speed is passed in argument of
    do_spi_flash_probe(), call spi_flash_probe() otherwise call
    spi_flash_probe_bus_cs().

  - drivers/net/fm/fm.c: as by default spi speed and mode was set to
    0 and a comment indicates that speed and mode are read from DT,
    use spi_flash_probe_bus_cs().

  - drivers/net/pfe_eth/pfe_firmware.c: spi speed and mode are not read
    from DT by all platforms using this driver, so keep legacy and replace
    spi_flash_probe_bus_cs() by spi_flash_probe();

  - drivers/net/sni_netsec.c : spi speed and mode are not read from DT,
    so replace spi_flash_probe_bus_cs() by spi_flash_probe().

  - drivers/usb/gadget/max3420_udc.c: Can't find any platform which make
    usage of this driver, nevertheless, keep legacy and replace
    spi_get_bus_and_cs() by _spi_get_bus_and_cs().

  - env/sf.c: a comment indicates that speed and mode are read
    from DT. So use spi_flash_probe_bus_cs().

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>

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

Changes in v4:
  - Split previous series in 3 patches
  - Update commit message with additionnal information

Changes in v3:
  - Update commit header to reflect what the patch really do.
  - Rename legacy spi_get_bus_and_cs() to _spi_get_bus_and_cs().
  - New spi_get_bus_and_cs() rely on DT for spi speed and mode values.
  - spi_flash_probe_bus_cs() rely also on DT for spi and mode values.

Changes in v2:
  - add spi_flash_probe_bus_cs_default() which calls spi_get_bus_and_cs()
    with "use_dt" param set to true, whereas spi_flash_probe_bus_cs() calls
    spi_get_bus_and_cs() with "use_dt" param set to true.

 cmd/sf.c                           | 15 ++++++++++----
 drivers/mtd/spi/sf-uclass.c        | 33 +++++++++++++++---------------
 drivers/net/fm/fm.c                |  4 ++--
 drivers/net/pfe_eth/pfe_firmware.c | 19 ++++-------------
 drivers/net/sni_netsec.c           |  6 ++----
 env/sf.c                           |  1 -
 include/spi_flash.h                |  1 -
 7 files changed, 36 insertions(+), 43 deletions(-)

diff --git a/cmd/sf.c b/cmd/sf.c
index 8bdebd9fd8..8713736b2a 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,14 +134,18 @@ 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);
-	if (ret) {
+	if (use_dt) {
+		spi_flash_probe_bus_cs(bus, cs, &new);
+		flash = dev_get_uclass_priv(new);
+	} else {
+		flash = spi_flash_probe(bus, cs, speed, mode);
+	}
+
+	if (!flash) {
 		printf("Failed to initialize SPI flash at %u:%u (error %d)\n",
 		       bus, cs, ret);
 		return 1;
 	}
-
-	flash = dev_get_uclass_priv(new);
 #else
 	if (flash)
 		spi_flash_free(flash);
diff --git a/drivers/mtd/spi/sf-uclass.c b/drivers/mtd/spi/sf-uclass.c
index b45ba54ebf..e6e650ef8c 100644
--- a/drivers/mtd/spi/sf-uclass.c
+++ b/drivers/mtd/spi/sf-uclass.c
@@ -46,25 +46,12 @@ int spl_flash_get_sw_write_prot(struct udevice *dev)
  * TODO(sjg@chromium.org): This is an old-style function. We should remove
  * it when all SPI flash drivers use dm
  */
-struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
+struct spi_flash *spi_flash_probe(unsigned int busnum, unsigned int cs,
 				  unsigned int max_hz, unsigned int spi_mode)
-{
-	struct udevice *dev;
-
-	if (spi_flash_probe_bus_cs(bus, cs, max_hz, spi_mode, &dev))
-		return NULL;
-
-	return dev_get_uclass_priv(dev);
-}
-
-int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs,
-			   unsigned int max_hz, unsigned int spi_mode,
-			   struct udevice **devp)
 {
 	struct spi_slave *slave;
 	struct udevice *bus;
 	char *str;
-	int ret;
 
 #if defined(CONFIG_SPL_BUILD) && CONFIG_IS_ENABLED(USE_TINY_PRINTF)
 	str = "spi_flash";
@@ -74,8 +61,22 @@ 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);
+
+	if (_spi_get_bus_and_cs(busnum, cs, max_hz, spi_mode,
+				"jedec_spi_nor", str, &bus, &slave))
+		return NULL;
+
+	return dev_get_uclass_priv(slave->dev);
+}
+
+int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs,
+			   struct udevice **devp)
+{
+	struct spi_slave *slave;
+	struct udevice *bus;
+	int ret;
+
+	ret = spi_get_bus_and_cs(busnum, cs, &bus, &slave);
 	if (ret)
 		return ret;
 
diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c
index f825612640..d0b492b5a1 100644
--- a/drivers/net/fm/fm.c
+++ b/drivers/net/fm/fm.c
@@ -388,7 +388,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_SF_DEFAULT_BUS,
-					     CONFIG_SF_DEFAULT_CS, 0, 0, &new);
+					     CONFIG_SF_DEFAULT_CS, &new);
 
 		ucode_flash = dev_get_uclass_priv(new);
 #else
@@ -475,7 +475,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_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
-				     0, 0, &new);
+				     &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 6669048181..82a4aa89a4 100644
--- a/drivers/net/pfe_eth/pfe_firmware.c
+++ b/drivers/net/pfe_eth/pfe_firmware.c
@@ -172,31 +172,20 @@ static int pfe_fit_check(void)
 int pfe_spi_flash_init(void)
 {
 	struct spi_flash *pfe_flash;
-	struct udevice *new;
 	int ret = 0;
 	void *addr = malloc(CONFIG_SYS_LS_PFE_FW_LENGTH);
 
 	if (!addr)
 		return -ENOMEM;
 
-	ret = spi_flash_probe_bus_cs(CONFIG_SYS_FSL_PFE_SPI_BUS,
-				     CONFIG_SYS_FSL_PFE_SPI_CS,
-				     CONFIG_SYS_FSL_PFE_SPI_MAX_HZ,
-				     CONFIG_SYS_FSL_PFE_SPI_MODE,
-				     &new);
-	if (ret) {
-		printf("SF: failed to probe spi\n");
-		free(addr);
-		device_remove(new, DM_REMOVE_NORMAL);
-		return ret;
-	}
-
+	pfe_flash = spi_flash_probe(CONFIG_SYS_FSL_PFE_SPI_BUS,
+				    CONFIG_SYS_FSL_PFE_SPI_CS,
+				    CONFIG_SYS_FSL_PFE_SPI_MAX_HZ,
+				    CONFIG_SYS_FSL_PFE_SPI_MODE);
 
-	pfe_flash = dev_get_uclass_priv(new);
 	if (!pfe_flash) {
 		printf("SF: probe for pfe failed\n");
 		free(addr);
-		device_remove(new, DM_REMOVE_NORMAL);
 		return -ENODEV;
 	}
 
diff --git a/drivers/net/sni_netsec.c b/drivers/net/sni_netsec.c
index 24caacf847..9780f2092b 100644
--- a/drivers/net/sni_netsec.c
+++ b/drivers/net/sni_netsec.c
@@ -621,12 +621,10 @@ static int netsec_stop_gmac(struct netsec_priv *priv)
 
 static void netsec_spi_read(char *buf, loff_t len, loff_t offset)
 {
-	struct udevice *new;
 	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);
-	flash = dev_get_uclass_priv(new);
+	flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
+				CONFIG_SF_DEFAULT_SPEED, CONFIG_SF_DEFAULT_MODE);
 
 	spi_flash_read(flash, offset, len, buf);
 }
diff --git a/env/sf.c b/env/sf.c
index d2c07cd716..4b768542c1 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -48,7 +48,6 @@ 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);
 	if (ret) {
 		env_set_default("spi_flash_probe_bus_cs() failed", 0);
diff --git a/include/spi_flash.h b/include/spi_flash.h
index d33d0dd46a..10d19fd4b1 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -102,7 +102,6 @@ int spl_flash_get_sw_write_prot(struct udevice *dev);
 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);
 
 /* Compatibility function - this is the old U-Boot API */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [RESEND PATCH v4 3/3] test: dm: spi: Replace _spi_get_bus_and_cs() by spi_get_bus_and_cs() in some case
  2022-05-18  6:46 [RESEND PATCH v4 0/3] spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode Patrice Chotard
  2022-05-18  6:46 ` [RESEND PATCH v4 1/3] spi: spi-uclass: Add new spi_get_bus_and_cs() implementation Patrice Chotard
  2022-05-18  6:46 ` [RESEND PATCH v4 2/3] spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode Patrice Chotard
@ 2022-05-18  6:46 ` Patrice Chotard
  2 siblings, 0 replies; 4+ messages in thread
From: Patrice Chotard @ 2022-05-18  6:46 UTC (permalink / raw)
  To: u-boot
  Cc: Patrice CHOTARD, Patrick DELAUNAY, U-Boot STM32, Marek Behun,
	Jagan Teki, Vignesh R, Joe Hershberger, Ramon Fried,
	Lukasz Majewski, Marek Vasut, Wolfgang Denk, Simon Glass,
	Stefan Roese, Pali Rohár, Konstantin Porotchkin,
	Igal Liberman, Bin Meng, Pratyush Yadav, Sean Anderson, Anji J,
	Biwen Li, Priyanka Jain, Chaitanya Sakinam

In case _spi_get_bus_and_cs()'s parameters drv_name and dev_name are
respectively set to NULL and 0, use spi_get_bus_and_cs() instead.

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>

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

(no changes since v1)

 test/dm/spi.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/test/dm/spi.c b/test/dm/spi.c
index 7ab0820abb..325799bbf1 100644
--- a/test/dm/spi.c
+++ b/test/dm/spi.c
@@ -46,8 +46,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));
+	ut_asserteq(-ENODEV, spi_get_bus_and_cs(busnum, cs, &bus, &slave));
 
 	/*
 	 * This forces the device to be re-added, but there is no emulation
@@ -143,14 +142,12 @@ static int dm_test_spi_claim_bus(struct unit_test_state *uts)
 	struct udevice *bus;
 	struct spi_slave *slave_a, *slave_b;
 	struct dm_spi_slave_plat *slave_plat;
-	const int busnum = 0, cs_a = 0, cs_b = 1, mode = 0;
+	const int busnum = 0, cs_a = 0, cs_b = 1;
 
 	/* Get spi slave on CS0 */
-	ut_assertok(_spi_get_bus_and_cs(busnum, cs_a, 1000000, mode, NULL, 0,
-					&bus, &slave_a));
+	ut_assertok(spi_get_bus_and_cs(busnum, cs_a, &bus, &slave_a));
 	/* Get spi slave on CS1 */
-	ut_assertok(_spi_get_bus_and_cs(busnum, cs_b, 1000000, mode, NULL, 0,
-					&bus, &slave_b));
+	ut_assertok(spi_get_bus_and_cs(busnum, cs_b, &bus, &slave_b));
 
 	/* Different max_hz, different mode. */
 	ut_assert(slave_a->max_hz != slave_b->max_hz);
@@ -179,12 +176,11 @@ static int dm_test_spi_xfer(struct unit_test_state *uts)
 {
 	struct spi_slave *slave;
 	struct udevice *bus;
-	const int busnum = 0, cs = 0, mode = 0;
+	const int busnum = 0, cs = 0;
 	const char dout[5] = {0x9f};
 	unsigned char din[5];
 
-	ut_assertok(_spi_get_bus_and_cs(busnum, cs, 1000000, mode, NULL, 0,
-					&bus, &slave));
+	ut_assertok(spi_get_bus_and_cs(busnum, cs, &bus, &slave));
 	ut_assertok(spi_claim_bus(slave));
 	ut_assertok(spi_xfer(slave, 40, dout, din,
 			     SPI_XFER_BEGIN | SPI_XFER_END));
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-05-18  6:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-18  6:46 [RESEND PATCH v4 0/3] spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode Patrice Chotard
2022-05-18  6:46 ` [RESEND PATCH v4 1/3] spi: spi-uclass: Add new spi_get_bus_and_cs() implementation Patrice Chotard
2022-05-18  6:46 ` [RESEND PATCH v4 2/3] spi: spi_flash_probe_bus_cs() rely on DT for spi speed and mode Patrice Chotard
2022-05-18  6:46 ` [RESEND PATCH v4 3/3] test: dm: spi: Replace _spi_get_bus_and_cs() by spi_get_bus_and_cs() in some case Patrice Chotard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.