From: Daniel Golle <daniel@makrotopia.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
michael@fossekall.de, linux@armlinux.org.uk, edumazet@google.com,
andrew@lunn.ch, olek2@wp.pl, davem@davemloft.net,
vladimir.oltean@nxp.com, netdev@vger.kernel.org,
pabeni@redhat.com
Subject: Re: [v2,2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2
Date: Sat, 17 Jan 2026 23:40:36 +0000 [thread overview]
Message-ID: <aWwd9LoVI6j8JBTc@makrotopia.org> (raw)
In-Reply-To: <20260117232006.1000673-1-kuba@kernel.org>
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
next prev parent reply other threads:[~2026-01-17 23:40 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-13 3:43 [PATCH v2 0/5] net: phy: realtek: simplify and reunify C22/C45 drivers Daniel Golle
2026-01-13 3:44 ` [PATCH v2 1/5] net: phy: realtek: support interrupt also for C22 variants Daniel Golle
2026-01-13 3:44 ` [PATCH v2 2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2 Daniel Golle
2026-01-17 23:20 ` [v2,2/5] " Jakub Kicinski
2026-01-17 23:40 ` Daniel Golle [this message]
2026-01-17 23:55 ` Jakub Kicinski
2026-01-18 0:05 ` Daniel Golle
2026-01-18 0:10 ` Chris Mason
2026-01-18 0:17 ` Jakub Kicinski
2026-01-18 0:35 ` Chris Mason
2026-01-13 3:44 ` [PATCH v2 3/5] net: phy: realtek: reunify C22 and C45 drivers Daniel Golle
2026-01-13 3:44 ` [PATCH v2 4/5] net: phy: realtek: demystify PHYSR register location Daniel Golle
2026-01-13 3:44 ` [PATCH v2 5/5] net: phy: realtek: simplify bogus paged operations Daniel Golle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aWwd9LoVI6j8JBTc@makrotopia.org \
--to=daniel@makrotopia.org \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=michael@fossekall.de \
--cc=netdev@vger.kernel.org \
--cc=olek2@wp.pl \
--cc=pabeni@redhat.com \
--cc=vladimir.oltean@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox