Add 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.
Introduce spi_flash_probe_bus_cs_default() which is identical to spi_flash_probe_bus_cs() except it calls spi_get_bus_and_cs() with use_dt param set to false.
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_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@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 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.
board/CZ.NIC/turris_mox/turris_mox.c | 2 +- cmd/sf.c | 9 ++++++++- cmd/spi.c | 2 +- drivers/mtd/spi/sf-uclass.c | 30 ++++++++++++++++++++++++++-- drivers/spi/spi-uclass.c | 8 ++++---- drivers/usb/gadget/max3420_udc.c | 2 +- include/spi.h | 7 ++++--- include/spi_flash.h | 4 ++++ test/dm/spi.c | 15 +++++++------- 9 files changed, 59 insertions(+), 20 deletions(-)
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index f0c5aa6a52..4b755e1420 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -148,7 +148,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@1", &dev, &slave); if (ret) diff --git a/cmd/sf.c b/cmd/sf.c index 8bdebd9fd8..40b2cc3297 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,11 @@ 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 (use_dt) + ret = spi_flash_probe_bus_cs(bus, cs, speed, mode, &new); + else + ret = spi_flash_probe_bus_cs_default(bus, cs, speed, mode, &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..f38518b7fd 100644 --- a/drivers/mtd/spi/sf-uclass.c +++ b/drivers/mtd/spi/sf-uclass.c @@ -74,8 +74,34 @@ 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, true, + "jedec_spi_nor", str, &bus, &slave); + if (ret) + return ret; + + *devp = slave->dev; + return 0; +} + +int spi_flash_probe_bus_cs_default(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"; +#else + char name[30]; + + 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, false, + "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..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/include/spi.h b/include/spi.h index fa9ab12dbe..0aa5fe033c 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 */ -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 d33d0dd46a..256878df6a 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -105,6 +105,10 @@ int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs, unsigned int max_hz, unsigned int spi_mode, struct udevice **devp);
+int spi_flash_probe_bus_cs_default(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 */ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs, unsigned int max_hz, unsigned int spi_mode); 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,