From: Daniel Golle <daniel@makrotopia.org>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
Russell King <linux@armlinux.org.uk>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Vladimir Oltean <vladimir.oltean@nxp.com>,
Michael Klein <michael@fossekall.de>,
Aleksander Jan Bajkowski <olek2@wp.pl>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 4/5] net: phy: realtek: demystify PHYSR register location
Date: Fri, 9 Jan 2026 12:26:42 +0000 [thread overview]
Message-ID: <aWD0AuYGO9ZJm9wa@makrotopia.org> (raw)
In-Reply-To: <1261b3d5-3e09-4dd6-8645-fd546cbdce62@gmail.com>
On Fri, Jan 09, 2026 at 08:32:33AM +0100, Heiner Kallweit wrote:
> On 1/9/2026 4:03 AM, Daniel Golle wrote:
> > Turns out that register address RTL_VND2_PHYSR (0xa434) maps to
> > Clause-22 register MII_RESV2. Use that to get rid of yet another magic
> > number, and rename access macros accordingly.
> >
>
> RTL_VND2_PHYSR is documented in the datasheet, at least for RTL8221B(I)-VB-CG.
> (this datasheet is publicly available, I don't have access to other datasheets)
> MII_RESV2 isn't documented there. Is MII_RESV2 documented in any other datasheet?
No datasheet mentions the nature of paging only affecting registers
0x10~0x17, I've figured that out by code analysis and testing (ie.
dumping all registers for all known/used pages using mdio-tools in
userspace, and writing to PHYCR1 toggling BIT(13) and confirming that it
affects the PHY in the expected way). Don't ask me why they ommit this
in the datasheets, I suspect the people writing the datasheets are given
some auto-generated code and also don't have unterstanding of the actual
internals (maybe to "protect" their precious IP?).
Anyway, as RTL_VND2_PHYSR is 0xa434 on MDIO_MMD_VEND2, and we know that
0xa400~0xa43c maps to the standard C22 registers, I concluded that
0xa434 on MDIO_MMD_VEND2 is identical to C22 register 0x1a, ie.
MII_RESV2. I've also noticed that the mechanism to translate registers
on MDIO_MMD_VEND2 to paged C22 registers only makes use of registers
0x10~0x17, so it became apparent that other registers are not affected
by paging.
I've confirmed all that by testing on RTL8211F and RTL8221B. As pointed
out this also holds true for internal PHYs on r8169 which emulate C22
registers in the exact same way. Hence the PHY driver can be simplified,
as there is no need to set and restore the page around the reading of
PHYSR.
>
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> > drivers/net/phy/realtek/realtek_main.c | 24 ++++++++++++------------
> > 1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> > index d07d60bc1ce34..5712372c71f91 100644
> > --- a/drivers/net/phy/realtek/realtek_main.c
> > +++ b/drivers/net/phy/realtek/realtek_main.c
> > @@ -178,12 +178,12 @@
> > #define RTL9000A_GINMR 0x14
> > #define RTL9000A_GINMR_LINK_STATUS BIT(4)
> >
> > -#define RTL_VND2_PHYSR 0xa434
> > -#define RTL_VND2_PHYSR_DUPLEX BIT(3)
> > -#define RTL_VND2_PHYSR_SPEEDL GENMASK(5, 4)
> > -#define RTL_VND2_PHYSR_SPEEDH GENMASK(10, 9)
> > -#define RTL_VND2_PHYSR_MASTER BIT(11)
> > -#define RTL_VND2_PHYSR_SPEED_MASK (RTL_VND2_PHYSR_SPEEDL | RTL_VND2_PHYSR_SPEEDH)
> > +#define RTL_PHYSR MII_RESV2
> > +#define RTL_PHYSR_DUPLEX BIT(3)
> > +#define RTL_PHYSR_SPEEDL GENMASK(5, 4)
> > +#define RTL_PHYSR_SPEEDH GENMASK(10, 9)
> > +#define RTL_PHYSR_MASTER BIT(11)
> > +#define RTL_PHYSR_SPEED_MASK (RTL_PHYSR_SPEEDL | RTL_PHYSR_SPEEDH)
> >
> > #define RTL_MDIO_PCS_EEE_ABLE 0xa5c4
> > #define RTL_MDIO_AN_EEE_ADV 0xa5d0
> > @@ -1102,12 +1102,12 @@ static void rtlgen_decode_physr(struct phy_device *phydev, int val)
> > * 0: Half Duplex
> > * 1: Full Duplex
> > */
> > - if (val & RTL_VND2_PHYSR_DUPLEX)
> > + if (val & RTL_PHYSR_DUPLEX)
> > phydev->duplex = DUPLEX_FULL;
> > else
> > phydev->duplex = DUPLEX_HALF;
> >
> > - switch (val & RTL_VND2_PHYSR_SPEED_MASK) {
> > + switch (val & RTL_PHYSR_SPEED_MASK) {
> > case 0x0000:
> > phydev->speed = SPEED_10;
> > break;
> > @@ -1135,7 +1135,7 @@ static void rtlgen_decode_physr(struct phy_device *phydev, int val)
> > * 1: Master Mode
> > */
> > if (phydev->speed >= 1000) {
> > - if (val & RTL_VND2_PHYSR_MASTER)
> > + if (val & RTL_PHYSR_MASTER)
> > phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
> > else
> > phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
> > @@ -1155,8 +1155,7 @@ static int rtlgen_read_status(struct phy_device *phydev)
> > if (!phydev->link)
> > return 0;
> >
> > - val = phy_read_paged(phydev, RTL822X_VND2_TO_PAGE(RTL_VND2_PHYSR),
> > - RTL822X_VND2_TO_PAGE_REG(RTL_VND2_PHYSR));
> > + val = phy_read(phydev, RTL_PHYSR);
> > if (val < 0)
> > return val;
> >
> > @@ -1622,7 +1621,8 @@ static int rtl822x_c45_read_status(struct phy_device *phydev)
> > }
> >
> > /* Read actual speed from vendor register. */
> > - val = phy_read_mmd(phydev, MDIO_MMD_VEND2, RTL_VND2_PHYSR);
> > + val = phy_read_mmd(phydev, MDIO_MMD_VEND2,
> > + RTL822X_VND2_C22_REG(RTL_PHYSR));
> > if (val < 0)
> > return val;
> >
>
next prev parent reply other threads:[~2026-01-09 12:26 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-09 3:02 [PATCH net-next 0/5] net: phy: realtek: simplify and reunify C22/C45 drivers Daniel Golle
2026-01-09 3:03 ` [PATCH net-next 1/5] net: phy: realtek: support interrupt also for C22 variants Daniel Golle
2026-01-09 3:03 ` [PATCH net-next 2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2 Daniel Golle
2026-01-09 7:27 ` Heiner Kallweit
2026-01-09 21:32 ` Russell King (Oracle)
2026-01-09 22:21 ` Daniel Golle
2026-01-09 3:03 ` [PATCH net-next 3/5] net: phy: realtek: reunify C22 and C45 drivers Daniel Golle
2026-01-09 13:18 ` Andrew Lunn
2026-01-09 13:25 ` Daniel Golle
2026-01-10 14:28 ` Heiner Kallweit
2026-01-09 3:03 ` [PATCH net-next 4/5] net: phy: realtek: demystify PHYSR register location Daniel Golle
2026-01-09 7:32 ` Heiner Kallweit
2026-01-09 12:26 ` Daniel Golle [this message]
2026-01-09 17:31 ` Daniel Golle
2026-01-09 20:19 ` Daniel Golle
2026-01-09 3:03 ` [PATCH net-next 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=aWD0AuYGO9ZJm9wa@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