Re: [RFC net-next 1/1] ethtool: Add support for set eeprom metadata.

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 3158 bytes --]

On Sun, 2016-06-05 at 13:29 +0000, Yuval Mintz wrote:
> > 
> > > > Currently ethtool implementation does not have a way to pass the
> > > > metadata for eeprom related operations. Some adapters have a
> > > > complicated non-volatile memory implementation that requires
> > > > additional information – there are drivers [bnx2x and bnxt] that use
> > > > the ‘magic’ field in the {G,S}EEPROM  for that purpose, although that’s not
> > its intended usage.
> > > > 
> > > > This patch adds a provision to pass the eeprom metadata for
> > > > %ETHTOOL_SEEPROM/%ETHTOOL_GEEPROM implementations. User
> > provided
> > > > metadata will be cached by the driver and assigns a magic value
> > > > which the application need to use for the subsequent {G,S}EEPROM
> > command.
> > > 
> > > Hi Dave,
> > > 
> > > This got no comments at all.
> > > What do you want us to with it next? Should we re-send it as a patch?
> > 
> > Here's a comment: I really dislike this.
> :-)
> 
> > - It doesn't specify any semantics for the 'metadata'.  The comment hints that
> > they are driver-specific identifiers for different NVRAM partitions.
> Not exactly, but close [in our use case there are 2 'methods' of accessing the
> flash - either according to addresses or logical 'files'].
> 
> > - It doesn't provide a way for userland to enumerate the valid metadata values.
> I agree; I can't think of any good way of enumerating device-specific values.
> 
> > - It's not clear whether the driver is supposed to maintain just one
> > metadata:magic mapping, or more than that.
> Theoretically, I guess it could maintain multiple, but that wasn't the intention.
> One should do.
> 
> > Is the ethtool API really the right interface for access to flash?  The sfc driver
> > exposes a large number of flash partitions using MTD instead.  These can be
> > enumerated (through /proc/mtd or sysfs) and they can be read and written
> > through block devices.
> 
> I think the better question then is 'what's the purpose of this ethtool API at all'?

I think it's a bit of an accident - MTD was designed for flash in
embedded systems, and it used to have a static limit on the number of
partitions.  The ethtool API was then rather better suited to plug-in
cards that would have a single small EEPROM.

> I agree we can go and do everything via MTD; The reason we've tried using this
> API was mainly... because it was there. And thus we thought this is the RIGHT
> method for providing users the way of reading their flash.
[...]

I think that MTD makes more sense for flash partitions, especially when
there are several of them.  I already did the work of removing 
the static limit on the number of partitions, and convincing
distributions to enable the MTD core drivers.  (That said, you will
still find some users who need to change their custom kernel
configurations.)

Ben.

-- 
Ben Hutchings
Everything should be made as simple as possible, but not simpler.
                                                           - Albert
Einstein


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]