Hello Simon,
On 28.09.26 16:45, Simon Glass wrote:
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.
Good catch... Hmm.. I rework this too, as I drop the erase function.
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.
I will do.
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.
I look into it.
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.
I add it.
Thanks!
bye,
Heiko
Regards,
Simon
--
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: [email protected]
Geschäftsführer : Stefano Babic
