On Mon, Jan 22, 2024 at 10:45:22AM +0200, Danielle Ratson wrote: > /** > - * 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. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
