diff mbox series

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

Message ID 1603973277-1634-1-git-send-email-willy.liu@realtek.com (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series [net-next,1/2] net: phy: realtek: Add phy ids for RTL8226-CG/RTL8226B-CG | expand

Commit Message

Willy Liu Oct. 29, 2020, 12:07 p.m. UTC 
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 <willy.liu@realtek.com>
---
 drivers/net/phy/realtek.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)
 mode change 100644 => 100755 drivers/net/phy/realtek.c

Comments

Andrew Lunn Oct. 29, 2020, 1:37 p.m. UTC | #1 
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 <willy.liu@realtek.com>
> ---
>  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
Heiner Kallweit Oct. 29, 2020, 1:48 p.m. UTC | #2 
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 <willy.liu@realtek.com>
>> ---
>>  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.
Jakub Kicinski Oct. 29, 2020, 9:32 p.m. UTC | #3 
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 <willy.liu@realtek.com>

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);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Willy Liu Oct. 30, 2020, 2:45 a.m. UTC | #4 
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 <willy.liu@realtek.com>
> >> ---
> >>  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.
diff mbox series

Patch

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,