mbox series

[0/9] AT24 EEPROM MTD Support

Message ID 20240701-b4-v6-10-topic-usbc-tcpci-v1-0-3fd5f4a193cc@pengutronix.de
Headers show
Series AT24 EEPROM MTD Support | expand

Message

Marco Felsch July 1, 2024, 1:53 p.m. UTC
This series adds the intial support to handle EEPROMs via the MTD layer
as well. This allow the user-space to have separate paritions since
EEPROMs can become quite large nowadays.

With this patchset applied EEPROMs can be accessed via:
  - legacy 'eeprom' device
  - nvmem device
  - mtd device(s)

The patchset targets only the AT24 (I2C) EEPROMs since I have no access
to AT25 (SPI) EEPROMs nor to one of the other misc/eeprom/* devices.

Note: I'm not familiar with Kconfig symbol migration so I don't know if
the last patch is required at the moment. Please be notified that the
list of recipients is quite large due to the defconfig changes.

Regards,
  Marco

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
Marco Felsch (9):
      mtd: core: add nvmem_write support
      mtd: add mtd_is_master helper
      mtd: add support to handle EEPROM devices
      mtd: devices: add AT24 eeprom support
      ARM: defconfig: convert to MTD_EEPROM_AT24
      powerpc: convert to MTD_EEPROM_AT24
      MIPS: configs: convert to MTD_EEPROM_AT24
      LoongArch: convert to MTD_EEPROM_AT24
      eeprom: at24: remove deprecated Kconfig symbol

 MAINTAINERS                                 |   2 +-
 arch/arm/configs/aspeed_g4_defconfig        |   2 +-
 arch/arm/configs/aspeed_g5_defconfig        |   2 +-
 arch/arm/configs/at91_dt_defconfig          |   2 +-
 arch/arm/configs/axm55xx_defconfig          |   2 +-
 arch/arm/configs/davinci_all_defconfig      |   2 +-
 arch/arm/configs/imx_v4_v5_defconfig        |   2 +-
 arch/arm/configs/imx_v6_v7_defconfig        |   2 +-
 arch/arm/configs/ixp4xx_defconfig           |   2 +-
 arch/arm/configs/keystone_defconfig         |   2 +-
 arch/arm/configs/lpc18xx_defconfig          |   2 +-
 arch/arm/configs/lpc32xx_defconfig          |   2 +-
 arch/arm/configs/multi_v5_defconfig         |   2 +-
 arch/arm/configs/multi_v7_defconfig         |   2 +-
 arch/arm/configs/mvebu_v5_defconfig         |   2 +-
 arch/arm/configs/mvebu_v7_defconfig         |   2 +-
 arch/arm/configs/mxs_defconfig              |   2 +-
 arch/arm/configs/omap2plus_defconfig        |   2 +-
 arch/arm/configs/pxa_defconfig              |   2 +-
 arch/arm/configs/s3c6400_defconfig          |   2 +-
 arch/arm/configs/sama5_defconfig            |   2 +-
 arch/arm/configs/sama7_defconfig            |   2 +-
 arch/arm/configs/shmobile_defconfig         |   2 +-
 arch/arm/configs/socfpga_defconfig          |   2 +-
 arch/arm/configs/tegra_defconfig            |   2 +-
 arch/arm/configs/wpcm450_defconfig          |   2 +-
 arch/loongarch/configs/loongson3_defconfig  |   2 +-
 arch/mips/configs/cavium_octeon_defconfig   |   2 +-
 arch/mips/configs/db1xxx_defconfig          |   2 +-
 arch/powerpc/configs/44x/warp_defconfig     |   2 +-
 arch/powerpc/configs/mpc512x_defconfig      |   2 +-
 arch/powerpc/configs/mpc5200_defconfig      |   2 +-
 arch/powerpc/configs/ppc6xx_defconfig       |   2 +-
 arch/powerpc/configs/skiroot_defconfig      |   2 +-
 drivers/misc/eeprom/Kconfig                 |  31 -------
 drivers/misc/eeprom/Makefile                |   1 -
 drivers/mtd/devices/Kconfig                 |  31 +++++++
 drivers/mtd/devices/Makefile                |   1 +
 drivers/{misc/eeprom => mtd/devices}/at24.c | 122 +++++++++++++++-------------
 drivers/mtd/mtdcore.c                       |  49 ++++++++++-
 include/linux/mtd/mtd.h                     |   5 ++
 include/uapi/mtd/mtd-abi.h                  |   2 +
 42 files changed, 187 insertions(+), 123 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240701-b4-v6-10-topic-usbc-tcpci-c4bc9bcce604

Best regards,

Comments

Sergei Shtylyov July 1, 2024, 4:14 p.m. UTC | #1
On 7/1/24 4:53 PM, Marco Felsch wrote:

> Provide a simple helper to make it easy to detect an master mtd device.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  include/linux/mtd/mtd.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 8d10d9d2e830..bf3fc2ea7230 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -408,6 +408,11 @@ static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
>  	return mtd;
>  }
>  
> +static inline bool mtd_is_master(struct mtd_info *mtd)
> +{
> +	return mtd->parent ? false : true;

   Perhaps:

	return !mtd->parent;

[...]

MBR, Sergey
Tudor Ambarus July 1, 2024, 4:14 p.m. UTC | #2
On 7/1/24 2:53 PM, Marco Felsch wrote:
> EEPROMs can become quite large nowadays (>=64K). Exposing such devices
> as single device isn't always sufficient. There may be partitions which
> require different access permissions. Also write access always need to
> to verify the offset.
> 
> Port the current misc/eeprom/at24.c driver to the MTD framework since
> EEPROMs are memory-technology devices and the framework already supports

I was under the impression that MTD devices are tightly coupled by erase
blocks. But then we see MTD_NO_ERASE, so what are MTD devices after all?

> partitioning. This allow using of-paritions like we do for SPI-NOR
> devices already:
Marco Felsch July 2, 2024, 8:22 a.m. UTC | #3
On 24-07-01, Sergei Shtylyov wrote:
> On 7/1/24 4:53 PM, Marco Felsch wrote:
> 
> > Provide a simple helper to make it easy to detect an master mtd device.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  include/linux/mtd/mtd.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> > index 8d10d9d2e830..bf3fc2ea7230 100644
> > --- a/include/linux/mtd/mtd.h
> > +++ b/include/linux/mtd/mtd.h
> > @@ -408,6 +408,11 @@ static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> >  	return mtd;
> >  }
> >  
> > +static inline bool mtd_is_master(struct mtd_info *mtd)
> > +{
> > +	return mtd->parent ? false : true;
> 
>    Perhaps:
> 
> 	return !mtd->parent;

Sure, if you prefer this style rather I will change it.

Regards,
  Marco
Bartosz Golaszewski July 2, 2024, 8:57 a.m. UTC | #4
On Mon, Jul 1, 2024 at 3:54 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
>
> All kernel users are shifted to the new MTD_EEPROM_AT24 Kconfig symbol
> so we can drop the old one.
>

Nope, with this series arm64 still selects the old symbol.

Bart
Marco Felsch July 2, 2024, 9:15 a.m. UTC | #5
Hi,

On 24-07-02, Bartosz Golaszewski wrote:
> On Mon, Jul 1, 2024 at 3:54 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
> >
> > All kernel users are shifted to the new MTD_EEPROM_AT24 Kconfig symbol
> > so we can drop the old one.
> >
> 
> Nope, with this series arm64 still selects the old symbol.

sry. I must have forgotten to add the arm64 hunk :/ I also noticed one
powerpc config which still select the old symbol. I will fix this in v2.

Thank you,
  Marco

> 
> Bart
>
Pratyush Yadav July 2, 2024, 1:41 p.m. UTC | #6
On Mon, Jul 01 2024, Tudor Ambarus wrote:

> On 7/1/24 2:53 PM, Marco Felsch wrote:
>> EEPROMs can become quite large nowadays (>=64K). Exposing such devices
>> as single device isn't always sufficient. There may be partitions which
>> require different access permissions. Also write access always need to
>> to verify the offset.
>> 
>> Port the current misc/eeprom/at24.c driver to the MTD framework since
>> EEPROMs are memory-technology devices and the framework already supports
>
> I was under the impression that MTD devices are tightly coupled by erase
> blocks. But then we see MTD_NO_ERASE, so what are MTD devices after all?

I was curious as well so I did some digging.

The Kconfig help says:

    Memory Technology Devices are flash, RAM and similar chips, often
    used for solid state file systems on embedded devices [...]

The FAQ on the MTD documentation [0] says:

    Unix traditionally only knew block devices and character devices.
    Character devices were things like keyboards or mice, that you could
    read current data from, but couldn't be seek-ed and didn't have a size.
    Block devices had a fixed size and could be seek-ed. They also happened
    to be organized in blocks of multiple bytes, usually 512.

    Flash doesn't match the description of either block or character
    devices. They behave similar to block device, but have differences. For
    example, block devices don't distinguish between write and erase
    operations. Therefore, a special device type to match flash
    characteristics was created: MTD.

    So MTD is neither a block nor a char device. There are translations to
    use them, as if they were. But those translations are nowhere near the
    original, just like translated Chinese poems.

And in the section below, it lists some properties of an MTD device:

    - Consists of eraseblocks.
    - Eraseblocks are larger (typically 128KiB).
    - Maintains 3 main operations: read from eraseblock, write to
      eraseblock, and erase eraseblock.
    - Bad eraseblocks are not hidden and should be dealt with in
      software.
    - Eraseblocks wear-out and become bad and unusable after about 10^3
      (for MLC NAND) - 10^5 (NOR, SLC NAND) erase cycles.

This does support the assumption you had about MTD devices being tightly
coupled with erase block. It also makes it quite clear that an EEPROM is
not MTD -- since EEPROMs are byte-erasable.

Of course, the existence of MTD_NO_ERASE nullifies a lot of
these points. So it seems the subsystem has evolved. MTD_NO_ERASE was
added by 92cbfdcc3661d ("[MTD] replace MTD_RAM with MTD_GENERIC_TYPE")
in 2006, but this commit only adds the flag. The functionality of "not
requiring an explicit erase" for RAM devices has existed since the start
of the git history at least.

I also found a thread from 2013 by Maxime Ripard (+Cc) suggesting adding
EEPROMs to MTD [1]. The main purpose would have been unifying the EEPROM
drivers under a single interface. I am not sure what came of it though,
since I can't find any patches that followed up with the proposal.

Overall, I'd say that while originally MTD was written with flash
devices with erase blocks in mind, the subsystem seems to have evolved
with time to include other types of devices.

I don't see anything obviously wrong with adding EEPROMs to the type of
devices in MTD as well. It doesn't seem to be too invasive to the
subsystem (I do see some dubious code when skimming through the patches,
but nothing unfixable). And the EEPROM drivers can get a common
interface. The other option would be to create a separate subsystem for
EEPROMs, but perhaps that would just lead to a bunch of code being
duplicated.

I'd like to hear if somebody thinks otherwise, and sees reasons to _not_
do this.

[0] http://www.linux-mtd.infradead.org/faq/general.html
[1] https://lore.kernel.org/linux-mtd/20130705201118.GM2959@lukather/
Maxime Ripard July 2, 2024, 1:56 p.m. UTC | #7
On Tue, Jul 02, 2024 at 03:41:52PM GMT, Pratyush Yadav wrote:
> On Mon, Jul 01 2024, Tudor Ambarus wrote:
> 
> > On 7/1/24 2:53 PM, Marco Felsch wrote:
> >> EEPROMs can become quite large nowadays (>=64K). Exposing such devices
> >> as single device isn't always sufficient. There may be partitions which
> >> require different access permissions. Also write access always need to
> >> to verify the offset.
> >> 
> >> Port the current misc/eeprom/at24.c driver to the MTD framework since
> >> EEPROMs are memory-technology devices and the framework already supports
> >
> > I was under the impression that MTD devices are tightly coupled by erase
> > blocks. But then we see MTD_NO_ERASE, so what are MTD devices after all?
> 
> I was curious as well so I did some digging.
> 
> The Kconfig help says:
> 
>     Memory Technology Devices are flash, RAM and similar chips, often
>     used for solid state file systems on embedded devices [...]
> 
> The FAQ on the MTD documentation [0] says:
> 
>     Unix traditionally only knew block devices and character devices.
>     Character devices were things like keyboards or mice, that you could
>     read current data from, but couldn't be seek-ed and didn't have a size.
>     Block devices had a fixed size and could be seek-ed. They also happened
>     to be organized in blocks of multiple bytes, usually 512.
> 
>     Flash doesn't match the description of either block or character
>     devices. They behave similar to block device, but have differences. For
>     example, block devices don't distinguish between write and erase
>     operations. Therefore, a special device type to match flash
>     characteristics was created: MTD.
> 
>     So MTD is neither a block nor a char device. There are translations to
>     use them, as if they were. But those translations are nowhere near the
>     original, just like translated Chinese poems.
> 
> And in the section below, it lists some properties of an MTD device:
> 
>     - Consists of eraseblocks.
>     - Eraseblocks are larger (typically 128KiB).
>     - Maintains 3 main operations: read from eraseblock, write to
>       eraseblock, and erase eraseblock.
>     - Bad eraseblocks are not hidden and should be dealt with in
>       software.
>     - Eraseblocks wear-out and become bad and unusable after about 10^3
>       (for MLC NAND) - 10^5 (NOR, SLC NAND) erase cycles.
> 
> This does support the assumption you had about MTD devices being tightly
> coupled with erase block. It also makes it quite clear that an EEPROM is
> not MTD -- since EEPROMs are byte-erasable.
> 
> Of course, the existence of MTD_NO_ERASE nullifies a lot of
> these points. So it seems the subsystem has evolved. MTD_NO_ERASE was
> added by 92cbfdcc3661d ("[MTD] replace MTD_RAM with MTD_GENERIC_TYPE")
> in 2006, but this commit only adds the flag. The functionality of "not
> requiring an explicit erase" for RAM devices has existed since the start
> of the git history at least.
> 
> I also found a thread from 2013 by Maxime Ripard (+Cc) suggesting adding
> EEPROMs to MTD [1]. The main purpose would have been unifying the EEPROM
> drivers under a single interface. I am not sure what came of it though,
> since I can't find any patches that followed up with the proposal.

That discussion led to drivers/nvmem after I started to work on
some early prototype, and Srinivas took over that work.

Maxime
Pratyush Yadav July 2, 2024, 2:15 p.m. UTC | #8
On Tue, Jul 02 2024, Maxime Ripard wrote:

> On Tue, Jul 02, 2024 at 03:41:52PM GMT, Pratyush Yadav wrote:
>> On Mon, Jul 01 2024, Tudor Ambarus wrote:
>> 
>> > On 7/1/24 2:53 PM, Marco Felsch wrote:
>> >> EEPROMs can become quite large nowadays (>=64K). Exposing such devices
>> >> as single device isn't always sufficient. There may be partitions which
>> >> require different access permissions. Also write access always need to
>> >> to verify the offset.
>> >> 
>> >> Port the current misc/eeprom/at24.c driver to the MTD framework since
>> >> EEPROMs are memory-technology devices and the framework already supports
>> >
>> > I was under the impression that MTD devices are tightly coupled by erase
>> > blocks. But then we see MTD_NO_ERASE, so what are MTD devices after all?
>> 
>> I was curious as well so I did some digging.
>> 
[...]
>> 
>> I also found a thread from 2013 by Maxime Ripard (+Cc) suggesting adding
>> EEPROMs to MTD [1]. The main purpose would have been unifying the EEPROM
>> drivers under a single interface. I am not sure what came of it though,
>> since I can't find any patches that followed up with the proposal.
>
> That discussion led to drivers/nvmem after I started to work on
> some early prototype, and Srinivas took over that work.

So would you say it is better for EEPROM drivers to use nvmem instead of
moving under MTD?
Maxime Ripard July 2, 2024, 2:34 p.m. UTC | #9
On Tue, Jul 02, 2024 at 04:15:20PM GMT, Pratyush Yadav wrote:
> On Tue, Jul 02 2024, Maxime Ripard wrote:
> 
> > On Tue, Jul 02, 2024 at 03:41:52PM GMT, Pratyush Yadav wrote:
> >> On Mon, Jul 01 2024, Tudor Ambarus wrote:
> >> 
> >> > On 7/1/24 2:53 PM, Marco Felsch wrote:
> >> >> EEPROMs can become quite large nowadays (>=64K). Exposing such devices
> >> >> as single device isn't always sufficient. There may be partitions which
> >> >> require different access permissions. Also write access always need to
> >> >> to verify the offset.
> >> >> 
> >> >> Port the current misc/eeprom/at24.c driver to the MTD framework since
> >> >> EEPROMs are memory-technology devices and the framework already supports
> >> >
> >> > I was under the impression that MTD devices are tightly coupled by erase
> >> > blocks. But then we see MTD_NO_ERASE, so what are MTD devices after all?
> >> 
> >> I was curious as well so I did some digging.
> >> 
> [...]
> >> 
> >> I also found a thread from 2013 by Maxime Ripard (+Cc) suggesting adding
> >> EEPROMs to MTD [1]. The main purpose would have been unifying the EEPROM
> >> drivers under a single interface. I am not sure what came of it though,
> >> since I can't find any patches that followed up with the proposal.
> >
> > That discussion led to drivers/nvmem after I started to work on
> > some early prototype, and Srinivas took over that work.
> 
> So would you say it is better for EEPROM drivers to use nvmem instead of
> moving under MTD?

I thought so at the time, but that was more than 10y ago, and I have
followed neither nvmem nor MTD since so I don't really have an opinion
there.

It looks like drivers/misc/eeprom/at24.c has support for nvmem though,
and MTD can be used as an nvmem provider too, so it's not clear to me
why we would want to create yet another variant.

But again, you shouldn't really ask me in the first place :)

I'm sure Miquel, Srinivas, and surely others, are much more relevant to
answer that question.

Maxime
Miquel Raynal July 8, 2024, 6:44 a.m. UTC | #10
Hi,

> > >> >> Port the current misc/eeprom/at24.c driver to the MTD framework since
> > >> >> EEPROMs are memory-technology devices and the framework already supports  
> > >> >
> > >> > I was under the impression that MTD devices are tightly coupled by erase
> > >> > blocks. But then we see MTD_NO_ERASE, so what are MTD devices after all?  
> > >> 
> > >> I was curious as well so I did some digging.
> > >>   
> > [...]  
> > >> 
> > >> I also found a thread from 2013 by Maxime Ripard (+Cc) suggesting adding
> > >> EEPROMs to MTD [1]. The main purpose would have been unifying the EEPROM
> > >> drivers under a single interface. I am not sure what came of it though,
> > >> since I can't find any patches that followed up with the proposal.  
> > >
> > > That discussion led to drivers/nvmem after I started to work on
> > > some early prototype, and Srinivas took over that work.  
> > 
> > So would you say it is better for EEPROM drivers to use nvmem instead of
> > moving under MTD?  
> 
> I thought so at the time, but that was more than 10y ago, and I have
> followed neither nvmem nor MTD since so I don't really have an opinion
> there.
> 
> It looks like drivers/misc/eeprom/at24.c has support for nvmem though,
> and MTD can be used as an nvmem provider too, so it's not clear to me
> why we would want to create yet another variant.
> 
> But again, you shouldn't really ask me in the first place :)
> 
> I'm sure Miquel, Srinivas, and surely others, are much more relevant to
> answer that question.

More relevant, I doubt, but just a feeling: EEPROMs have their own
subsystem now, NVMEM, which, as Maxime said, was initially written for
that very specific case. EEPROMs don't have the complexity of MTD
devices, and thus pulling the whole MTD subsystem just for getting
partitions seems counter intuitive to me. You can definitely "split"
EEPROM devices with NVMEM as well anyway.

Overall I think the idea of getting rid of these misc/ drivers is goes
into the right direction, but registering directly into NVMEM makes
more sense IMO.

Thanks,
Miquèl
Miquel Raynal July 9, 2024, 9:43 a.m. UTC | #11
Hi Marco,

> > > > >> I also found a thread from 2013 by Maxime Ripard (+Cc) suggesting adding
> > > > >> EEPROMs to MTD [1]. The main purpose would have been unifying the EEPROM
> > > > >> drivers under a single interface. I am not sure what came of it though,
> > > > >> since I can't find any patches that followed up with the proposal.    
> > > > >
> > > > > That discussion led to drivers/nvmem after I started to work on
> > > > > some early prototype, and Srinivas took over that work.    
> > > > 
> > > > So would you say it is better for EEPROM drivers to use nvmem instead of
> > > > moving under MTD?    
> > > 
> > > I thought so at the time, but that was more than 10y ago, and I have
> > > followed neither nvmem nor MTD since so I don't really have an opinion
> > > there.
> > > 
> > > It looks like drivers/misc/eeprom/at24.c has support for nvmem though,
> > > and MTD can be used as an nvmem provider too, so it's not clear to me
> > > why we would want to create yet another variant.
> > > 
> > > But again, you shouldn't really ask me in the first place :)
> > > 
> > > I'm sure Miquel, Srinivas, and surely others, are much more relevant to
> > > answer that question.  
> > 
> > More relevant, I doubt, but just a feeling: EEPROMs have their own
> > subsystem now, NVMEM, which, as Maxime said, was initially written for
> > that very specific case. EEPROMs don't have the complexity of MTD
> > devices, and thus pulling the whole MTD subsystem just for getting
> > partitions seems counter intuitive to me. You can definitely "split"
> > EEPROM devices with NVMEM as well anyway.  
> 
> I asked for feedback on my RFC [1] and all I got was to merge both
> drivers into one and make the driver backward compatible, which I did by
> this commit.

I'm sorry for not bringing this earlier.

> > Overall I think the idea of getting rid of these misc/ drivers is goes
> > into the right direction, but registering directly into NVMEM makes
> > more sense IMO.  
> 
> So you propose to have two places for the partition handling (one for
> MTD and one for NVMEM) instead of one and moving the code into NVMEM
> directly?

Why two places for the partitions handling? Just one, in NVMEM. Also
usually EEPROMs don't require very advanced partitioning schemes,
unlike flashes (which are the most common MTD devices today).

> That doesn't sound right to me either. Also I don't get the
> point why EEPROMs can't be handled by the MTD layer?

They can, but should they? Just compile the two layers and observe
the size difference. MTD is complex and old, carries a lot of history,
and the user interface is also not straightforward because you need to
handle pages, blocks, erases, bitflips, ECC stats, OOB bytes and
positions, two OTP areas... None of that exists in the EEPROM world. So
why would you want to register into MTD and pull a huge subsystem while
there is a much more recent, simpler and way lighter subsystem fitting
much better your device?

> The layer already
> supports devices of type MTD_RAM which are very simple and don't require
> an erase-op at least I don't see one.

MTD_RAM has been there forever, probably for "bad" reasons. BTW there
has been an attempt at removing it which was reverted in _2006_ and then
felt into the cracks:
21c8db9eff95 ("[MTD] Restore MTD_ROM and MTD_RAM types")

Thanks,
Miquèl
Miquel Raynal July 17, 2024, 8:19 a.m. UTC | #12
Hi Marco,

> > > > Overall I think the idea of getting rid of these misc/ drivers is goes
> > > > into the right direction, but registering directly into NVMEM makes
> > > > more sense IMO.    
> > > 
> > > So you propose to have two places for the partition handling (one for
> > > MTD and one for NVMEM) instead of one and moving the code into NVMEM
> > > directly?  
> > 
> > Why two places for the partitions handling? Just one, in NVMEM. Also  
> 
> Without checking the details I think that converting the MTD
> partitioning code into NVMEM partitioning code is a bigger task. As you
> said below there are many legacy code paths you need to consider so they
> still work afterwards as well.
> 
> > usually EEPROMs don't require very advanced partitioning schemes,
> > unlike flashes (which are the most common MTD devices today).  
> 
> As said in my cover letter EEPROMs can become quite large and MTD
> supports partitioning storage devices which is very handy for large
> EEPROMs as well.

Did you had a look at nvmem-layouts ? In particular the fixed-layout.

Is there anything you would like to achieve already that is not
possible with nvmem but is with mtd?

Thanks,
Miquèl
Marco Felsch July 18, 2024, 9:17 a.m. UTC | #13
Hi Miquel,

On 24-07-17, Miquel Raynal wrote:
> Hi Marco,
> 
> > > > > Overall I think the idea of getting rid of these misc/ drivers is goes
> > > > > into the right direction, but registering directly into NVMEM makes
> > > > > more sense IMO.    
> > > > 
> > > > So you propose to have two places for the partition handling (one for
> > > > MTD and one for NVMEM) instead of one and moving the code into NVMEM
> > > > directly?  
> > > 
> > > Why two places for the partitions handling? Just one, in NVMEM. Also  
> > 
> > Without checking the details I think that converting the MTD
> > partitioning code into NVMEM partitioning code is a bigger task. As you
> > said below there are many legacy code paths you need to consider so they
> > still work afterwards as well.
> > 
> > > usually EEPROMs don't require very advanced partitioning schemes,
> > > unlike flashes (which are the most common MTD devices today).  
> > 
> > As said in my cover letter EEPROMs can become quite large and MTD
> > supports partitioning storage devices which is very handy for large
> > EEPROMs as well.
> 
> Did you had a look at nvmem-layouts ? In particular the fixed-layout.

Yes I had a look at nvmem-layouts and we use them within a
mtd-partition. Using them instead of a mtd-partition is not sufficient
since they:
 1) don't support user-space write (I send a patch for it but it doesn't
    seem to be accepted soon).
 2) If write would be supported the user-space need to write the
    complete cell e.g. no partial writes.

> Is there anything you would like to achieve already that is not
> possible with nvmem but is with mtd?

Please see above.

Regards,
  Marco
Miquel Raynal Aug. 23, 2024, 3:37 p.m. UTC | #14
Hi Marco,

m.felsch@pengutronix.de wrote on Thu, 18 Jul 2024 11:17:53 +0200:

> Hi Miquel,
> 
> On 24-07-17, Miquel Raynal wrote:
> > Hi Marco,
> >   
> > > > > > Overall I think the idea of getting rid of these misc/ drivers is goes
> > > > > > into the right direction, but registering directly into NVMEM makes
> > > > > > more sense IMO.      
> > > > > 
> > > > > So you propose to have two places for the partition handling (one for
> > > > > MTD and one for NVMEM) instead of one and moving the code into NVMEM
> > > > > directly?    
> > > > 
> > > > Why two places for the partitions handling? Just one, in NVMEM. Also    
> > > 
> > > Without checking the details I think that converting the MTD
> > > partitioning code into NVMEM partitioning code is a bigger task. As you
> > > said below there are many legacy code paths you need to consider so they
> > > still work afterwards as well.
> > >   
> > > > usually EEPROMs don't require very advanced partitioning schemes,
> > > > unlike flashes (which are the most common MTD devices today).    
> > > 
> > > As said in my cover letter EEPROMs can become quite large and MTD
> > > supports partitioning storage devices which is very handy for large
> > > EEPROMs as well.  
> > 
> > Did you had a look at nvmem-layouts ? In particular the fixed-layout.  
> 
> Yes I had a look at nvmem-layouts and we use them within a
> mtd-partition. Using them instead of a mtd-partition is not sufficient
> since they:
>  1) don't support user-space write (I send a patch for it but it doesn't
>     seem to be accepted soon).

Yes, this needed improvements maybe but was not refused either.

>  2) If write would be supported the user-space need to write the
>     complete cell e.g. no partial writes.

Maybe that can also be brought to nvmem. Again, nvmem was introduced
for handling EEPROMs in the first place.

Anyway, if other people in Cc: want to share some thoughts, they are
also welcomed, I don't want to block this series for bad reasons. I'm
also adding Michael Walle in Cc: who might have an opinion on that.

Link: https://lore.kernel.org/linux-mtd/20240701-b4-v6-10-topic-usbc-tcpci-v1-4-3fd5f4a193cc@pengutronix.de/

Thanks,
Miquèl
Andy Shevchenko Aug. 23, 2024, 4:24 p.m. UTC | #15
On Mon, Jul 01, 2024 at 03:53:39PM +0200, Marco Felsch wrote:
> This series adds the intial support to handle EEPROMs via the MTD layer
> as well. This allow the user-space to have separate paritions since
> EEPROMs can become quite large nowadays.
> 
> With this patchset applied EEPROMs can be accessed via:
>   - legacy 'eeprom' device
>   - nvmem device
>   - mtd device(s)
> 
> The patchset targets only the AT24 (I2C) EEPROMs since I have no access
> to AT25 (SPI) EEPROMs nor to one of the other misc/eeprom/* devices.
> 
> Note: I'm not familiar with Kconfig symbol migration so I don't know if
> the last patch is required at the moment. Please be notified that the
> list of recipients is quite large due to the defconfig changes.

FWIW, I think that MTD is *not* the place for EEPROMs.

Yeah, we have the driver spread over the kernel for EEPROMs (mostly due to
historical reasons and absence an umbrella subsystem for them), but it's not
the reason to hack them into something which is not quite suitable.

If NVMEM needs to be updated and may cover these cases after all (and do not
forget about *small* size EEPROMs that most likely appear on the devices with
limited amount of resources!) in a reasonable size and performance, why not?
Marco Felsch Aug. 26, 2024, 7:51 a.m. UTC | #16
Hi Andy,

On 24-08-23, Andy Shevchenko wrote:
> On Mon, Jul 01, 2024 at 03:53:39PM +0200, Marco Felsch wrote:
> > This series adds the intial support to handle EEPROMs via the MTD layer
> > as well. This allow the user-space to have separate paritions since
> > EEPROMs can become quite large nowadays.
> > 
> > With this patchset applied EEPROMs can be accessed via:
> >   - legacy 'eeprom' device
> >   - nvmem device
> >   - mtd device(s)
> > 
> > The patchset targets only the AT24 (I2C) EEPROMs since I have no access
> > to AT25 (SPI) EEPROMs nor to one of the other misc/eeprom/* devices.
> > 
> > Note: I'm not familiar with Kconfig symbol migration so I don't know if
> > the last patch is required at the moment. Please be notified that the
> > list of recipients is quite large due to the defconfig changes.
> 
> FWIW, I think that MTD is *not* the place for EEPROMs.
> 
> Yeah, we have the driver spread over the kernel for EEPROMs (mostly due to
> historical reasons and absence an umbrella subsystem for them), but it's not
> the reason to hack them into something which is not quite suitable.

Thank you for you input. There are two things to mention:
 1st) I send a RFC patch and asked for feedback and all I got was: looks
      okay, please send a proper patch [1] which I did.
 2nd) I don't see the hacky part in this patchset.

Anyway the customer doesn't need the nvmem-partitions anymore and
therefore this patchset can be seen as obsolote.

Regards,
  Marco

[1] https://lore.kernel.org/lkml/20231201144441.imk7rrjnv2dugo7p@pengutronix.de/T/#m1e0e5778448971b50a883f62bd95622f6422b9a2

> 
> If NVMEM needs to be updated and may cover these cases after all (and do not
> forget about *small* size EEPROMs that most likely appear on the devices with
> limited amount of resources!) in a reasonable size and performance, why not?
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
>
Andy Shevchenko Aug. 26, 2024, 10:32 a.m. UTC | #17
On Mon, Aug 26, 2024 at 09:51:10AM +0200, Marco Felsch wrote:
> On 24-08-23, Andy Shevchenko wrote:
> > On Mon, Jul 01, 2024 at 03:53:39PM +0200, Marco Felsch wrote:
> > > This series adds the intial support to handle EEPROMs via the MTD layer
> > > as well. This allow the user-space to have separate paritions since
> > > EEPROMs can become quite large nowadays.
> > > 
> > > With this patchset applied EEPROMs can be accessed via:
> > >   - legacy 'eeprom' device
> > >   - nvmem device
> > >   - mtd device(s)
> > > 
> > > The patchset targets only the AT24 (I2C) EEPROMs since I have no access
> > > to AT25 (SPI) EEPROMs nor to one of the other misc/eeprom/* devices.
> > > 
> > > Note: I'm not familiar with Kconfig symbol migration so I don't know if
> > > the last patch is required at the moment. Please be notified that the
> > > list of recipients is quite large due to the defconfig changes.
> > 
> > FWIW, I think that MTD is *not* the place for EEPROMs.
> > 
> > Yeah, we have the driver spread over the kernel for EEPROMs (mostly due to
> > historical reasons and absence an umbrella subsystem for them), but it's not
> > the reason to hack them into something which is not quite suitable.
> 
> Thank you for you input. There are two things to mention:
>  1st) I send a RFC patch and asked for feedback and all I got was: looks
>       okay, please send a proper patch [1] which I did.

I was on a long vacation, I haven't had time or even wishes to look at the
patches or patch series. Sorry for that.

Second point, RFC means "request for comments", here is mine. It's up to the
maintainers and you on how to proceed it.

>  2nd) I don't see the hacky part in this patchset.

I haven't talked about patchset, I have talked about architectural / design
point of view. I read the discussion and to me it seems like it solves the
issue with a quite big hammer. If you can prove that on embedded systems with
limited resources it is not a problem, just mention that in the cover letter.

> Anyway the customer doesn't need the nvmem-partitions anymore and
> therefore this patchset can be seen as obsolote.
> 
> [1] https://lore.kernel.org/lkml/20231201144441.imk7rrjnv2dugo7p@pengutronix.de/T/#m1e0e5778448971b50a883f62bd95622f6422b9a2
> 
> > If NVMEM needs to be updated and may cover these cases after all (and do not
> > forget about *small* size EEPROMs that most likely appear on the devices with
> > limited amount of resources!) in a reasonable size and performance, why not?