Hi Igor,
On 2026-10-27T05:57:26, Igor Velkov via B4 Relay
<[email protected]> wrote:
> cmd: sf: Probe a flash at the requested speed
>
> 'sf probe <bus>:<cs> <speed>' ignores the speed for a flash that has a
> device tree node: the probe, and with it the JEDEC ID read, runs at
> spi-max-frequency, and the requested speed reaches the controller only
> after it.
>
> On ROCK Pi 4A the device tree sets spi-max-frequency = <108000000> and
> the flash does not answer at the resulting clock, at any requested
> speed:
>
> => sf probe 1:0 1000000
> jedec_spi_nor flash@0: unrecognized JEDEC id bytes: ff, ff, ff
>
> Before the probe, set the requested speed in the platform data of the
> device, as _spi_get_bus_and_cs() already does for a device it binds
> without a device tree node, and restore the saved value right after it,
> so the platform data ends up unchanged. slave->mode keeps the device
> tree value. A speed above spi-max-frequency is honoured, as it already
> is after the probe. Only sf probe with an explicit speed changes: sf
> [...]
>
> cmd/sf.c | 11 +++++++++++
> test/dm/sf.c | 29 +++++++++++++++++++++++++++++
> 2 files changed, 40 insertions(+)
> diff --git a/cmd/sf.c b/cmd/sf.c
> @@ -146,7 +146,18 @@ static int do_spi_flash_probe(int argc, char *const
> argv[])
> + struct dm_spi_slave_plat *plat = NULL;
> + uint saved_max_hz = 0;
> +
> + /* Probe at the requested speed: the flash reads its ID there */
> + if (!ret && speed) {
> + plat = dev_get_parent_plat(new);
> + saved_max_hz = plat->max_hz;
> + plat->max_hz = speed;
> + }
> flash = spi_flash_probe(bus, cs, speed, mode);
> + if (plat)
> + plat->max_hz = saved_max_hz;
This works, but the command now reaches into the SPI uclass's
per-child platform data, which spi_slave_of_to_plat() normally owns.
It also relies on spi_child_pre_probe() copying plat->max_hz into
slave->max_hz, so please can you move this into a helper in
spi-uclass.c, or a variant of spi_flash_probe(), so the command
doesn't depend on those internals? Other callers can keep their
current behaviour.
The mode has the same problem: _spi_get_bus_and_cs() sends the
requested mode to the controller, but the next dm_spi_claim_bus()
restores slave->mode, so 'sf probe 1:0 1000000 3' quietly ignores the
3 for a DT flash. It would be worth handling the mode in the same way,
or at least saying in the commit message why it is left alone. What do
you think?
Regards,
Simon
