Message-ID: <20240709114302.3c604ef3@xps-13>
Date: Tue, 9 Jul 2024 11:43:02 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Marco Felsch <m.felsch@...gutronix.de>
Cc: Maxime Ripard <mripard@...nel.org>, Pratyush Yadav
 <pratyush@...nel.org>, Tudor Ambarus <tudor.ambarus@...aro.org>, Richard
 Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>, Arnd
 Bergmann <arnd@...db.de>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Bartosz Golaszewski <brgl@...ev.pl>, Russell King <linux@...linux.org.uk>,
 Joel Stanley <joel@....id.au>, Andrew Jeffery
 <andrew@...econstruct.com.au>, Nicolas Ferre <nicolas.ferre@...rochip.com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>, Claudiu Beznea
 <claudiu.beznea@...on.dev>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer
 <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>, Vladimir Zapolskiy <vz@...ia.com>,
 Andrew Lunn <andrew@...n.ch>, Gregory Clement
 <gregory.clement@...tlin.com>, Sebastian Hesselbarth
 <sebastian.hesselbarth@...il.com>, Tony Lindgren <tony@...mide.com>, Geert
 Uytterhoeven <geert+renesas@...der.be>, Magnus Damm
 <magnus.damm@...il.com>, Dinh Nguyen <dinguyen@...nel.org>, Thierry Reding
 <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>,
 Jonathan Neuschäfer <j.neuschaefer@....net>, Michael
 Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>,
 Christophe Leroy <christophe.leroy@...roup.eu>, "Naveen N. Rao"
 <naveen.n.rao@...ux.ibm.com>, Thomas Bogendoerfer
 <tsbogend@...ha.franken.de>, Huacai Chen <chenhuacai@...nel.org>, WANG
 Xuerui <kernel@...0n.name>, linux-mtd@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-aspeed@...ts.ozlabs.org,
 imx@...ts.linux.dev, linux-omap@...r.kernel.org,
 linux-renesas-soc@...r.kernel.org, linux-tegra@...r.kernel.org,
 openbmc@...ts.ozlabs.org, linuxppc-dev@...ts.ozlabs.org,
 linux-mips@...r.kernel.org, loongarch@...ts.linux.dev
Subject: Re: [PATCH 4/9] mtd: devices: add AT24 eeprom support

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

Powered by blists - more mailing lists