Hi Takahiro.Kuwano,
On 09.10.26 11:28, Heiko Schocher wrote:
Hello Takahiro.Kuwano,
just answer here, before I looked into the comments from Simon...
On 09.10.26 10:29, [email protected] wrote:
Hi Heiko and Simon,
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?
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?
I would rather have a generic SPI-RAM driver that also supports MRAM [1]
and FRAM devices using the common opcode set without erase.
Currently, some FRAM chips are handled by SPI-NOR, but new devices should
be added to the SPI-RAM driver if it exists.
What are your thoughts on this?
Seems like a valid argument to me. Simon? What do you think?
After looking into patches from Yasir (added to cc). the FRAM
case is different. The FRAM (at least the one Yasir adds) has a JEDEC
interface, the 23LCV1024 has not! So 23LCV1024 is really special, and I
currently tend to say, that Simon is correct, that we should name
the 23LCV1024 driver drivers/mtd/devices/mchp23k256.c.
So I prepare a v3 for my patchset with:
CONFIG_SPI_RAM -> CONFIG_MTD_MCHP23K256
drivers/mtd/spi_ram.c -> drivers/mtd/devices/mchp23k256.c
driver name "spi_ram" -> "mchp23k256"
Thanks!
bye,
Heiko
Thanks!
bye,
Heiko
Thanks,
Takahiro
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?
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.
In any case:
Reviewed-by: Simon Glass <[email protected]>
Regards,
Simon
[1]
https://lore.kernel.org/u-boot/20261008-everspin_mr24h40_mram_support-v1-0-d8f14da1a...@gmail.com/
--
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: [email protected]
Geschäftsführer : Stefano Babic
