Re: [PATCH RFC v2] r8169: implement SFP support

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Apr 10, 2026 at 01:53:31AM +0100, Fabio Baltieri wrote:
> Implement support for reading the identification and diagnostic
> information on SFP modules for rtl8127atf devices.
> 
> This uses the sfp module, implements a GPIO devices for presence
> detection and loss of signal and i2c communication using the designware
> module.

I would probably break this up into smaller patches, GPIO, I2C, and
the swnode.

It might be you need to Cc: the GPIO Maintainers, the I2C Maintainers
for those patches.

> +static int r8169_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct rtl8169_private *tp = gpiochip_get_data(chip);
> +	int val;
> +
> +	val = r8168_mac_ocp_read(tp, 0xdc30);
> +
> +	return !!(val & BIT(offset));
> +}
> +
> +static int r8169_gpio_init(struct rtl8169_private *tp)
> +{
> +	struct gpio_chip *gc;
> +	struct pci_dev *pdev = tp->pci_dev;
> +	struct device *dev;
> +	int ret;
> +
> +	dev = &pdev->dev;
> +
> +	gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
> +	if (!gc)
> +		return -ENOMEM;
> +
> +	gc->label = devm_kasprintf(dev, GFP_KERNEL, "r8169_gpio-%x",
> +				   pci_dev_id(pdev));
> +	if (!gc->label)
> +		return -ENOMEM;
> +
> +	gc->base = -1;
> +	gc->ngpio = 16;
> +	gc->owner = THIS_MODULE;
> +	gc->parent = dev;
> +	gc->fwnode = software_node_fwnode(tp->nodes.group[SWNODE_GPIO]);
> +	gc->get = r8169_gpio_get;

So there is no set? The SFP cage has a transmit enable which is
generally connected to a GPIO output. You can use it to turn off the
laser, which phylink will do when the interface is admin down.

Can you trace the lines from the SFP cage back to the chip? At least
see if it connects back?

Are registers 0xdc30 +/- 4 used for anything? Maybe there is 16 GPI
and 16 GPO? Although that sounds like a lot of pins. Or it could be
there is a direction register, and an output register.

This looks quite good otherwise.

     Andrew