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?
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/
