Hi,
The "sf probe" command is documented to take optional speed/mode args:
sf probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus and chip select
This worked correctly in older u-boot versions, but as of 2019.07 the speed/mode arguments appear to be effectively ignored. As this feature is quite useful for testing purposes, I would like to find a way to support it again. But the logic is surprisingly complicated, so I would like to get some advice/ideas.
The user-specified speed/mode are parsed in do_spi_flash_probe() and eventually are passed to spi_get_bus_and_cs(). In 2019.04 and earlier, the logic here was pretty straightforward:
plat = dev_get_parent_platdata(dev); if (!speed) { speed = plat->max_hz; mode = plat->mode; } ret = spi_set_speed_mode(bus, speed, mode); if (ret) goto err;
So this calls spi_set_speed_mode() with the user-specified speed, or a default value.
As of commit 0cc1b846fcb310c0ac2f8cbeb4ed5947dc52912 ("dm: spi: Read default speed and mode values from DT") the logic has changed. The user-specified speed value is now stored into plat->max_hz, but *only* when no chip select has been configured. In practice this means only the first call to spi_get_bus_and_cs() uses the speed parameter. Thereafter the speed will not change.
Related commit f7dd5370986087af9b9cfa601f34b344ec910b87 ("spi: prevent overriding established bus settings") removes the call to spi_set_speed_mode() entirely. So the speed is now set by dm_spi_claim_bus() which uses slave->max_hz, which in turn is set from plat->max_hz in the spi_child_pre_probe() function.
The first call to spi_get_bus_and_cs() typically occurs when searching for u-boot environment, and uses the speed specified in DT. This becomes the only speed, as subsequent "sf probe" do not update plat->max_hz.
One potential work-around would be to clear the chip select prior to re-binding the device. This allows plat->max_hz to be updated, andalso means that device_bind_driver() will be repeateed. However I am not entirely certain if this is correct approach. In particular, is using -1 for the cs appropriate in all cases? It seems it can come from DT.
diff --git a/cmd/sf.c b/cmd/sf.c index d18f6a888c..8cb70f6487 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -128,6 +128,9 @@ static int do_spi_flash_probe(int argc, char *const argv[]) /* Remove the old device, otherwise probe will just be a nop */ ret = spi_find_bus_and_cs(bus, cs, &bus_dev, &new); if (!ret) { + struct dm_spi_slave_platdata *plat; + plat = dev_get_parent_platdata(new); + plat->cs = -1; device_remove(new, DM_REMOVE_NORMAL); } flash = NULL;