HI Heiko,
On Fri, 9 Oct 2026 at 10:29, Heiko Schocher <[email protected]> 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?
Seems reasonable to me, but I have not dug into how these chips actually work.
Regards,
Simon
>
> 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
