| Message ID | 20261004040657.3156402-4-hs@nabladev.com |
|---|---|
| State | New |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | mtd: Microchip 23LCV1024 SPI SRAM | expand |
Hi Heiko, On 2026-10-04T04:06:44, Heiko Schocher <hs@nabladev.com> 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 <stefan.bigler@securiton.ch> > Signed-off-by: Heiko Schocher <hs@nabladev.com> > > 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? > 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 <sjg@chromium.org> Regards, Simon
Hi Heiko and Simon, > Hi Heiko, > > On 2026-10-04T04:06:44, Heiko Schocher <hs@nabladev.com> 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 <stefan.bigler@securiton.ch> > > Signed-off-by: Heiko Schocher <hs@nabladev.com> > > > > 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 <sjg@chromium.org> > > Regards, > Simon [1] https://lore.kernel.org/u-boot/20261008-everspin_mr24h40_mram_support-v1-0-d8f14da1a997@gmail.com/
Hello Takahiro.Kuwano, just answer here, before I looked into the comments from Simon... On 09.10.26 10:29, Takahiro.Kuwano@infineon.com wrote: > Hi Heiko and Simon, > >> Hi Heiko, >> >> On 2026-10-04T04:06:44, Heiko Schocher <hs@nabladev.com> 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 <stefan.bigler@securiton.ch> >>> Signed-off-by: Heiko Schocher <hs@nabladev.com> >>> >>> 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? 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 <sjg@chromium.org> >> >> Regards, >> Simon > > [1] https://lore.kernel.org/u-boot/20261008-everspin_mr24h40_mram_support-v1-0-d8f14da1a997@gmail.com/ >
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, Takahiro.Kuwano@infineon.com wrote: >> Hi Heiko and Simon, >> >>> Hi Heiko, >>> >>> On 2026-10-04T04:06:44, Heiko Schocher <hs@nabladev.com> 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 <stefan.bigler@securiton.ch> >>>> Signed-off-by: Heiko Schocher <hs@nabladev.com> >>>> >>>> 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 <sjg@chromium.org> >>> >>> Regards, >>> Simon >> >> [1] >> https://lore.kernel.org/u-boot/20261008-everspin_mr24h40_mram_support-v1-0-d8f14da1a997@gmail.com/ >> >
Hello Simon, On 09.10.26 09:06, Simon Glass wrote: > Hi Heiko, > > On 2026-10-04T04:06:44, Heiko Schocher <hs@nabladev.com> 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 <stefan.bigler@securiton.ch> >> Signed-off-by: Heiko Schocher <hs@nabladev.com> >> >> 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 <sjg@chromium.org> Thanks! bye, Heiko > > Regards, > Simon >
Hi Heiko, > 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, Takahiro.Kuwano@infineon.com wrote: > >> Hi Heiko and Simon, > >> > >>> Hi Heiko, > >>> > >>> On 2026-10-04T04:06:44, Heiko Schocher <hs@nabladev.com> 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 <stefan.bigler@securiton.ch> > >>>> Signed-off-by: Heiko Schocher <hs@nabladev.com> > >>>> > >>>> 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. The MRAM (not FRAM) part doesn't support Read ID (JEDEC ID) command. Yasir mentions that in the commit description and I checked datasheet [1]. Why did you get it has a JEDEC interface? Thanks, Takahiro [1] https://www.mouser.com/datasheet/2/144/EST_MR25H40_prod-2578.pdf?srsltid=AU7gw4XF8HonuNZoC78eWKFbMoaNLlWH1vNgEGA6NH-YFQrIpWVFulpv
Hi Takahiro.Kuwano, On 13.10.26 03:20, Takahiro.Kuwano@infineon.com wrote: > Hi Heiko, > >> 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, Takahiro.Kuwano@infineon.com wrote: >>>> Hi Heiko and Simon, >>>> >>>>> Hi Heiko, >>>>> >>>>> On 2026-10-04T04:06:44, Heiko Schocher <hs@nabladev.com> 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 <stefan.bigler@securiton.ch> >>>>>> Signed-off-by: Heiko Schocher <hs@nabladev.com> >>>>>> >>>>>> 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. > > The MRAM (not FRAM) part doesn't support Read ID (JEDEC ID) command. Yasir > mentions that in the commit description and I checked datasheet [1]. > Why did you get it has a JEDEC interface? Seems I looked into the wrong datasheet! Maybe it is too early for me here... I need a coffee now. Thanks! In linux the MRAM Yasir adds, is also handled in spi nor driver. the register 5/1 (read/write status register) and 3/2 (read/write data) seems the same for both chips and for spi-nor subsystem. But, spi-nor and MRAM expect a "status register" and the 23LCV1024 [2] has there a mode register, with different bit meanings! Seems to me, that is why the 23LCV1024 got his own driver in linux... even if this is the only obvious difference (not checked deeper now). I tend to be in sync with linux here, and still add the MRAM Yasir adds to the spi-nor driver, and have an own driver for the 23LCV1024 in drivers/mtd/devices. If not I see 2 options: a) Yasir adds the MR25H40 to the "spi-ram" driver in this thread in a follow up series. I tend to say, I first go with driver place and name Simon proposed. b) add 23LCV1024 to spi-nor subsystem and find a way to work out mode/status register differences other options? b) seems ugly, a) doable... but all options differ to linux approach. bye, Heiko [2] https://ww1.microchip.com/downloads/en/DeviceDoc/25156A.pdf > > Thanks, > Takahiro > > [1] https://www.mouser.com/datasheet/2/144/EST_MR25H40_prod-2578.pdf?srsltid=AU7gw4XF8HonuNZoC78eWKFbMoaNLlWH1vNgEGA6NH-YFQrIpWVFulpv >
HI Heiko, On Fri, 9 Oct 2026 at 10:29, Heiko Schocher <hs@nabladev.com> wrote: > > Hello Takahiro.Kuwano, > > just answer here, before I looked into the comments from Simon... > > On 09.10.26 10:29, Takahiro.Kuwano@infineon.com wrote: > > Hi Heiko and Simon, > > > >> Hi Heiko, > >> > >> On 2026-10-04T04:06:44, Heiko Schocher <hs@nabladev.com> 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 <stefan.bigler@securiton.ch> > >>> Signed-off-by: Heiko Schocher <hs@nabladev.com> > >>> > >>> 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 <sjg@chromium.org> > >> > >> Regards, > >> Simon > > > > [1] https://lore.kernel.org/u-boot/20261008-everspin_mr24h40_mram_support-v1-0-d8f14da1a997@gmail.com/ > > > > -- > Nabla Software Engineering > HRB 40522 Augsburg > Phone: +49 821 45592596 > E-Mail: office@nabladev.com > Geschäftsführer : Stefano Babic
diff --git a/MAINTAINERS b/MAINTAINERS index 2b3a9ac1f2a..1669972e724 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1649,6 +1649,11 @@ S: Maintained T: git https://git.u-boot-project.org/u-boot/custodians/u-boot-nand-flash.git F: drivers/mtd/nand/spi/ +SPI RAM +M: Heiko Schocher <hs@nabladev.com> +S: Maintained +F: drivers/mtd/spi_ram.c + SPI-NOR M: Vignesh R <vigneshr@ti.com> R: Takahiro Kuwano <takahiro.kuwano@infineon.com> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index c6edd9d2423..0210d54f462 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -260,6 +260,7 @@ CONFIG_MMC_PCI=y CONFIG_MMC_SANDBOX=y CONFIG_MMC_SDHCI=y CONFIG_DM_MTD=y +CONFIG_SPI_RAM=y CONFIG_MTD_RAW_NAND=y CONFIG_SYS_MAX_NAND_DEVICE=8 CONFIG_SYS_NAND_USE_FLASH_BBT=y diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig index f6bd9f6f2e9..437e6ebd196 100644 --- a/drivers/mtd/Kconfig +++ b/drivers/mtd/Kconfig @@ -194,6 +194,20 @@ config ALTERA_QSPI NOR flash to parallel flash interface. Please find details on the "Embedded Peripherals IP User Guide" of Altera. +config SPI_RAM + bool "Microchip 23LCV1024 SPI SRAM" + depends on DM_SPI && DM_MTD + help + Enable support for the Microchip 23LCV1024 serial SRAM, a 1 Mbit + SRAM with battery backup on an SPI bus. The device is registered + as an MTD device of type MTD_RAM, so the mtd command reaches it. + + Writing needs no erase, so the driver implements none and the MTD + flags say MTD_NO_ERASE. An erase request is answered with + -EOPNOTSUPP rather than silently doing nothing. + + If unsure, say N + config RENESAS_RPC_HF bool "Renesas R-Car Gen3 RPC HyperFlash driver" depends on RCAR_64 && DM_MTD diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile index ce05e206073..a19498da920 100644 --- a/drivers/mtd/Makefile +++ b/drivers/mtd/Makefile @@ -13,6 +13,7 @@ mtd-$(CONFIG_FLASH_CFI_MTD) += cfi_mtd.o mtd-$(CONFIG_FLASH_CFI_LEGACY) += jedec_flash.o mtd-$(CONFIG_STM32_FLASH) += stm32_flash.o mtd-$(CONFIG_RENESAS_RPC_HF) += renesas_rpc_hf.o +mtd-$(CONFIG_SPI_RAM) += spi_ram.o mtd-$(CONFIG_HBMC_AM654) += hbmc-am654.o # U-Boot build diff --git a/drivers/mtd/spi_ram.c b/drivers/mtd/spi_ram.c new file mode 100644 index 00000000000..1f5c97e26ec --- /dev/null +++ b/drivers/mtd/spi_ram.c @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Microchip 23LCV1024 SPI SRAM + * + * Copyright (C) 2019 Stefan Bigler <stefan.bigler@securiton.ch> + * Copyright (C) 2023 Heiko Schocher, DENX Software Engineering, hs@denx.de + * Copyright (C) 2026 Heiko Schocher, Nabla Software Engineering <hs@nabladev.com> + */ + +#include <dm.h> +#include <dm/device_compat.h> +#include <log.h> +#include <spi.h> +#include <vsprintf.h> +#include <linux/bitops.h> +#include <linux/mtd/mtd.h> +#include <linux/sizes.h> + +/* Instruction set of the 23x series serial SRAMs */ +#define MCHP23_CMD_WRITE_STATUS 0x01 +#define MCHP23_CMD_WRITE 0x02 +#define MCHP23_CMD_READ 0x03 + +/* MODE register, bits 7:6 select byte, page or sequential operation */ +#define MCHP23_MODE_SEQ BIT(6) + +/* The 1 Mbit parts take a 24 bit address */ +#define MCHP23_ADDR_LEN 3 + +/** + * struct spi_ram_data - what one supported device looks like + * + * @size: usable size of the array in bytes + * @name: name the MTD device is registered under + */ +struct spi_ram_data { + u32 size; + const char *name; +}; + +/** + * struct spi_ram_priv - per device state + * + * @name: MTD name, built from the device name and the sequence number + * so that several of these can coexist + */ +struct spi_ram_priv { + char name[16]; +}; + +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; + + ret = spi_xfer(spi, cmd_len * 8, cmd, NULL, flags); + if (ret) { + log_debug("failed to send command (%zu bytes): %d\n", + cmd_len, ret); + return ret; + } + + if (!data_len) + return 0; + + ret = spi_xfer(spi, data_len * 8, data_out, data_in, SPI_XFER_END); + if (ret) + log_debug("failed to transfer %zu bytes of data: %d\n", + data_len, ret); + + return ret; +} + +/** + * spi_ram_command() - run one read or write command over the whole length + * + * Sequential operation was selected during probe, so the device serves a + * transfer of any length from a single command and the bus is claimed once. + * + * @dev: the SRAM device + * @opcode: MCHP23_CMD_READ or MCHP23_CMD_WRITE + * @offset: byte offset into the array + * @data_out: data to write, NULL when reading + * @data_in: where to put data read, NULL when writing + * @len: number of bytes to transfer + * Return: 0 on success, negative on error + */ +static int spi_ram_command(struct udevice *dev, u8 opcode, u32 offset, + const void *data_out, void *data_in, size_t len) +{ + struct spi_slave *spi = dev_get_parent_priv(dev); + u8 cmd[1 + MCHP23_ADDR_LEN]; + int ret; + + cmd[0] = opcode; + cmd[1] = offset >> 16; + cmd[2] = offset >> 8; + cmd[3] = offset; + + 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); + + spi_release_bus(spi); + + return ret; +} + +static int spi_ram_read(struct mtd_info *mtd, loff_t from, size_t len, + size_t *retlen, u_char *buf) +{ + int ret; + + ret = spi_ram_command(mtd->dev, MCHP23_CMD_READ, from, NULL, buf, len); + if (ret) + return ret; + + *retlen = len; + + return 0; +} + +static int spi_ram_write(struct mtd_info *mtd, loff_t to, size_t len, + size_t *retlen, const u_char *buf) +{ + int ret; + + ret = spi_ram_command(mtd->dev, MCHP23_CMD_WRITE, to, buf, NULL, len); + if (ret) + return ret; + + *retlen = len; + + return 0; +} + +static int spi_ram_probe(struct udevice *dev) +{ + const struct spi_ram_data *data = + (const struct spi_ram_data *)dev_get_driver_data(dev); + struct mtd_info *mtd = dev_get_uclass_priv(dev); + struct spi_ram_priv *priv = dev_get_priv(dev); + struct spi_slave *spi = dev_get_parent_priv(dev); + u8 cmd[2]; + int ret; + + ret = spi_claim_bus(spi); + if (ret) { + dev_dbg(dev, "failed to claim SPI bus: %d\n", ret); + return ret; + } + + /* + * The device powers up in byte operation, where a command transfers + * exactly one byte. Select sequential operation instead, so that a + * transfer of any length is served by a single command. + */ + cmd[0] = MCHP23_CMD_WRITE_STATUS; + cmd[1] = MCHP23_MODE_SEQ; + ret = spi_ram_xfer(spi, cmd, sizeof(cmd), NULL, NULL, 0); + + spi_release_bus(spi); + + if (ret) { + dev_dbg(dev, "failed to select sequential operation: %d\n", + ret); + return ret; + } + + snprintf(priv->name, sizeof(priv->name), "%s%d", data->name, + dev_seq(dev)); + mtd->name = priv->name; + mtd->dev = dev; + mtd->type = MTD_RAM; + mtd->flags = MTD_CAP_RAM; + mtd->size = data->size; + mtd->writesize = 1; + mtd->writebufsize = 1; + + /* + * No _erase: a RAM has nothing to erase, which MTD_NO_ERASE in the + * flags above states, and mtd_erase() answers -EOPNOTSUPP for a device + * that provides none. + */ + mtd->_read = spi_ram_read; + mtd->_write = spi_ram_write; + + return add_mtd_device(mtd); +} + +static int spi_ram_remove(struct udevice *dev) +{ + return del_mtd_device(dev_get_uclass_priv(dev)); +} + +static const struct spi_ram_data mchp23lcv1024_data = { + .size = SZ_128K, + .name = "spi-ram", +}; + +static const struct udevice_id spi_ram_ids[] = { + { .compatible = "microchip,mchp23lcv1024", + .data = (ulong)&mchp23lcv1024_data }, + { } +}; + +U_BOOT_DRIVER(spi_ram) = { + .name = "spi_ram", + .id = UCLASS_MTD, + .of_match = spi_ram_ids, + .probe = spi_ram_probe, + .remove = spi_ram_remove, + .priv_auto = sizeof(struct spi_ram_priv), +};