Re: [v2,2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

Daniel Golle
2026-01-17T23:40:53Z

On Sat, Jan 17, 2026 at 03:20:06PM -0800, 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.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2
>
> This commit adds support for translating MDIO_MMD_VEND2 registers back
> to Clause-22 registers for PHYs on a C22-only bus. It also renames
> RTL_PHYSR to RTL_VND2_PHYSR and changes the register access method in
> several read_status functions.
>
> > diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> > index a593150b6cbb..40cae9968d20 100644
> > --- a/drivers/net/phy/realtek/realtek_main.c
> > +++ b/drivers/net/phy/realtek/realtek_main.c
>
> [ ... ]
>
> > @@ -1156,7 +1156,8 @@ static int rtlgen_read_status(struct phy_device *phydev)
> > if (!phydev->link)
> > return 0;
> >
> > - val = phy_read(phydev, RTL_PHYSR);
> > + val = phy_read_paged(phydev, RTL822X_VND2_TO_PAGE(RTL_VND2_PHYSR),
> > + RTL822X_VND2_TO_PAGE_REG(RTL_VND2_PHYSR));
>
> This changes rtlgen_read_status() from reading C22 register MII_RESV2
> (0x1a) directly to using paged access at page 0xa43, register 18.

Yeah. Just that this is not part of the series submitted.
It's rather a (halucinated) partial revert of
[v2,4/5] net: phy: realtek: demystify PHYSR register location

>
> rtlgen_read_status() is used by RTL8211F, RTL8211F-VD, and Generic FE-GE
> Realtek PHY drivers. Are these Gigabit-only PHYs guaranteed to have the
> PHYSR at the same paged location as the RTL822x 2.5GbE PHYs?
>
> I notice this change is reverted in a later commit in the series:
> efef1a8042ec ("net: phy: realtek: demystify PHYSR register location")
> which changes back to:
> val = phy_read(phydev, RTL_PHYSR);
>
> with the note "Turns out that register address RTL_VND2_PHYSR (0xa434)
> maps to Clause-22 register MII_RESV2".
>
> Is the paged access in this intermediate commit correct, or could it
> cause issues during bisection?
>
> [ ... ]
>
> > @@ -2127,7 +2137,7 @@ static int rtlgen_sfp_read_status(struct phy_device *phydev)
> > if (!phydev->link)
> > return 0;
> >
> > - val = phy_read(phydev, RTL_PHYSR);
> > + val = rtlgen_read_vend2(phydev, RTL_VND2_PHYSR);
>
> Similarly, this changes from C22 register read to C45 MDIO_MMD_VEND2
> access via rtlgen_read_vend2(). The SFP PHY mode uses PHY_IS_INTERNAL
> and is used by r8169 for 2.5GbE chips in SFP mode.

Just that, again, this is the revert, and not the actual patch I'm
submitting. And yes, it has been tested on SFP NICs.

>
> This is also reverted in the same later commit to:
> val = phy_read(phydev, RTL_PHYSR);
>
> Does rtlgen_read_vend2() work correctly for all PHYs that could be in
> SFP mode?
> --
> pw-bot: cr