Hi Heiko, On 2026-10-04T04:06:44, 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 MTD layer. > > 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. > > Test also if erase returns -EOPNOTSUPP and data is not touched. > > A second test gives the device two partitions in the test device tree > and writes through the one whose offset is on no erase block boundary, > which a device without an erase size does not have to care about. > > Signed-off-by: Heiko Schocher <[email protected]> > > MAINTAINERS | 2 + > arch/sandbox/dts/test.dts | 37 ++++++++ > arch/sandbox/include/asm/test.h | 8 ++ > configs/sandbox_defconfig | 1 + > drivers/mtd/Kconfig | 9 ++ > drivers/mtd/Makefile | 1 + > drivers/mtd/spi_ram_sandbox.c | 190 > ++++++++++++++++++++++++++++++++++++++++ > drivers/spi/Kconfig | 2 +- > test/dm/Makefile | 1 + > test/dm/spi_ram.c | 167 +++++++++++++++++++++++++++++++++++ > 10 files changed, 417 insertions(+), 1 deletion(-)
> diff --git a/test/dm/Makefile b/test/dm/Makefile > @@ -120,6 +120,7 @@ obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o > +obj-$(CONFIG_SPI_RAM) += spi_ram.o I'm assuming that CONFIG_SPI_RAM_SANDBOX is always set for sanebox builds. > diff --git a/test/dm/spi_ram.c b/test/dm/spi_ram.c > @@ -0,0 +1,167 @@ > + bus = dev_get_parent(dev); > + ut_assertok(spi_get_bus_and_cs(dev_seq(bus), SPI_RAM_CS, &bus, &slave)); > + ut_assertok(sandbox_spi_get_emul(state, bus, slave->dev, &emul)); slave->dev is just 'dev', so you can drop spi_get_bus_and_cs() and 'slave' and call sandbox_spi_get_emul(state, bus, dev, &emul) directly. > diff --git a/test/dm/spi_ram.c b/test/dm/spi_ram.c > @@ -0,0 +1,167 @@ > + ut_assert(mtd->flags & MTD_NO_ERASE); > + instr.addr = 0; > + instr.len = SPI_RAM_OFFSET; > + ut_asserteq(-EOPNOTSUPP, mtd_erase(mtd, &instr)); > + > + ut_assertok(mtd_read(mtd, SPI_RAM_OFFSET, SPI_RAM_LEN, &retlen, dst)); > + ut_asserteq_mem(src, dst, SPI_RAM_LEN); The erase covers [0, 0x1f) but the read-back covers [0x1f, 0x83), so if the erase did touch the data, this check would not see it. Please can you erase over the written data instead, i.e. addr = SPI_RAM_OFFSET and len = SPI_RAM_LEN? > diff --git a/test/dm/spi_ram.c b/test/dm/spi_ram.c > @@ -0,0 +1,167 @@ > +/* > + * The second partition of the device in the test device tree. Its offset is > + * on no erase block boundary, which a device that cannot erase does not have > + * to care about. > + */ > +#define SPI_RAM_PART "spi-ram-part1" > +#define SPI_RAM_PART_OFFSET 0xa000 0xa000 is 8K-aligned, so contrary to the comment it does sit on a typical erase-block boundary. The test catches the divide-by-zero from patch 2 but does not show that an unaligned offset still gives a writeable partition. How about an odd offset such as 0xa001 (in test.dts too), and rewording the comment to say the device has no erase size, so the write size of 1 is the only alignment that applies? > diff --git a/drivers/mtd/spi_ram_sandbox.c b/drivers/mtd/spi_ram_sandbox.c > @@ -0,0 +1,190 @@ > +/* > + * The command and the data it carries arrive as two transfers, the first > + * one holding the opcode and, for a read or a write, the address. > + */ This ties the emulation to how the driver currently splits its transfers: a driver that sends command and data in a single BEGIN|END transfer would have its data silently dropped. The real device just sees a byte stream while CS is asserted. You could treat any bytes after the address in the first transfer as data (n starting at 0 from there), as the SPI flash emulator does, so the emulation follows the data sheet rather than the driver. What do you think? Regards, Simon
