RE: [PATCH net-next 1/2] net: ethtool: Add new parameters and a function to support EPL

[Danielle Ratson]

> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Friday, 6 September 2024 18:36
> To: Danielle Ratson <danieller@xxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx;
> kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; yuehaibing@xxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Petr Machata <petrm@xxxxxxxxxx>
> Subject: Re: [PATCH net-next 1/2] net: ethtool: Add new parameters and a
> function to support EPL
> 
> > +/* For accessing the EPL field on page 9Fh, the allowable length
> > +extension is
> > + * min(i, 255) byte octets where i specifies the allowable additional
> > +number of
> > + * byte octets in a READ or a WRITE.
> > + */
> > +u32 ethtool_cmis_get_max_epl_size(u8 num_of_byte_octs) {
> > +	return 8 * (1 + min_t(u8, num_of_byte_octs, 255)); }
> 
> Does this get mapped to a 255 byte I2C bus transfer?
> 
> https://elixir.bootlin.com/linux/v6.11-rc6/source/drivers/net/phy/sfp.c#L218
> 
> /* SFP_EEPROM_BLOCK_SIZE is the size of data chunk to read the EEPROM
>  * at a time. Some SFP modules and also some Linux I2C drivers do not like
>  * reads longer than 16 bytes.
>  */
> #define SFP_EEPROM_BLOCK_SIZE	16
> 
> If an SMBUS is being used, rather than I2C, there is a hard limit of
> 32 bytes in a message transfer.
> 
> I've not looked in details at these patches, but maybe you need a mechanism
> to ask the hardware or I2C driver what it can actually do?
> Is it possible to say LPL is the only choice?
> 
> 	Andrew

Hi Andrew,

Thanks for your reply.

As I wrote in the commit message, the EPL is an extended type of data payload in which you can send the firmware image, for example, in blocks.
The LPL contains ap to 128 bytes, and the EPL up to 2048. The exact allowed extension can be derived from the 'readWriteLengthExt' and the two functions (ethtool_cmis_get_max_lpl/epl_size ()) that calculates the specific value of the module from it.

The obligations on the chunks that are related to the bus type limit, should be referred to in the driver itself. 
The driver should be the responsible for bus that cannot read/write larger chunks and then split it to the supported size chunks accordingly. 

Hope it makes more sense now.

Thanks,
Danielle