All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH V2 net-next] net: phy: realtek: add support for RTL8221F(D)(I)-VD-CG
  2022-08-16 19:48 [PATCH V2 net-next] net: phy: realtek: add support for RTL8221F(D)(I)-VD-CG wei.fang
@ 2022-08-16 13:22 ` Heiner Kallweit
  2022-08-17  1:19   ` Wei Fang
  2022-08-16 18:27 ` Jakub Kicinski
  1 sibling, 1 reply; 4+ messages in thread
From: Heiner Kallweit @ 2022-08-16 13:22 UTC (permalink / raw)
  To: wei.fang, andrew, linux, davem, edumazet, kuba, pabeni, netdev
  Cc: xiaoning.wang

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,


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH V2 net-next] net: phy: realtek: add support for RTL8221F(D)(I)-VD-CG
  2022-08-16 19:48 [PATCH V2 net-next] net: phy: realtek: add support for RTL8221F(D)(I)-VD-CG wei.fang
  2022-08-16 13:22 ` Heiner Kallweit
@ 2022-08-16 18:27 ` Jakub Kicinski
  1 sibling, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2022-08-16 18:27 UTC (permalink / raw)
  To: wei.fang
  Cc: andrew, hkallweit1, linux, davem, edumazet, pabeni, netdev,
	xiaoning.wang

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 :/

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH V2 net-next] net: phy: realtek: add support for RTL8221F(D)(I)-VD-CG
@ 2022-08-16 19:48 wei.fang
  2022-08-16 13:22 ` Heiner Kallweit
  2022-08-16 18:27 ` Jakub Kicinski
  0 siblings, 2 replies; 4+ messages in thread
From: wei.fang @ 2022-08-16 19:48 UTC (permalink / raw)
  To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev
  Cc: xiaoning.wang

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(-)

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,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* RE: [PATCH V2 net-next] net: phy: realtek: add support for RTL8221F(D)(I)-VD-CG
  2022-08-16 13:22 ` Heiner Kallweit
@ 2022-08-17  1:19   ` Wei Fang
  0 siblings, 0 replies; 4+ messages in thread
From: Wei Fang @ 2022-08-17  1:19 UTC (permalink / raw)
  To: Heiner Kallweit, andrew, linux, davem, edumazet, kuba, pabeni, netdev
  Cc: Clark Wang



> -----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,


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-17  1:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-16 19:48 [PATCH V2 net-next] net: phy: realtek: add support for RTL8221F(D)(I)-VD-CG wei.fang
2022-08-16 13:22 ` Heiner Kallweit
2022-08-17  1:19   ` Wei Fang
2022-08-16 18:27 ` Jakub Kicinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.