Hi Heiko,
On 2026-10-04T04:06:44, Heiko Schocher <[email protected]> wrote:
> sandbox: spi: let a slave point at the emulation it needs
>
> sandbox_sf_bind_emul() binds the SPI NOR flash emulator for every slave
> on the sandbox SPI bus, whatever that slave is, so a device of another
> kind cannot be emulated and cannot be tested.
>
> As with I2C and PCI, a slave can point to an emulation node via a
> sandbox,emul phandle. Its compatible selects the driver. Without the
> property, the flash emulator remains bound to the slave node as before.
>
> Rename sandbox_sf_unbind_emul() to sandbox_spi_unbind_emul() while
> here. It is no longer specific to SPI NOR, and the old name would
> mislead every new caller.
>
> Signed-off-by: Heiko Schocher <[email protected]>
>
> drivers/mtd/spi/sandbox.c | 65 ++++++++++++++++++++++++++++++++++++++---------
> include/spi_flash.h | 2 +-
> test/dm/sf.c | 4 +--
> test/dm/spi.c | 10 ++++----
> 4 files changed, 61 insertions(+), 20 deletions(-)
> diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
> @@ -518,39 +520,78 @@ static const struct dm_spi_emul_ops sandbox_sf_emul_ops
> = {
> #ifdef CONFIG_SPI_FLASH
> -int sandbox_sf_bind_emul(struct sandbox_state *state, int busnum, int cs,
> - struct udevice *bus, ofnode node, const char *spec)
> +/**
> + * sandbox_spi_emul_lookup() - bind the emulator standing in for a slave
The generic part (following the sandbox,emul phandle) now lives in the
SPI NOR emulator, inside #ifdef CONFIG_SPI_FLASH, as does the only
non-weak sandbox_spi_get_emul(). So emulating an SRAM, or anything
else that is not flash, needs SPI_FLASH and SPI_FLASH_SANDBOX enabled.
Please can you move the phandle lookup and sandbox_spi_get_emul() into
drivers/spi/sandbox_spi.c, replacing the __weak version? The flash
emulator can then supply just the fallback for nodes with no
sandbox,emul property.
> diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
> @@ -518,39 +520,78 @@ static const struct dm_spi_emul_ops sandbox_sf_emul_ops
> = {
> + if (!ofnode_parse_phandle_with_args(node, "sandbox,emul", NULL, 0, 0,
> + &args)) {
> + ret = lists_bind_fdt(bus, args.node, emulp, NULL, false);
> + if (ret)
> + printf("Cannot bind emulator from node '%s' (err=%d)\n",
> + ofnode_get_name(args.node), ret);
> +
> + return ret;
> + }
lists_bind_fdt() returns 0 and sets *emulp to NULL when no driver
matches the compatible, e.g. with CONFIG_SPI_RAM_SANDBOX off. NULL is
then stored in state->spi[][].emul, sandbox_spi_get_emul() succeeds,
and sandbox_spi_xfer() gets -EINVAL from device_probe(NULL) with no
hint about the cause. sandbox_spi_unbind_emul() would also pass NULL
to device_remove(). Please check for a NULL device and return -ENOENT
with a message naming the node.
> diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
> @@ -518,39 +520,78 @@ static const struct dm_spi_emul_ops sandbox_sf_emul_ops
> = {
> +static int sandbox_spi_emul_lookup(struct udevice *bus, ofnode node,
> + const char *spec, struct udevice **emulp)
This binds a device rather than looking one up, so the name is
misleading. Also, I2C and PCI do not bind on demand: their emulators
are bound by the normal DT scan under an emulation parent node, then
found with uclass_get_device_by_phandle(..., "sandbox,emul", ...). If
SPI did the same, you could drop the lists_bind_fdt() call, and tests
would no longer need to unbind the emulator by hand before devices are
destroyed. Here the emulator node is a child of the slave, so it would
also be bound twice if the SRAM driver (or the MTD uclass) ever
scanned its children. What do you think?
> diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
> @@ -518,39 +520,78 @@ static const struct dm_spi_emul_ops sandbox_sf_emul_ops
> = {
> +int sandbox_sf_bind_emul(struct sandbox_state *state, int busnum, int cs,
> + struct udevice *bus, ofnode node, const char *spec)
The unbind function is renamed because it is no longer specific to SPI
NOR, but this one, which now binds any emulator too, keeps the 'sf'
name. Please rename it to sandbox_spi_bind_emul() so the two stay
consistent.
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> @@ -136,7 +136,7 @@ struct sandbox_state;
> -void sandbox_sf_unbind_emul(struct sandbox_state *state, int busnum, int cs);
> +void sandbox_spi_unbind_emul(struct sandbox_state *state, int busnum, int
> cs);
If this is now a generic SPI helper, it should be declared in
include/spi.h next to sandbox_spi_get_emul(), not in spi_flash.h under
DM_SPI_FLASH.
Regards,
Simon
