RE: [RFC PATCH net-next 1/9] ethtool: Add ethtool operation to write to a transceiver module EEPROM

[Danielle Ratson]

> >  /**
> > - * struct ethtool_module_eeprom - EEPROM dump from specified page
> > - * @offset: Offset within the specified EEPROM page to begin read, in
> bytes.
> > - * @length: Number of bytes to read.
> > - * @page: Page number to read from.
> > - * @bank: Page bank number to read from, if applicable by EEPROM spec.
> > + * struct ethtool_module_eeprom - plug-in module EEPROM read / write
> > + parameters
> > + * @offset: Offset within the specified page, in bytes.
> > + * @length: Number of bytes to read / write.
> > + * @page: Page number.
> > + * @bank: Bank number, if supported by EEPROM spec.
> 
> I suppose I should have reviewed the addition of this (I can't recall whether I
> got the original or not.)
> 
> If one looks at SFF-8472, then the first 128 bytes of the EEPROM at
> 0x50 (0xA0 on the wire) are not paged. Whereas bytes 128..255 are the paged
> bytes. Therefore, "offset within the specified page" can sensibly be
> interpreted as referring to the EEPROM at 0x50, at an offset of
> 128 + offset.
> 
> Meanwhile, the actual implementation doesn't do that - the offset is the
> offset from the beginning of the EEPROM, and offsets >= 128 access the
> paged area.
> 
> What this means is that the parameter description here is basically wrong,
> both before and after your change.
> 
> This really ought to be fixed so that we describe things correctly rather than
> misleading people who read documentation. Otherwise, it's a recipe for
> broken implementations... and it's also completely pointless documenting it if
> the documentation is wrong.
> 

Will rephrase.

> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!