[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 3 Mar 2024 22:14:20 +0100
From: Heiner Kallweit <hkallweit1@...il.com>
To: Daniel Golle <daniel@...rotopia.org>
Cc: Eric Woudstra <ericwouds@...il.com>, Andrew Lunn <andrew@...n.ch>,
Russell King <linux@...linux.org.uk>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Marek BehĂșn <kabel@...nel.org>,
Frank Wunderlich <frank-w@...lic-files.de>, netdev@...r.kernel.org,
Alexander Couzens <lynxis@...0.eu>
Subject: Re: [PATCH v2 net-next 1/7] net: phy: realtek: configure SerDes mode
for rtl822x/8251b PHYs
On 03.03.2024 22:08, Daniel Golle wrote:
> On Sun, Mar 03, 2024 at 09:42:31PM +0100, Heiner Kallweit wrote:
>> On 03.03.2024 11:28, Eric Woudstra wrote:
>>> From: Alexander Couzens <lynxis@...0.eu>
>>>
>>> The rtl822x series and rtl8251b support switching SerDes mode between
>>> 2500base-x and sgmii based on the negotiated copper speed.
>>>
>>> Configure this switching mode according to SerDes modes supported by
>>> host.
>>>
>>> There is an additional datasheet for RTL8226B/RTL8221B called
>>> "SERDES MODE SETTING FLOW APPLICATION NOTE" where this sequence to
>>> setup interface and rate adapter mode.
>>>
>>> However, there is no documentation about the meaning of registers
>>> and bits, it's literally just magic numbers and pseudo-code.
>>>
>>> Signed-off-by: Alexander Couzens <lynxis@...0.eu>
>>> [ refactored, dropped HiSGMII mode and changed commit message ]
>>> Signed-off-by: Marek BehĂșn <kabel@...nel.org>
>>> [ changed rtl822x_update_interface() to use vendor register ]
>>> [ always fill in possible interfaces ]
>>> Signed-off-by: Eric Woudstra <ericwouds@...il.com>
>>> ---
>>> drivers/net/phy/realtek.c | 99 ++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 97 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
>>> index 1fa70427b2a2..8a876e003774 100644
>>> --- a/drivers/net/phy/realtek.c
>>> +++ b/drivers/net/phy/realtek.c
>>> @@ -54,6 +54,16 @@
>>> RTL8201F_ISR_LINK)
>>> #define RTL8201F_IER 0x13
>>>
>>> +#define RTL822X_VND1_SERDES_OPTION 0x697a
>>> +#define RTL822X_VND1_SERDES_OPTION_MODE_MASK GENMASK(5, 0)
>>> +#define RTL822X_VND1_SERDES_OPTION_MODE_2500BASEX_SGMII 0
>>> +#define RTL822X_VND1_SERDES_OPTION_MODE_2500BASEX 2
>>> +
>>> +#define RTL822X_VND1_SERDES_CTRL3 0x7580
>>> +#define RTL822X_VND1_SERDES_CTRL3_MODE_MASK GENMASK(5, 0)
>>> +#define RTL822X_VND1_SERDES_CTRL3_MODE_SGMII 0x02
>>> +#define RTL822X_VND1_SERDES_CTRL3_MODE_2500BASEX 0x16
>>> +
>>> #define RTL8366RB_POWER_SAVE 0x15
>>> #define RTL8366RB_POWER_SAVE_ON BIT(12)
>>>
>>> @@ -659,6 +669,60 @@ static int rtl822x_write_mmd(struct phy_device *phydev, int devnum, u16 regnum,
>>> return ret;
>>> }
>>>
>>> +static int rtl822x_config_init(struct phy_device *phydev)
>>> +{
>>> + bool has_2500, has_sgmii;
>>> + u16 mode;
>>> + int ret;
>>> +
>>> + has_2500 = test_bit(PHY_INTERFACE_MODE_2500BASEX,
>>> + phydev->host_interfaces) ||
>>> + phydev->interface == PHY_INTERFACE_MODE_2500BASEX;
>>> +
>>> + has_sgmii = test_bit(PHY_INTERFACE_MODE_SGMII,
>>> + phydev->host_interfaces) ||
>>> + phydev->interface == PHY_INTERFACE_MODE_SGMII;
>>> +
>>> + /* fill in possible interfaces */
>>> + __assign_bit(PHY_INTERFACE_MODE_2500BASEX, phydev->possible_interfaces,
>>> + has_2500);
>>> + __assign_bit(PHY_INTERFACE_MODE_SGMII, phydev->possible_interfaces,
>>> + has_sgmii);
>>> +
>>> + if (!has_2500 && !has_sgmii)
>>> + return 0;
>>> +
>>> + /* determine SerDes option mode */
>>> + if (has_2500 && !has_sgmii)
>>> + mode = RTL822X_VND1_SERDES_OPTION_MODE_2500BASEX;
>>> + else
>>> + mode = RTL822X_VND1_SERDES_OPTION_MODE_2500BASEX_SGMII;
>>> +
>>> + /* the following sequence with magic numbers sets up the SerDes
>>> + * option mode
>>> + */
>>> + ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x75f3, 0);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = phy_modify_mmd_changed(phydev, MDIO_MMD_VEND1,
>>> + RTL822X_VND1_SERDES_OPTION,
>>> + RTL822X_VND1_SERDES_OPTION_MODE_MASK,
>>> + mode);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x6a04, 0x0503);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x6f10, 0xd455);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x6f11, 0x8020);
>>> +}
>>> +
>>> static int rtl822x_get_features(struct phy_device *phydev)
>>> {
>>> int val;
>>> @@ -695,6 +759,28 @@ static int rtl822x_config_aneg(struct phy_device *phydev)
>>> return __genphy_config_aneg(phydev, ret);
>>> }
>>>
>>> +static void rtl822x_update_interface(struct phy_device *phydev)
>>> +{
>>> + int val;
>>> +
>>> + if (!phydev->link)
>>> + return;
>>> +
>>> + /* Change interface according to serdes mode */
>>> + val = phy_read_mmd(phydev, MDIO_MMD_VEND1, RTL822X_VND1_SERDES_CTRL3);
>>> + if (val < 0)
>>> + return;
>>> +
>>> + switch (val & RTL822X_VND1_SERDES_CTRL3_MODE_MASK) {
>>> + case RTL822X_VND1_SERDES_CTRL3_MODE_2500BASEX:
>>> + phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
>>> + break;
>>> + case RTL822X_VND1_SERDES_CTRL3_MODE_SGMII:
>>> + phydev->interface = PHY_INTERFACE_MODE_SGMII;
>>> + break;
>>> + }
>>> +}
>>> +
>>> static int rtl822x_read_status(struct phy_device *phydev)
>>> {
>>> int ret;
>>> @@ -709,11 +795,13 @@ static int rtl822x_read_status(struct phy_device *phydev)
>>> lpadv);
>>> }
>>>
>>> - ret = genphy_read_status(phydev);
>>> + ret = rtlgen_read_status(phydev);
>>> if (ret < 0)
>>> return ret;
>>>
>>> - return rtlgen_get_speed(phydev);
>>> + rtl822x_update_interface(phydev);
>>> +
>>> + return 0;
>>> }
>>>
>>> static bool rtlgen_supports_2_5gbps(struct phy_device *phydev)
>>> @@ -976,6 +1064,7 @@ static struct phy_driver realtek_drvs[] = {
>>> .match_phy_device = rtl8226_match_phy_device,
>>> .get_features = rtl822x_get_features,
>>> .config_aneg = rtl822x_config_aneg,
>>> + .config_init = rtl822x_config_init,
>>
>> Did you test this (and the rest of the series) on RTL8125A, where
>> MMD register access for the integrated 2.5G PHY isn't supported?
>
> None of this should be done on RealTek NICs, and the easiest way to
> prevent that is to test if phydev->phylink is NULL or not (as the NIC
> driver doesn't use phylink but rather just calls phy_connect_direct()).
>
Right. I'm asking because this PHY driver entry is used on RTL8125A.
> Also note that the datasheet with the magic SerDes config sequences
> lists only 2nd generation 2.5G PHYs as supported:
> RTL8226B
> RTL8221B(I)
> RTL8221B(I)-VB
> RTL8221B(I)-VM
>
> So RTL8226 and RTL8226-CG are **not** listed there and being from the
> 1st generation of RealTek 2.5G PHYs I would assume that it won't
> support setting up the SerDes mode in this way.
>
> Afaik those anyway were only used in early RealTek-based 2.5G switches
> and maybe NICs, I'm pretty sure you won't find them inside SFP modules
> or as part non-RealTek-based routers or switches -- hence switching
> SerDes mode most likely anyway will never be required for those.
>
>>
>>> .read_status = rtl822x_read_status,
>>> .suspend = genphy_suspend,
>>> .resume = rtlgen_resume,
>>> @@ -988,6 +1077,7 @@ static struct phy_driver realtek_drvs[] = {
>>> .name = "RTL8226B_RTL8221B 2.5Gbps PHY",
>>> .get_features = rtl822x_get_features,
>>> .config_aneg = rtl822x_config_aneg,
>>> + .config_init = rtl822x_config_init,
>>> .read_status = rtl822x_read_status,
>>> .suspend = genphy_suspend,
>>> .resume = rtlgen_resume,
>>> @@ -1000,6 +1090,7 @@ static struct phy_driver realtek_drvs[] = {
>>> .name = "RTL8226-CG 2.5Gbps PHY",
>>> .get_features = rtl822x_get_features,
>>> .config_aneg = rtl822x_config_aneg,
>>> + .config_init = rtl822x_config_init,
>>> .read_status = rtl822x_read_status,
>>> .suspend = genphy_suspend,
>>> .resume = rtlgen_resume,
>>> @@ -1010,6 +1101,7 @@ static struct phy_driver realtek_drvs[] = {
>>> .name = "RTL8226B-CG_RTL8221B-CG 2.5Gbps PHY",
>>> .get_features = rtl822x_get_features,
>>> .config_aneg = rtl822x_config_aneg,
>>> + .config_init = rtl822x_config_init,
>>> .read_status = rtl822x_read_status,
>>> .suspend = genphy_suspend,
>>> .resume = rtlgen_resume,
>>> @@ -1019,6 +1111,7 @@ static struct phy_driver realtek_drvs[] = {
>>> PHY_ID_MATCH_EXACT(0x001cc849),
>>> .name = "RTL8221B-VB-CG 2.5Gbps PHY",
>>> .get_features = rtl822x_get_features,
>>> + .config_init = rtl822x_config_init,
>>> .config_aneg = rtl822x_config_aneg,
>>> .read_status = rtl822x_read_status,
>>> .suspend = genphy_suspend,
>>> @@ -1030,6 +1123,7 @@ static struct phy_driver realtek_drvs[] = {
>>> .name = "RTL8221B-VM-CG 2.5Gbps PHY",
>>> .get_features = rtl822x_get_features,
>>> .config_aneg = rtl822x_config_aneg,
>>> + .config_init = rtl822x_config_init,
>>> .read_status = rtl822x_read_status,
>>> .suspend = genphy_suspend,
>>> .resume = rtlgen_resume,
>>> @@ -1040,6 +1134,7 @@ static struct phy_driver realtek_drvs[] = {
>>> .name = "RTL8251B 5Gbps PHY",
>>> .get_features = rtl822x_get_features,
>>> .config_aneg = rtl822x_config_aneg,
>>> + .config_init = rtl822x_config_init,
>>> .read_status = rtl822x_read_status,
>>> .suspend = genphy_suspend,
>>> .resume = rtlgen_resume,
>>
Powered by blists - more mailing lists