Hello Simon,
On 09.10.26 09:06, Simon Glass wrote:
Hi Heiko,
On 2026-10-04T04:06:44, Heiko Schocher <[email protected]> wrote:
mtd: add driver for the Microchip 23LCV1024 SPI SRAM
The 23LCV1024 is a 1 Mbit serial SRAM with battery backup, sitting on
an SPI bus and used as non volatile scratch storage. Register it as an
MTD device of type MTD_RAM, so the mtd command reaches it.
The device powers up in byte operation, where every command carries
exactly one data byte. Select sequential operation once during probe,
Just to check, is this right? My understanding is that the
23LC1024/23LCV1024 data sheet gives sequential mode (01) as the MODE
register default; byte mode is the default only on the older 23K256.
The Linux mchp23k256 driver never writes the MODE register. Setting it
explicitly is still a fine defensive step, since something else could
have changed the mode and the part keeps it while on battery. Please
can you reword the commit message and the comment in spi_ram_probe()
to match?
Indeed, my comment is wrong, fix that in v3.
But the linux driver drivers/mtd/devices/mchp23k256.c
base * 24a19b953e2a - (HEAD -> master, tag: v7.3-rc7, origin/master,
origin/HEAD) Linux 7.3-rc7)
171 static int mchp23k256_probe(struct spi_device *spi)
[...]
185 err = mchp23k256_set_mode(spi);
186 if (err)
187 return err;
with
143 static int mchp23k256_set_mode(struct spi_device *spi)
[...]
151 command[0] = MCHP23K256_CMD_WRITE_STATUS;
152 command[1] = MCHP23K256_MODE_SEQ;
so the same as my patch does...
so that a read or a write of any length is served by a single command
and the bus is claimed once per transfer.
Writing needs no erase. The driver therefore implements no erase
operation and says MTD_NO_ERASE in the MTD flags.
Signed-off-by: Stefan Bigler <[email protected]>
Signed-off-by: Heiko Schocher <[email protected]>
MAINTAINERS | 5 ++
configs/sandbox_defconfig | 1 +
drivers/mtd/Kconfig | 14 +++
drivers/mtd/Makefile | 1 +
drivers/mtd/spi_ram.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 243 insertions(+)
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
@@ -194,6 +194,20 @@ config ALTERA_QSPI
+config SPI_RAM
+ bool "Microchip 23LCV1024 SPI SRAM"
+ depends on DM_SPI && DM_MTD
SPI_RAM, spi_ram.c and the spi_ram driver name are very generic for a
driver that handles one Microchip part. Linux already supports this
chip in drivers/mtd/devices/mchp23k256.c (CONFIG_MTD_MCHP23K256), and
the binding in dts/upstream/Bindings/mtd/microchip,mchp23k256.yaml
lists both microchip,mchp23k256 and microchip,mchp23lcv1024. How about
using the Linux names, here and in MAINTAINERS?
Yep, as in an earlier reply to Takahire Kuwano, I will make this
renaming in a v3.
And while looking through the linux driver, I will add mchp23k256
compatible too in v3, and add a sandbox emulation for this (I cannot test
it).
diff --git a/drivers/mtd/spi_ram.c b/drivers/mtd/spi_ram.c
@@ -0,0 +1,222 @@
+static int spi_ram_xfer(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
+ const void *data_out, void *data_in, size_t data_len)
+{
+ unsigned long flags = SPI_XFER_BEGIN;
+ int ret;
+
+ if (!data_len)
+ flags |= SPI_XFER_END;
This duplicates spi_write_then_read() in drivers/spi/spi-uclass.c,
which has the same flag handling and debug messages. You could drop
this function and use that one instead?
Will drop it, thanks!
diff --git a/drivers/mtd/spi_ram.c b/drivers/mtd/spi_ram.c
@@ -0,0 +1,222 @@
+ ret = spi_claim_bus(spi);
+ if (ret) {
+ dev_dbg(dev, "failed to claim SPI bus: %d\n", ret);
+ return ret;
+ }
+
+ ret = spi_ram_xfer(spi, cmd, sizeof(cmd), data_out, data_in, len);
A read or write of the whole 128KB goes out as a single spi_xfer().
This fails on controllers that only implement spi-mem ops or that
limit the transfer size, where the SPI NAND driver you are modelling
this on would work. Using spi_mem_exec_op() with
spi_mem_adjust_op_size() in a loop would cover both cases. I'm not
sure how much that matters for your boards, so I'm just mentioning it.
I will look into it.
In any case:
Reviewed-by: Simon Glass <[email protected]>
Thanks!
bye,
Heiko
Regards,
Simon
--
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: [email protected]
Geschäftsführer : Stefano Babic
