Re: [PATCH net-next v1] r8169: implement get_module functions for rtl8127atf

[Fabio Baltieri <fabio.baltieri@gmail.com>]

On Sat, Apr 04, 2026 at 11:55:25PM +0200, Heiner Kallweit wrote:
> On 04.04.2026 22:13, Fabio Baltieri wrote:
> > Implement get_module_info and get_module_eeprom for supported devices,
> > right now that's the rtl8127atf.
> > 
> > This enables using ethtool to retrieve the transceiver identification
> > and diagnostic data.
> > 
> > Signed-off-by: Fabio Baltieri <fabio.baltieri@gmail.com>
> > ---
> > 
> > Hi,
> > 
> > Realtek released a new out of tree driver for the rtl8127 and it
> > includes some code that exposes the I2C controller connected to the SFP
> > port.
> > 
> > This patch implements get_module_info and get_module_eeprom based on the
> > information found on that driver, it allows ethtool to fetch the SFP
> > module information, which is useful both to identify the module and to
> > track the health of fiber optic transceivers.
> > 
> > The logic for the SFF standard and i2c address selection is borrowed
> > directly from the aquantia/atlantic driver, I don't have a lot of stuff
> > to test it but I was able to trigger all code paths from a fiber optic
> > transceiver and an SFP cable I have available.
> > 
> 
> Hi Fabio, thanks for the patch. Being able to access the SFP I2C bus is an
> important step towards future phylink/SFP support.
> 
> @Russell/@Andrew
> I'm not really familiar with the phylink and sfp code. And I would like to have
> the functionality being implemented here reusable once we do further steps
> towards phylink support in r8169. So how shall the code be modeled, which
> components are needed?
> - Shall the SFP I2C bus be modeled as a struct i2c_adapter?
> - Shall we (partially?) implement a struct sfp, so that functions like
>   sfp_module_eeprom() can be used (implicitly)?

Yeah happy to hear any input on it, the controller actually seems to be
a designware IP and there is some reusable code for it but what I've
seen done for other drivers for this functionality is to implement a
minimum i2c "read bulk" functionality in the driver itself, so that's
what I proposed here too, the code is fairly simple after all. However I
did borrow some logic much verbatim from the atlantis driver as I
mentioned, happy to hear if there's a path to reuse.

> 
> I assume this patch includes logic which duplicates what is available in
> phylink/sfp already. I'd a appreciate any hints you can provide.
> 
> Thanks, Heiner