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

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

On Sun, Apr 05, 2026 at 12:46:41AM +0200, Andrew Lunn wrote:
> > 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?
> 
> Yes, that would be best. Call i2c_add_adapter() to add it to the I2C
> core. The SFP code in drivers/net/phy can then make use of it.

Hey Andrew, thanks for the pointers, I was able to instantiate the dw
i2c controller, same as txgbe/txgbe_phy.c really.

For some reasons I had to revert 560072246088, was getting a "spurious
STOP detected" but I'll follow up on that separately. Also find an issue
in the txgbe driver (I think?), sent
https://lore.kernel.org/netdev/20260405222013.5347-1-fabio.baltieri@gmail.com/
for it.

> > - Shall we (partially?) implement a struct sfp, so that functions like
> >   sfp_module_eeprom() can be used (implicitly)?
> > 
> > I assume this patch includes logic which duplicates what is available in
> > phylink/sfp already. I'd a appreciate any hints you can provide.
> 
> No. phylink etc will end up populating netdev->sfp_bus, and then
> get_module_eeprom_by_page() should then just make the module work in
> ethtool.
> 
> The interesting bit if gluing it all together, without DT. But
> txgbe_phy.c should be something you can copy from.
> 
> Does the out of tree driver give access to GPIOs connected to the SFP
> cage pins? Ideally you want those as well, for loss of signal,
> transmitter disable, knowing when a module has been inserted into the
> cage, etc.

It doesn't, but I was able to find them anyway, just dumping registers
randomly. :-) It looks like that is a bit more than "Ideally", the sfp
driver probes happily with no gpios defined but then it doesn't have
any ways to detect module changes (insertion/removal), it doesn't even
go into polling mode but even then it seems like it doesn't do detection
without a detect pin. I think the sfp probe function could be a bit more
defensive to inoperable configurations.

Anyway I was able to make detection and los work, so I have some code
with gpio, i2c and the sfp module detect and showing up in hwmon, and
sitting in waitdev state.

> 
> And you will need a PCS driver.
> 
> But first step is probably to work with the existing Base-T devices
> and convert the driver to phylink.

Ok I was going to ask how to connect the one above with the ethtool
handlers, so you are saying that first the whole driver needs to be
converted to use phylink/pcs and then that should work automatically-ish
right?

Can I send the current code I have as an RFC in the meantime to get some
early feedback? And then I guess it'll have to be reworked on top of the
phylink stuff (which I did not try to implement).

It's a pretty substantial amount of boilerplate code so I suspect Heiner
may want to refactor things about it, feels like it could live in its
own file.

Thanks for the help so far,
Fabio