[PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support

Samin Guo posted 2 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support
Posted by Samin Guo 2 years, 7 months ago
The motorcomm phy (YT8531) supports the ability to adjust the drive
strength of the rx_clk/rx_data, and the default strength may not be
suitable for all boards. So add configurable options to better match
the boards.(e.g. StarFive VisionFive 2)

Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
---
 drivers/net/phy/motorcomm.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 2fa5a90e073b..08f28ed83e60 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -236,6 +236,11 @@
  */
 #define YTPHY_WCR_TYPE_PULSE			BIT(0)
 
+#define YTPHY_PAD_DRIVE_STRENGTH_REG		0xA010
+#define YTPHY_RGMII_RXC_DS			GENMASK(15, 13)
+#define YTPHY_RGMII_RXD_DS			GENMASK(5, 4)	/* Bit 1 and 0 of rgmii_rxd_ds */
+#define YTPHY_RGMII_RXD_DS2			BIT(12) 	/* Bit 2 of rgmii_rxd_ds */
+
 #define YTPHY_SYNCE_CFG_REG			0xA012
 #define YT8521_SCR_SYNCE_ENABLE			BIT(5)
 /* 1b0 output 25m clock
@@ -1495,6 +1500,7 @@ static int yt8531_config_init(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
 	int ret;
+	u32 val;
 
 	ret = ytphy_rgmii_clk_delay_config_with_lock(phydev);
 	if (ret < 0)
@@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev)
 			return ret;
 	}
 
+	if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) {
+		ret = ytphy_modify_ext_with_lock(phydev,
+						 YTPHY_PAD_DRIVE_STRENGTH_REG,
+						 YTPHY_RGMII_RXC_DS,
+						 FIELD_PREP(YTPHY_RGMII_RXC_DS, val));
+		if (ret < 0)
+			return ret;
+	}
+
+	if (!of_property_read_u32(node, "rx-data-driver-strength", &val)) {
+		if (val > FIELD_MAX(YTPHY_RGMII_RXD_DS)) {
+			val &= FIELD_MAX(YTPHY_RGMII_RXD_DS);
+			val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val);
+			val |= YTPHY_RGMII_RXD_DS2;
+		} else {
+			val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val);
+		}
+
+		ret = ytphy_modify_ext_with_lock(phydev,
+						 YTPHY_PAD_DRIVE_STRENGTH_REG,
+						 YTPHY_RGMII_RXD_DS | YTPHY_RGMII_RXD_DS2,
+						 val);
+		if (ret < 0)
+			return ret;
+	}
+
 	return 0;
 }
 
-- 
2.17.1
Re: [PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support
Posted by Frank Sae 2 years, 7 months ago

On 2023/4/26 14:35, Samin Guo wrote:
> The motorcomm phy (YT8531) supports the ability to adjust the drive
> strength of the rx_clk/rx_data, and the default strength may not be
> suitable for all boards. So add configurable options to better match
> the boards.(e.g. StarFive VisionFive 2)
> 
> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
> ---
>  drivers/net/phy/motorcomm.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
> index 2fa5a90e073b..08f28ed83e60 100644
> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c
> @@ -236,6 +236,11 @@
>   */
>  #define YTPHY_WCR_TYPE_PULSE			BIT(0)
>  
> +#define YTPHY_PAD_DRIVE_STRENGTH_REG		0xA010
> +#define YTPHY_RGMII_RXC_DS			GENMASK(15, 13)
> +#define YTPHY_RGMII_RXD_DS			GENMASK(5, 4)	/* Bit 1 and 0 of rgmii_rxd_ds */
> +#define YTPHY_RGMII_RXD_DS2			BIT(12) 	/* Bit 2 of rgmii_rxd_ds */
> +

Please  change YTPHY_RGMII_XXX  to YT8531_RGMII_XXX. YT8521's reg (0xA010) is not same as this.
Keep bit order.

>  #define YTPHY_SYNCE_CFG_REG			0xA012
>  #define YT8521_SCR_SYNCE_ENABLE			BIT(5)
>  /* 1b0 output 25m clock
> @@ -1495,6 +1500,7 @@ static int yt8531_config_init(struct phy_device *phydev)
>  {
>  	struct device_node *node = phydev->mdio.dev.of_node;
>  	int ret;
> +	u32 val;
>  
>  	ret = ytphy_rgmii_clk_delay_config_with_lock(phydev);
>  	if (ret < 0)
> @@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev)
>  			return ret;
>  	}
>  
> +	if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) {

Please check the val of "val", add the handle of default value.

> +		ret = ytphy_modify_ext_with_lock(phydev,
> +						 YTPHY_PAD_DRIVE_STRENGTH_REG,
> +						 YTPHY_RGMII_RXC_DS,
> +						 FIELD_PREP(YTPHY_RGMII_RXC_DS, val));
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	if (!of_property_read_u32(node, "rx-data-driver-strength", &val)) {
> +		if (val > FIELD_MAX(YTPHY_RGMII_RXD_DS)) {
> +			val &= FIELD_MAX(YTPHY_RGMII_RXD_DS);
> +			val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val);
> +			val |= YTPHY_RGMII_RXD_DS2;
> +		} else {
> +			val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val);
> +		}
> +
> +		ret = ytphy_modify_ext_with_lock(phydev,
> +						 YTPHY_PAD_DRIVE_STRENGTH_REG,
> +						 YTPHY_RGMII_RXD_DS | YTPHY_RGMII_RXD_DS2,
> +						 val);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>
Re: [PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support
Posted by Guo Samin 2 years, 7 months ago
Re: [PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support
From: Frank Sae <Frank.Sae@motor-comm.com>
to: Samin Guo <samin.guo@starfivetech.com>, <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>, <netdev@vger.kernel.org>, Peter Geis <pgwipeout@gmail.com>
data: 2023/4/26

> 
> 
> On 2023/4/26 14:35, Samin Guo wrote:
>> The motorcomm phy (YT8531) supports the ability to adjust the drive
>> strength of the rx_clk/rx_data, and the default strength may not be
>> suitable for all boards. So add configurable options to better match
>> the boards.(e.g. StarFive VisionFive 2)
>>
>> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
>> ---
>>  drivers/net/phy/motorcomm.c | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
>> index 2fa5a90e073b..08f28ed83e60 100644
>> --- a/drivers/net/phy/motorcomm.c
>> +++ b/drivers/net/phy/motorcomm.c
>> @@ -236,6 +236,11 @@
>>   */
>>  #define YTPHY_WCR_TYPE_PULSE			BIT(0)
>>  
>> +#define YTPHY_PAD_DRIVE_STRENGTH_REG		0xA010
>> +#define YTPHY_RGMII_RXC_DS			GENMASK(15, 13)
>> +#define YTPHY_RGMII_RXD_DS			GENMASK(5, 4)	/* Bit 1 and 0 of rgmii_rxd_ds */
>> +#define YTPHY_RGMII_RXD_DS2			BIT(12) 	/* Bit 2 of rgmii_rxd_ds */
>> +
> 
> Please  change YTPHY_RGMII_XXX  to YT8531_RGMII_XXX. YT8521's reg (0xA010) is not same as this.
> Keep bit order.

will fix.

> 
>>  #define YTPHY_SYNCE_CFG_REG			0xA012
>>  #define YT8521_SCR_SYNCE_ENABLE			BIT(5)
>>  /* 1b0 output 25m clock
>> @@ -1495,6 +1500,7 @@ static int yt8531_config_init(struct phy_device *phydev)
>>  {
>>  	struct device_node *node = phydev->mdio.dev.of_node;
>>  	int ret;
>> +	u32 val;
>>  
>>  	ret = ytphy_rgmii_clk_delay_config_with_lock(phydev);
>>  	if (ret < 0)
>> @@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev)
>>  			return ret;
>>  	}
>>  
>> +	if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) {
> 
> Please check the val of "val", add the handle of default value.
> 
Will fix it in the next version, thanks.
>> +		ret = ytphy_modify_ext_with_lock(phydev,
>> +						 YTPHY_PAD_DRIVE_STRENGTH_REG,
>> +						 YTPHY_RGMII_RXC_DS,
>> +						 FIELD_PREP(YTPHY_RGMII_RXC_DS, val));
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	if (!of_property_read_u32(node, "rx-data-driver-strength", &val)) {
>> +		if (val > FIELD_MAX(YTPHY_RGMII_RXD_DS)) {
>> +			val &= FIELD_MAX(YTPHY_RGMII_RXD_DS);
>> +			val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val);
>> +			val |= YTPHY_RGMII_RXD_DS2;
>> +		} else {
>> +			val = FIELD_PREP(YTPHY_RGMII_RXD_DS, val);
>> +		}
>> +
>> +		ret = ytphy_modify_ext_with_lock(phydev,
>> +						 YTPHY_PAD_DRIVE_STRENGTH_REG,
>> +						 YTPHY_RGMII_RXD_DS | YTPHY_RGMII_RXD_DS2,
>> +						 val);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>>  	return 0;
>>  }
>>
Re: [PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support
Posted by Andrew Lunn 2 years, 7 months ago
> > +	u32 val;
> >  
> >  	ret = ytphy_rgmii_clk_delay_config_with_lock(phydev);
> >  	if (ret < 0)
> > @@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev)
> >  			return ret;
> >  	}
> >  
> > +	if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) {
> 
> Please check the val of "val", add the handle of default value.

You can assign val to 3, or better still some #define, before calling
of_property_read_u32(). If the property is not found, val will retain
that value, and you can then write it to the register.

But please do add range checks for when val is in DT. We don't want
anybody using 42. -EINVAL should be returned.

	Andrew
Re: [PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support
Posted by Guo Samin 2 years, 7 months ago
Re: [PATCH v1 2/2] net: phy: motorcomm: Add pad drive strength cfg support
From: Andrew Lunn <andrew@lunn.ch>
to: Frank Sae <Frank.Sae@motor-comm.com>
data: 2023/4/26

>>> +	u32 val;
>>>  
>>>  	ret = ytphy_rgmii_clk_delay_config_with_lock(phydev);
>>>  	if (ret < 0)
>>> @@ -1518,6 +1524,32 @@ static int yt8531_config_init(struct phy_device *phydev)
>>>  			return ret;
>>>  	}
>>>  
>>> +	if (!of_property_read_u32(node, "rx-clk-driver-strength", &val)) {
>>
>> Please check the val of "val", add the handle of default value.
> 
> You can assign val to 3, or better still some #define, before calling
> of_property_read_u32(). If the property is not found, val will retain
> that value, and you can then write it to the register.
> 
> But please do add range checks for when val is in DT. We don't want
> anybody using 42. -EINVAL should be returned.
> 
> 	Andrew

Thanks, good advice. 

Best regards,
Samin