Hi Heiko,
On 2026-09-22T06:09:29, Heiko Schocher <[email protected]> wrote:
> test: dm: add a test for the Microchip 23LCV1024 SPI SRAM
>
> Emulate the device on a second sandbox SPI bus and exercise the driver
> through the spi_flash uclass.
>
> The emulation implements byte, page and sequential operation and
> advances the internal address counter the way the data sheet describes
> for each of them. A transfer running past the 32 byte page of the
> device therefore only comes back unharmed once the driver has selected
> sequential operation, which is what its probe is for. The test asserts
> that mode directly as well, so a failure says which of the two went
> wrong.
>
> The erase is checked on both counts that matter for a device that has
> nothing to erase: that the area reads back as 0xff afterwards, and that
> the fill stops at the end of the range it was given rather than running
> into the data next to it.
>
> Signed-off-by: Heiko Schocher <[email protected]>
>
> arch/sandbox/dts/test.dts | 14 +++
> arch/sandbox/include/asm/test.h | 8 ++
> configs/sandbox_defconfig | 1 +
> drivers/mtd/spi/Kconfig | 9 ++
> drivers/mtd/spi/Makefile | 1 +
> drivers/mtd/spi/spi_ram_sandbox.c | 189
> ++++++++++++++++++++++++++++++++++++++
> drivers/spi/Kconfig | 2 +-
> test/dm/Makefile | 1 +
> test/dm/spi_ram.c | 96 +++++++++++++++++++
> 9 files changed, 320 insertions(+), 1 deletion(-)
> diff --git a/test/dm/spi_ram.c b/test/dm/spi_ram.c
> @@ -0,0 +1,96 @@
> + /* Now erase the data itself, over more than one staging step */
> + ut_assertok(spi_flash_erase_dm(dev, SPI_RAM_OFFSET, SPI_RAM_LEN));
The comment does not match the code: SPI_RAM_LEN is 100 and
SPI_RAM_ERASE_STEP in patch 1 is SZ_1K, so spi_ram_erase() runs a
single iteration here. If the intent is to cover the staging loop,
please raise the length past SZ_1K (or drop the staging step for the
test); otherwise reword the comment.
> diff --git a/test/dm/spi_ram.c b/test/dm/spi_ram.c
> @@ -0,0 +1,96 @@
> + bus = dev_get_parent(dev);
> + emul = state->spi[dev_seq(bus)][SPI_RAM_CS].emul;
> + ut_assertnonnull(emul);
> + ut_assert(sandbox_spi_ram_is_sequential(emul));
Please use sandbox_spi_get_emul() here rather than reaching into
state->spi[][] directly - it already knows how to locate the emulator
for a given slave.
> diff --git a/test/dm/spi_ram.c b/test/dm/spi_ram.c
> @@ -0,0 +1,96 @@
> + /*
> + * Since we are about to destroy all devices, we must tell sandbox
> + * to forget the emulation device
> + */
> + sandbox_sf_unbind_emul(state, dev_seq(bus), SPI_RAM_CS);
The name sandbox_sf_unbind_emul() reads as SPI-NOR specific, but after
patch 2 it is used for any SPI slave's emulator. Please rename it
(e.g. sandbox_spi_unbind_emul()) in patch 2 and update the callers, so
this new test does not entrench the misleading name.
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> @@ -1672,6 +1672,20 @@
> + spi@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1 1>;
> + compatible = "sandbox,spi";
Please add a spi1 entry to the aliases block above, so dev_seq(bus) is
deterministic rather than depending on probe order - the test relies
on it to look up the emul.
Regards,
Simon
