Re: [PATCH net-next 2/2] net: pcs: xpcs: Add handling for 4 channel rsfec device

[Mike Marciniszyn <mike.marciniszyn@gmail.com>]

On Fri, May 08, 2026 at 06:06:34PM -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: pcs: xpcs: Add handling for 4 channel rsfec device
>
> This commit introduces the configuration of vendor-specific registers for
> alignment encoding, PCS Mode, and VL_INTVL over one or two instances. It
> derives encoding tables from the IEEE 8023-2022 specification and handles
> register conflicts for DW_PCS IP.
>
> > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> > index 0987621632a70..c42eacafad91e 100644
> > --- a/drivers/net/pcs/pcs-xpcs.c
> > +++ b/drivers/net/pcs/pcs-xpcs.c
> [ ... ]
> > @@ -1429,21 +1429,94 @@ static int xpcs_get_pma_mmd(struct dw_xpcs *xpcs)
> [ ... ]
> >  static int
> >  xpcs_config_rsfec_pma(struct dw_xpcs *xpcs, const struct pma_pcs_values *v)
> >  {
> > -	int ret = 0, i, pma_mmd;
> > +	int ret = 0, i, vl, pma_mmd;
> >
> >  	pma_mmd = xpcs_get_pma_mmd(xpcs);
> >  	if (pma_mmd < 1)
> >  		return pma_mmd;
> >
> >  	for (i = 0; ret >= 0 && i < v->lanes; i++) {
> > +		/* code word markings */
> > +		for (vl = 0; ret >= 0 && vl < 4; vl++)
> > +			ret = xpcs_config_vl_markers(xpcs, vl, i,
> > +						     !vl ? &v->vl0_markers[0] :
> > +						     &v->vl123_markers[vl - 1]);
> > +		if (ret < 0)
> > +			break;
> > +		/* vendor registers */
> > +		ret = xpcs_write_pcs_prtad(xpcs, i, DW_VR_MII_PCS_VL_INTVL,
> > +					   v->vl_intvl);
>
> Does passing the loop index i directly as the prtad (MDIO port address) here
> overwrite registers of unrelated PHY devices on the shared MDIO bus?
>
> Looking at xpcs_write_pcs_prtad() and xpcs_bus_write(), the address is passed
> directly to mdiobus_c45_write():
>
> xpcs_bus_write(struct dw_xpcs *xpcs, int prtad, int dev, u32 reg, u16 val) {
>     return mdiobus_c45_write(xpcs->mdiodev->bus, prtad, dev, reg, val);
> }
>
> Since i is just the lane index (0 to v->lanes - 1), it appears to disregard
> the actual base address of the device.
>
> Could this be offset by the base address (xpcs->mdiodev->addr) to ensure the
> writes hit the correct XPCS block instead of unrelated PHYs that might happen
> to reside at addresses 0-3 on the bus?
>

I'm going to send a v2 based on the phy_find* helpers that removes
all hardcoded addresses.

To facilitate that change,  the bus probing is now done for addr 0 and addr 1
to ensure the underlying structures support the phy_find* mechanism.

Mike