2020-10-29 12:11:29

by Willy Liu

Subject: [PATCH net-next 1/2] net: phy: realtek: Add phy ids for RTL8226-CG/RTL8226B-CG

Realtek single-port 2.5Gbps Ethernet PHY ids as below:
RTL8226-CG: 0x001cc800(ES)/0x001cc838(MP)
RTL8226B-CG/RTL8221B-CG: 0x001cc840(ES)/0x001cc848(MP)
ES: engineer sample
MP: mass production

Since above PHYs are already in mass production stage,
mass production id should be added.

Signed-off-by: Willy Liu <[email protected]>
---
drivers/net/phy/realtek.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
mode change 100644 => 100755 drivers/net/phy/realtek.c

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
old mode 100644
new mode 100755
index fb1db71..988f075
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -57,6 +57,9 @@
#define RTLGEN_SPEED_MASK 0x0630

#define RTL_GENERIC_PHYID 0x001cc800
+#define RTL_8226_MP_PHYID 0x001cc838
+#define RTL_8221B_ES_PHYID 0x001cc840
+#define RTL_8221B_MP_PHYID 0x001cc848

MODULE_DESCRIPTION("Realtek PHY driver");
MODULE_AUTHOR("Johnson Leung");
@@ -533,10 +536,17 @@ static int rtlgen_match_phy_device(struct phy_device *phydev)

static int rtl8226_match_phy_device(struct phy_device *phydev)
{
- return phydev->phy_id == RTL_GENERIC_PHYID &&
+ return (phydev->phy_id == RTL_GENERIC_PHYID) ||
+ (phydev->phy_id == RTL_8226_MP_PHYID) &&
rtlgen_supports_2_5gbps(phydev);
}

+static int rtl8221b_match_phy_device(struct phy_device *phydev)
+{
+ return (phydev->phy_id == RTL_8221B_ES_PHYID) ||
+ (phydev->phy_id == RTL_8221B_MP_PHYID);
+}
+
static int rtlgen_resume(struct phy_device *phydev)
{
int ret = genphy_resume(phydev);
@@ -636,7 +646,7 @@ static int rtlgen_resume(struct phy_device *phydev)
.read_mmd = rtlgen_read_mmd,
.write_mmd = rtlgen_write_mmd,
}, {
- .name = "RTL8226 2.5Gbps PHY",
+ .name = "RTL8226-CG 2.5Gbps PHY",
.match_phy_device = rtl8226_match_phy_device,
.get_features = rtl822x_get_features,
.config_aneg = rtl822x_config_aneg,
@@ -648,8 +658,8 @@ static int rtlgen_resume(struct phy_device *phydev)
.read_mmd = rtl822x_read_mmd,
.write_mmd = rtl822x_write_mmd,
}, {
- PHY_ID_MATCH_EXACT(0x001cc840),
- .name = "RTL8226B_RTL8221B 2.5Gbps PHY",
+ .name = "RTL8226B-CG_RTL8221B-CG 2.5Gbps PHY",
+ .match_phy_device = rtl8221b_match_phy_device,
.get_features = rtl822x_get_features,
.config_aneg = rtl822x_config_aneg,
.read_status = rtl822x_read_status,
--
1.9.1

2020-10-29 13:41:59

by Andrew Lunn

Subject: Re: [PATCH net-next 1/2] net: phy: realtek: Add phy ids for RTL8226-CG/RTL8226B-CG

On Thu, Oct 29, 2020 at 08:07:57PM +0800, Willy Liu wrote:
> Realtek single-port 2.5Gbps Ethernet PHY ids as below:
> RTL8226-CG: 0x001cc800(ES)/0x001cc838(MP)
> RTL8226B-CG/RTL8221B-CG: 0x001cc840(ES)/0x001cc848(MP)
> ES: engineer sample
> MP: mass production
>
> Since above PHYs are already in mass production stage,
> mass production id should be added.
>
> Signed-off-by: Willy Liu <[email protected]>
> ---
> drivers/net/phy/realtek.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
> mode change 100644 => 100755 drivers/net/phy/realtek.c
>
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> old mode 100644
> new mode 100755
> index fb1db71..988f075
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -57,6 +57,9 @@
> #define RTLGEN_SPEED_MASK 0x0630
>
> #define RTL_GENERIC_PHYID 0x001cc800
> +#define RTL_8226_MP_PHYID 0x001cc838
> +#define RTL_8221B_ES_PHYID 0x001cc840
> +#define RTL_8221B_MP_PHYID 0x001cc848
>
> MODULE_DESCRIPTION("Realtek PHY driver");
> MODULE_AUTHOR("Johnson Leung");
> @@ -533,10 +536,17 @@ static int rtlgen_match_phy_device(struct phy_device *phydev)
>
> static int rtl8226_match_phy_device(struct phy_device *phydev)
> {
> - return phydev->phy_id == RTL_GENERIC_PHYID &&
> + return (phydev->phy_id == RTL_GENERIC_PHYID) ||
> + (phydev->phy_id == RTL_8226_MP_PHYID) &&
> rtlgen_supports_2_5gbps(phydev);

Hi Willy

If i understand the code correctly, this match function is used
because the engineering sample did not use a proper ID? The mass
production part does, so there is no need to make use of this
hack. Please just list it as a normal PHY using PHY_ID_MATCH_EXACT().


> }
>
> +static int rtl8221b_match_phy_device(struct phy_device *phydev)
> +{
> + return (phydev->phy_id == RTL_8221B_ES_PHYID) ||
> + (phydev->phy_id == RTL_8221B_MP_PHYID);
> +}

Again, these appear to be well defined ID, so just list them in the
normal ways.

Andrew

2020-10-29 13:51:34

by Heiner Kallweit

Subject: Re: [PATCH net-next 1/2] net: phy: realtek: Add phy ids for RTL8226-CG/RTL8226B-CG

On 29.10.2020 14:37, Andrew Lunn wrote:
> On Thu, Oct 29, 2020 at 08:07:57PM +0800, Willy Liu wrote:
>> Realtek single-port 2.5Gbps Ethernet PHY ids as below:
>> RTL8226-CG: 0x001cc800(ES)/0x001cc838(MP)
>> RTL8226B-CG/RTL8221B-CG: 0x001cc840(ES)/0x001cc848(MP)
>> ES: engineer sample
>> MP: mass production
>>
>> Since above PHYs are already in mass production stage,
>> mass production id should be added.
>>
>> Signed-off-by: Willy Liu <[email protected]>
>> ---
>> drivers/net/phy/realtek.c | 18 ++++++++++++++----
>> 1 file changed, 14 insertions(+), 4 deletions(-)
>> mode change 100644 => 100755 drivers/net/phy/realtek.c
>>
>> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
>> old mode 100644
>> new mode 100755
>> index fb1db71..988f075
>> --- a/drivers/net/phy/realtek.c
>> +++ b/drivers/net/phy/realtek.c
>> @@ -57,6 +57,9 @@
>> #define RTLGEN_SPEED_MASK 0x0630
>>
>> #define RTL_GENERIC_PHYID 0x001cc800
>> +#define RTL_8226_MP_PHYID 0x001cc838
>> +#define RTL_8221B_ES_PHYID 0x001cc840
>> +#define RTL_8221B_MP_PHYID 0x001cc848
>>
>> MODULE_DESCRIPTION("Realtek PHY driver");
>> MODULE_AUTHOR("Johnson Leung");
>> @@ -533,10 +536,17 @@ static int rtlgen_match_phy_device(struct phy_device *phydev)
>>
>> static int rtl8226_match_phy_device(struct phy_device *phydev)
>> {
>> - return phydev->phy_id == RTL_GENERIC_PHYID &&
>> + return (phydev->phy_id == RTL_GENERIC_PHYID) ||
>> + (phydev->phy_id == RTL_8226_MP_PHYID) &&
>> rtlgen_supports_2_5gbps(phydev);
>
> Hi Willy
>
> If i understand the code correctly, this match function is used
> because the engineering sample did not use a proper ID? The mass
> production part does, so there is no need to make use of this
> hack. Please just list it as a normal PHY using PHY_ID_MATCH_EXACT().
>
Right. My understanding:
These PHY's exist as standalone chips and integrated with RTL8125 MAC.
IIRC for RTL8125A the integrated PHY reports RTL_GENERIC_PHYID, since
RTL8125B it reports the same PHYID as the standalone model.

2020-10-29 21:34:30

by Jakub Kicinski

Subject: Re: [PATCH net-next 1/2] net: phy: realtek: Add phy ids for RTL8226-CG/RTL8226B-CG

On Thu, 29 Oct 2020 20:07:57 +0800 Willy Liu wrote:
> Realtek single-port 2.5Gbps Ethernet PHY ids as below:
> RTL8226-CG: 0x001cc800(ES)/0x001cc838(MP)
> RTL8226B-CG/RTL8221B-CG: 0x001cc840(ES)/0x001cc848(MP)
> ES: engineer sample
> MP: mass production
>
> Since above PHYs are already in mass production stage,
> mass production id should be added.
>
> Signed-off-by: Willy Liu <[email protected]>

drivers/net/phy/realtek.c: In function ‘rtl8226_match_phy_device’:
drivers/net/phy/realtek.c:540:47: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
540 | (phydev->phy_id == RTL_8226_MP_PHYID) &&
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
541 | rtlgen_supports_2_5gbps(phydev);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

2020-10-30 02:49:03

by Willy Liu

Subject: RE: [PATCH net-next 1/2] net: phy: realtek: Add phy ids for RTL8226-CG/RTL8226B-CG

On Thu, Oct 29, 2020 21:49, Heiner Hallweit wrote:
> On 29.10.2020 14:37, Andrew Lunn wrote:
> > On Thu, Oct 29, 2020 at 08:07:57PM +0800, Willy Liu wrote:
> >> Realtek single-port 2.5Gbps Ethernet PHY ids as below:
> >> RTL8226-CG: 0x001cc800(ES)/0x001cc838(MP)
> >> RTL8226B-CG/RTL8221B-CG: 0x001cc840(ES)/0x001cc848(MP)
> >> ES: engineer sample
> >> MP: mass production
> >>
> >> Since above PHYs are already in mass production stage, mass
> >> production id should be added.
> >>
> >> Signed-off-by: Willy Liu <[email protected]>
> >> ---
> >> drivers/net/phy/realtek.c | 18 ++++++++++++++----
> >> 1 file changed, 14 insertions(+), 4 deletions(-) mode change 100644
> >> => 100755 drivers/net/phy/realtek.c
> >>
> >> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> >> old mode 100644 new mode 100755 index fb1db71..988f075
> >> --- a/drivers/net/phy/realtek.c
> >> +++ b/drivers/net/phy/realtek.c
> >> @@ -57,6 +57,9 @@
> >> #define RTLGEN_SPEED_MASK 0x0630
> >>
> >> #define RTL_GENERIC_PHYID 0x001cc800
> >> +#define RTL_8226_MP_PHYID 0x001cc838
> >> +#define RTL_8221B_ES_PHYID 0x001cc840
> >> +#define RTL_8221B_MP_PHYID 0x001cc848
> >>
> >> MODULE_DESCRIPTION("Realtek PHY driver");
> MODULE_AUTHOR("Johnson
> >> Leung"); @@ -533,10 +536,17 @@ static int
> >> rtlgen_match_phy_device(struct phy_device *phydev)
> >>
> >> static int rtl8226_match_phy_device(struct phy_device *phydev) {
> >> - return phydev->phy_id == RTL_GENERIC_PHYID &&
> >> + return (phydev->phy_id == RTL_GENERIC_PHYID) ||
> >> + (phydev->phy_id == RTL_8226_MP_PHYID) &&
> >> rtlgen_supports_2_5gbps(phydev);
> >
> > Hi Willy
> >
> > If i understand the code correctly, this match function is used
> > because the engineering sample did not use a proper ID? The mass
> > production part does, so there is no need to make use of this hack.
> > Please just list it as a normal PHY using PHY_ID_MATCH_EXACT().
> >
> Right. My understanding:
> These PHY's exist as standalone chips and integrated with RTL8125 MAC.
> IIRC for RTL8125A the integrated PHY reports RTL_GENERIC_PHYID, since
> RTL8125B it reports the same PHYID as the standalone model.
Hi Andrew && Heiner,
Thanks for your information, I will create drivers for RTL8226-CG & RTL8221B-CG
as standalone models.

> ------Please consider the environment before printing this e-mail.