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

[Alexander H Duyck <alexander.duyck@gmail.com>]

On Mon, 2026-05-11 at 14:26 -0400, mike.marciniszyn@gmail.com wrote:
> From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>
> 
> This patch introduces the configuration of vendor specific
> registers for alignment encoding, PCS Mode, and VL_INTVL
> over the one or two instances as required.
> 
> The DW_PCS IP specification calls out the need to configure both
> lanes identically when using 2 lane modes such as 50-R2 and 100-R2, so
> the programming is repeated for each lane.
> 
> The encoding tables are derived from the IEEE 8023-2022 spec sections
> 82.2.7 and tables 82-2 and 82-3 for the alignment markers and their
> insertion.
> 
> Note that there is a conflict between VRs DW_VR_XS_PCS_DIG_STS and
> and the DW_PCS_IP DW_VR_MII_PCS_PCS_MODE.  The bit mask for
> DW_VR_XS_PCS_DIG_STS/RX_FIFO_ERR fits within the reserved bits for
> the DW_PCS IP the DW_VR_MII_PCS_PCS_MODE register so there is no issue.
> 
> There is also a confict between DW_VR_MII_PCS_VL_INTVL and
> DW_VR_MII_AN_INTR_STS but an_mode differs, so again there is no issue.
> 
> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
> Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com>
> ---
> v2:
>   - Replace xpcs_write_pcs_prtad() wiht xpcs_write_pcs_mdev() to
>     avoid hardcoded prtaddr value
> v1: https://lore.kernel.org/all/20260506190904.4029-3-mike.marciniszyn@gmail.com/
> 
>  drivers/net/pcs/pcs-xpcs.c | 91 +++++++++++++++++++++++++++++++++++++-
>  drivers/net/pcs/pcs-xpcs.h | 25 +++++++++++
>  2 files changed, 115 insertions(+), 1 deletion(-)
> 
> 

[...]

> @@ -1464,6 +1521,23 @@ xpcs_config_rsfec_pma(struct dw_xpcs *xpcs, const struct pma_pcs_values *v)
>  
>  	for (i = 0; mdev && ret >= 0 && i < v->lanes;
>  	     i++, mdev = xpcs_find_next_mdev(xpcs, mdev)) {

The use of the "find_next_mdev" essentially taints this patch with the
same issues from the first.

We know the offsets. We shouldn't be blindly reaching around for
devices and writing to them. If we need to we should be creating the
devices for the second lane of the PCS ourselves and using that instead
of letting the phy_device code autoprobe in a device based on the PMD.

Specifically we should just be writing to our bus ID and bus ID + 1 in
the case of the 50GR2 and 100G configurations as we have to configure
the 2 lanes. We shouldn't risk this thing going off the rails and
grabbing some random bus ID if it were loaded on another device. If we
need to we could make it so that the Meta device ID on the PCS is what
triggers it setting.

My advice would be to look at creating a new version of
xpcs_create_pcs_mdiodev, maybe "xpcs_create_pcs_mdiodevs", that you can
pass a lanes value to and perhaps look at making addr an array of
addresses. Then you could have that spawn the additional lanes via
mdio_device_create, record them in the dw_xpcs struct, and avoid having
to do as much blind searching to find the devices that you are
currently doing.