Re: [RFC PATCH V3 net-next 1/5] ethtool: Allow network drivers to dump arbitrary EEPROM data

[Andrew Lunn <andrew@lunn.ch>]

> > > +static int eeprom_data_parse_request(struct ethnl_req_info *req_info,
> > > struct nlattr **tb,
> > > +                                  struct netlink_ext_ack *extack) {
> > > +     struct eeprom_data_req_info *request =
> > > EEPROM_DATA_REQINFO(req_info);
> > > +     struct net_device *dev = req_info->dev;
> > > +
> > > +     if (!tb[ETHTOOL_A_EEPROM_DATA_OFFSET] ||
> > > +         !tb[ETHTOOL_A_EEPROM_DATA_LENGTH] ||
> > > +         !tb[ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS])
> > > +             return -EINVAL;
> > Suggestion:  Consider using i2c address 0x50 as a default if none is given.
> > 0x50 is the first 256 bytes of SFP, and all of QSFP and CMIS EEPROM.  If
> > there is a page given on an SFP device, then you know i2c address is 0x51.
> > The only thing that REQUIRES 0x51 is legacy offset 256-511 on SFP.  Keep the
> > i2c address, but make it optional for the non-standard devices that Andrew
> > has mentioned, and for that one section of SFP data that requires it.  And
> > document it for the user.
> Agree, but thought to have that i2c address default set on userspace, so
> here we expect it.

I would make it mandatory. If you default it to 0x50, the current
message structure has no way of including this information in the
reply. So user space gets some data, but it has no idea from where?

       Andrew