diff mbox series

[V2,net-next] net: phy: realtek: add support for RTL8221F(D)(I)-VD-CG

Message ID 20220816194859.2369-1-wei.fang@nxp.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [V2,net-next] net: phy: realtek: add support for RTL8221F(D)(I)-VD-CG | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 85 this patch: 85
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 85 this patch: 85
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Wei Fang Aug. 16, 2022, 7:48 p.m. UTC 
From: Clark Wang <xiaoning.wang@nxp.com>

RTL8221F(D)(I)-VD-CG is the pin-to-pin upgrade chip from
RTL8221F(D)(I)-CG.

Add new PHY ID for this chip.
It does not support RTL8211F_PHYCR2 anymore, so remove the w/r operation
of this register.

Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
V2 change:
1. Commit message changed, RTL8221 instead of RTL8821.
2. Add has_phycr2 to struct rtl821x_priv.
---
 drivers/net/phy/realtek.c | 44 ++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 12 deletions(-)

Comments

Heiner Kallweit Aug. 16, 2022, 1:22 p.m. UTC | #1 
On 16.08.2022 21:48, wei.fang@nxp.com wrote:
> From: Clark Wang <xiaoning.wang@nxp.com>
> 
> RTL8221F(D)(I)-VD-CG is the pin-to-pin upgrade chip from
> RTL8221F(D)(I)-CG.

Here you talk about RTL8221, in the driver struct definition you say RTL8211.
You changed the naming already for v2. It would be time for you to clarify
which chip you actually mean.
RTL8221 is a 2.5Gbps PHY, however you don't handle this mode in your code.

> 
> Add new PHY ID for this chip.
> It does not support RTL8211F_PHYCR2 anymore, so remove the w/r operation
> of this register.
> 
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> V2 change:
> 1. Commit message changed, RTL8221 instead of RTL8821.
> 2. Add has_phycr2 to struct rtl821x_priv.
> ---
>  drivers/net/phy/realtek.c | 44 ++++++++++++++++++++++++++++-----------
>  1 file changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index a5671ab896b3..3d99fd6664d7 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -70,6 +70,7 @@
>  #define RTLGEN_SPEED_MASK			0x0630
>  
>  #define RTL_GENERIC_PHYID			0x001cc800
> +#define RTL_8211FVD_PHYID			0x001cc878
>  
>  MODULE_DESCRIPTION("Realtek PHY driver");
>  MODULE_AUTHOR("Johnson Leung");
> @@ -78,6 +79,7 @@ MODULE_LICENSE("GPL");
>  struct rtl821x_priv {
>  	u16 phycr1;
>  	u16 phycr2;
> +	bool has_phycr2;
>  };
>  
>  static int rtl821x_read_page(struct phy_device *phydev)
> @@ -94,6 +96,7 @@ static int rtl821x_probe(struct phy_device *phydev)
>  {
>  	struct device *dev = &phydev->mdio.dev;
>  	struct rtl821x_priv *priv;
> +	u32 phy_id = phydev->drv->phy_id;
>  	int ret;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -108,13 +111,16 @@ static int rtl821x_probe(struct phy_device *phydev)
>  	if (of_property_read_bool(dev->of_node, "realtek,aldps-enable"))
>  		priv->phycr1 |= RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF;
>  
> -	ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2);
> -	if (ret < 0)
> -		return ret;
> +	priv->has_phycr2 = !(phy_id == RTL_8211FVD_PHYID);
> +	if (priv->has_phycr2) {
> +		ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2);
> +		if (ret < 0)
> +			return ret;
>  
> -	priv->phycr2 = ret & RTL8211F_CLKOUT_EN;
> -	if (of_property_read_bool(dev->of_node, "realtek,clkout-disable"))
> -		priv->phycr2 &= ~RTL8211F_CLKOUT_EN;
> +		priv->phycr2 = ret & RTL8211F_CLKOUT_EN;
> +		if (of_property_read_bool(dev->of_node, "realtek,clkout-disable"))
> +			priv->phycr2 &= ~RTL8211F_CLKOUT_EN;
> +	}
>  
>  	phydev->priv = priv;
>  
> @@ -400,12 +406,14 @@ static int rtl8211f_config_init(struct phy_device *phydev)
>  			val_rxdly ? "enabled" : "disabled");
>  	}
>  
> -	ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2,
> -			       RTL8211F_CLKOUT_EN, priv->phycr2);
> -	if (ret < 0) {
> -		dev_err(dev, "clkout configuration failed: %pe\n",
> -			ERR_PTR(ret));
> -		return ret;
> +	if (priv->has_phycr2) {
> +		ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2,
> +				       RTL8211F_CLKOUT_EN, priv->phycr2);
> +		if (ret < 0) {
> +			dev_err(dev, "clkout configuration failed: %pe\n",
> +				ERR_PTR(ret));
> +			return ret;
> +		}
>  	}
>  
>  	return genphy_soft_reset(phydev);
> @@ -923,6 +931,18 @@ static struct phy_driver realtek_drvs[] = {
>  		.resume		= rtl821x_resume,
>  		.read_page	= rtl821x_read_page,
>  		.write_page	= rtl821x_write_page,
> +	}, {
> +		PHY_ID_MATCH_EXACT(RTL_8211FVD_PHYID),
> +		.name		= "RTL8211F-VD Gigabit Ethernet",

This conflicts with RTL8221 in the commit message.

> +		.probe		= rtl821x_probe,
> +		.config_init	= &rtl8211f_config_init,
> +		.read_status	= rtlgen_read_status,
> +		.config_intr	= &rtl8211f_config_intr,
> +		.handle_interrupt = rtl8211f_handle_interrupt,
> +		.suspend	= genphy_suspend,
> +		.resume		= rtl821x_resume,
> +		.read_page	= rtl821x_read_page,
> +		.write_page	= rtl821x_write_page,
>  	}, {
>  		.name		= "Generic FE-GE Realtek PHY",
>  		.match_phy_device = rtlgen_match_phy_device,
Jakub Kicinski Aug. 16, 2022, 6:27 p.m. UTC | #2 
On Wed, 17 Aug 2022 05:48:59 +1000 wei.fang@nxp.com wrote:
> From: Clark Wang <xiaoning.wang@nxp.com>
> 
> RTL8221F(D)(I)-VD-CG is the pin-to-pin upgrade chip from
> RTL8221F(D)(I)-CG.
> 
> Add new PHY ID for this chip.
> It does not support RTL8211F_PHYCR2 anymore, so remove the w/r operation
> of this register.

Also please fix the date on your system, you seem to be talking to us
from the future :/
Wei Fang Aug. 17, 2022, 1:19 a.m. UTC | #3 
> -----Original Message-----
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Sent: 2022年8月16日 21:23
> To: Wei Fang <wei.fang@nxp.com>; andrew@lunn.ch; linux@armlinux.org.uk;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; netdev@vger.kernel.org
> Cc: Clark Wang <xiaoning.wang@nxp.com>
> Subject: Re: [PATCH V2 net-next] net: phy: realtek: add support for
> RTL8221F(D)(I)-VD-CG
> 
> On 16.08.2022 21:48, wei.fang@nxp.com wrote:
> > From: Clark Wang <xiaoning.wang@nxp.com>
> >
> > RTL8221F(D)(I)-VD-CG is the pin-to-pin upgrade chip from
> > RTL8221F(D)(I)-CG.
> 
> Here you talk about RTL8221, in the driver struct definition you say RTL8211.
> You changed the naming already for v2. It would be time for you to clarify
> which chip you actually mean.
> RTL8221 is a 2.5Gbps PHY, however you don't handle this mode in your code.
> 
Sorry again, actually it's RTL8211.

> >
> > Add new PHY ID for this chip.
> > It does not support RTL8211F_PHYCR2 anymore, so remove the w/r
> > operation of this register.
> >
> > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > ---
> > V2 change:
> > 1. Commit message changed, RTL8221 instead of RTL8821.
> > 2. Add has_phycr2 to struct rtl821x_priv.
> > ---
> >  drivers/net/phy/realtek.c | 44
> > ++++++++++++++++++++++++++++-----------
> >  1 file changed, 32 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> > index a5671ab896b3..3d99fd6664d7 100644
> > --- a/drivers/net/phy/realtek.c
> > +++ b/drivers/net/phy/realtek.c
> > @@ -70,6 +70,7 @@
> >  #define RTLGEN_SPEED_MASK			0x0630
> >
> >  #define RTL_GENERIC_PHYID			0x001cc800
> > +#define RTL_8211FVD_PHYID			0x001cc878
> >
> >  MODULE_DESCRIPTION("Realtek PHY driver");
> MODULE_AUTHOR("Johnson
> > Leung"); @@ -78,6 +79,7 @@ MODULE_LICENSE("GPL");  struct
> rtl821x_priv
> > {
> >  	u16 phycr1;
> >  	u16 phycr2;
> > +	bool has_phycr2;
> >  };
> >
> >  static int rtl821x_read_page(struct phy_device *phydev) @@ -94,6
> > +96,7 @@ static int rtl821x_probe(struct phy_device *phydev)  {
> >  	struct device *dev = &phydev->mdio.dev;
> >  	struct rtl821x_priv *priv;
> > +	u32 phy_id = phydev->drv->phy_id;
> >  	int ret;
> >
> >  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); @@ -108,13
> > +111,16 @@ static int rtl821x_probe(struct phy_device *phydev)
> >  	if (of_property_read_bool(dev->of_node, "realtek,aldps-enable"))
> >  		priv->phycr1 |= RTL8211F_ALDPS_PLL_OFF |
> RTL8211F_ALDPS_ENABLE |
> > RTL8211F_ALDPS_XTAL_OFF;
> >
> > -	ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2);
> > -	if (ret < 0)
> > -		return ret;
> > +	priv->has_phycr2 = !(phy_id == RTL_8211FVD_PHYID);
> > +	if (priv->has_phycr2) {
> > +		ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2);
> > +		if (ret < 0)
> > +			return ret;
> >
> > -	priv->phycr2 = ret & RTL8211F_CLKOUT_EN;
> > -	if (of_property_read_bool(dev->of_node, "realtek,clkout-disable"))
> > -		priv->phycr2 &= ~RTL8211F_CLKOUT_EN;
> > +		priv->phycr2 = ret & RTL8211F_CLKOUT_EN;
> > +		if (of_property_read_bool(dev->of_node, "realtek,clkout-disable"))
> > +			priv->phycr2 &= ~RTL8211F_CLKOUT_EN;
> > +	}
> >
> >  	phydev->priv = priv;
> >
> > @@ -400,12 +406,14 @@ static int rtl8211f_config_init(struct phy_device
> *phydev)
> >  			val_rxdly ? "enabled" : "disabled");
> >  	}
> >
> > -	ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2,
> > -			       RTL8211F_CLKOUT_EN, priv->phycr2);
> > -	if (ret < 0) {
> > -		dev_err(dev, "clkout configuration failed: %pe\n",
> > -			ERR_PTR(ret));
> > -		return ret;
> > +	if (priv->has_phycr2) {
> > +		ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2,
> > +				       RTL8211F_CLKOUT_EN, priv->phycr2);
> > +		if (ret < 0) {
> > +			dev_err(dev, "clkout configuration failed: %pe\n",
> > +				ERR_PTR(ret));
> > +			return ret;
> > +		}
> >  	}
> >
> >  	return genphy_soft_reset(phydev);
> > @@ -923,6 +931,18 @@ static struct phy_driver realtek_drvs[] = {
> >  		.resume		= rtl821x_resume,
> >  		.read_page	= rtl821x_read_page,
> >  		.write_page	= rtl821x_write_page,
> > +	}, {
> > +		PHY_ID_MATCH_EXACT(RTL_8211FVD_PHYID),
> > +		.name		= "RTL8211F-VD Gigabit Ethernet",
> 
> This conflicts with RTL8221 in the commit message.
> 
> > +		.probe		= rtl821x_probe,
> > +		.config_init	= &rtl8211f_config_init,
> > +		.read_status	= rtlgen_read_status,
> > +		.config_intr	= &rtl8211f_config_intr,
> > +		.handle_interrupt = rtl8211f_handle_interrupt,
> > +		.suspend	= genphy_suspend,
> > +		.resume		= rtl821x_resume,
> > +		.read_page	= rtl821x_read_page,
> > +		.write_page	= rtl821x_write_page,
> >  	}, {
> >  		.name		= "Generic FE-GE Realtek PHY",
> >  		.match_phy_device = rtlgen_match_phy_device,
diff mbox series

Patch

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index a5671ab896b3..3d99fd6664d7 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -70,6 +70,7 @@ 
 #define RTLGEN_SPEED_MASK			0x0630
 
 #define RTL_GENERIC_PHYID			0x001cc800
+#define RTL_8211FVD_PHYID			0x001cc878
 
 MODULE_DESCRIPTION("Realtek PHY driver");
 MODULE_AUTHOR("Johnson Leung");
@@ -78,6 +79,7 @@  MODULE_LICENSE("GPL");
 struct rtl821x_priv {
 	u16 phycr1;
 	u16 phycr2;
+	bool has_phycr2;
 };
 
 static int rtl821x_read_page(struct phy_device *phydev)
@@ -94,6 +96,7 @@  static int rtl821x_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
 	struct rtl821x_priv *priv;
+	u32 phy_id = phydev->drv->phy_id;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -108,13 +111,16 @@  static int rtl821x_probe(struct phy_device *phydev)
 	if (of_property_read_bool(dev->of_node, "realtek,aldps-enable"))
 		priv->phycr1 |= RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF;
 
-	ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2);
-	if (ret < 0)
-		return ret;
+	priv->has_phycr2 = !(phy_id == RTL_8211FVD_PHYID);
+	if (priv->has_phycr2) {
+		ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2);
+		if (ret < 0)
+			return ret;
 
-	priv->phycr2 = ret & RTL8211F_CLKOUT_EN;
-	if (of_property_read_bool(dev->of_node, "realtek,clkout-disable"))
-		priv->phycr2 &= ~RTL8211F_CLKOUT_EN;
+		priv->phycr2 = ret & RTL8211F_CLKOUT_EN;
+		if (of_property_read_bool(dev->of_node, "realtek,clkout-disable"))
+			priv->phycr2 &= ~RTL8211F_CLKOUT_EN;
+	}
 
 	phydev->priv = priv;
 
@@ -400,12 +406,14 @@  static int rtl8211f_config_init(struct phy_device *phydev)
 			val_rxdly ? "enabled" : "disabled");
 	}
 
-	ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2,
-			       RTL8211F_CLKOUT_EN, priv->phycr2);
-	if (ret < 0) {
-		dev_err(dev, "clkout configuration failed: %pe\n",
-			ERR_PTR(ret));
-		return ret;
+	if (priv->has_phycr2) {
+		ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2,
+				       RTL8211F_CLKOUT_EN, priv->phycr2);
+		if (ret < 0) {
+			dev_err(dev, "clkout configuration failed: %pe\n",
+				ERR_PTR(ret));
+			return ret;
+		}
 	}
 
 	return genphy_soft_reset(phydev);
@@ -923,6 +931,18 @@  static struct phy_driver realtek_drvs[] = {
 		.resume		= rtl821x_resume,
 		.read_page	= rtl821x_read_page,
 		.write_page	= rtl821x_write_page,
+	}, {
+		PHY_ID_MATCH_EXACT(RTL_8211FVD_PHYID),
+		.name		= "RTL8211F-VD Gigabit Ethernet",
+		.probe		= rtl821x_probe,
+		.config_init	= &rtl8211f_config_init,
+		.read_status	= rtlgen_read_status,
+		.config_intr	= &rtl8211f_config_intr,
+		.handle_interrupt = rtl8211f_handle_interrupt,
+		.suspend	= genphy_suspend,
+		.resume		= rtl821x_resume,
+		.read_page	= rtl821x_read_page,
+		.write_page	= rtl821x_write_page,
 	}, {
 		.name		= "Generic FE-GE Realtek PHY",
 		.match_phy_device = rtlgen_match_phy_device,