[PATCH v1 0/2] Add motorcomm phy pad-driver-strength-cfg support

[PATCH v1 0/2] Add motorcomm phy pad-driver-strength-cfg support

[Samin Guo]

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)

The first patch adds a description of dt-bingding, and the second patch adds
YT8531's parsing and settings for pad-driver-strength-cfg.

Samin Guo (2):
  dt-bindings: net: motorcomm: Add pad driver strength cfg
  net: phy: motorcomm: Add pad drive strength cfg support

 .../bindings/net/motorcomm,yt8xxx.yaml        | 10 ++++++
 drivers/net/phy/motorcomm.c                   | 32 +++++++++++++++++++
 2 files changed, 42 insertions(+)


base-commit: d3e1ee0e67e7603d36f4fa2fec6b881c01aabe89
--
2.17.1

[PATCH v1 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg

[Samin Guo]

The motorcomm phy (YT8531) supports the ability to adjust the drive
strength of the rx_clk/rx_data, the value range of pad driver
strength is 0 to 7.

Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
---
 .../devicetree/bindings/net/motorcomm,yt8xxx.yaml      | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
index 157e3bbcaf6f..e648e486b6d8 100644
--- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
+++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
@@ -18,6 +18,16 @@ properties:
       - ethernet-phy-id4f51.e91a
       - ethernet-phy-id4f51.e91b
 
+  rx-clk-driver-strength:
+    description: drive strength of rx_clk pad.
+    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
+    default: 3
+
+  rx-data-driver-strength:
+    description: drive strength of rxd/rx_ctl rgmii pad.
+    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
+    default: 3
+
   rx-internal-delay-ps:
     description: |
       RGMII RX Clock Delay used only when PHY operates in RGMII mode with
-- 
2.17.1

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

[Samin Guo]

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 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg

[Frank Sae]

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, the value range of pad driver
> strength is 0 to 7.
> 
> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
> ---
>  .../devicetree/bindings/net/motorcomm,yt8xxx.yaml      | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> index 157e3bbcaf6f..e648e486b6d8 100644
> --- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> @@ -18,6 +18,16 @@ properties:
>        - ethernet-phy-id4f51.e91a
>        - ethernet-phy-id4f51.e91b
>  
> +  rx-clk-driver-strength:
> +    description: drive strength of rx_clk pad.
> +    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
> +    default: 3
> +
> +  rx-data-driver-strength:
> +    description: drive strength of rxd/rx_ctl rgmii pad.
> +    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
> +    default: 3
> +

rx-clk-driver-strength and rx-data-driver-strength are not standard, so please add "motorcomm,".
rx-clk-driver-strength => motorcomm,rx-clk-driver-strength


>    rx-internal-delay-ps:
>      description: |
>        RGMII RX Clock Delay used only when PHY operates in RGMII mode with

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

[Frank Sae]

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 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg

[Andrew Lunn]

On Wed, Apr 26, 2023 at 02:35:40PM +0800, Samin Guo wrote:
> The motorcomm phy (YT8531) supports the ability to adjust the drive
> strength of the rx_clk/rx_data, the value range of pad driver
> strength is 0 to 7.
> 
> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
> ---
>  .../devicetree/bindings/net/motorcomm,yt8xxx.yaml      | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> index 157e3bbcaf6f..e648e486b6d8 100644
> --- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> @@ -18,6 +18,16 @@ properties:
>        - ethernet-phy-id4f51.e91a
>        - ethernet-phy-id4f51.e91b
>  
> +  rx-clk-driver-strength:
> +    description: drive strength of rx_clk pad.
> +    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
> +    default: 3

What do the numbers mean? What are the units? mA?

     Andrew

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

[Andrew Lunn]

> > +	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 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg

[Guo Samin]

Re: [PATCH v1 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg
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, the value range of pad driver
>> strength is 0 to 7.
>>
>> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
>> ---
>>  .../devicetree/bindings/net/motorcomm,yt8xxx.yaml      | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> index 157e3bbcaf6f..e648e486b6d8 100644
>> --- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> @@ -18,6 +18,16 @@ properties:
>>        - ethernet-phy-id4f51.e91a
>>        - ethernet-phy-id4f51.e91b
>>  
>> +  rx-clk-driver-strength:
>> +    description: drive strength of rx_clk pad.
>> +    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
>> +    default: 3
>> +
>> +  rx-data-driver-strength:
>> +    description: drive strength of rxd/rx_ctl rgmii pad.
>> +    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
>> +    default: 3
>> +
> 
> rx-clk-driver-strength and rx-data-driver-strength are not standard, so please add "motorcomm,".
> rx-clk-driver-strength => motorcomm,rx-clk-driver-strength
>

Thanks, I will fix it next version.


Best regards,
Samin
 
> 
>>    rx-internal-delay-ps:
>>      description: |
>>        RGMII RX Clock Delay used only when PHY operates in RGMII mode with

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

[Guo Samin]

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 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg

[Guo Samin]

Re: [PATCH v1 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg
From: Andrew Lunn <andrew@lunn.ch>
to: Samin Guo <samin.guo@starfivetech.com>
data: 2023/4/26

> On Wed, Apr 26, 2023 at 02:35:40PM +0800, Samin Guo wrote:
>> The motorcomm phy (YT8531) supports the ability to adjust the drive
>> strength of the rx_clk/rx_data, the value range of pad driver
>> strength is 0 to 7.
>>
>> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
>> ---
>>  .../devicetree/bindings/net/motorcomm,yt8xxx.yaml      | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> index 157e3bbcaf6f..e648e486b6d8 100644
>> --- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> @@ -18,6 +18,16 @@ properties:
>>        - ethernet-phy-id4f51.e91a
>>        - ethernet-phy-id4f51.e91b
>>  
>> +  rx-clk-driver-strength:
>> +    description: drive strength of rx_clk pad.
>> +    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
>> +    default: 3
> 
> What do the numbers mean? What are the units? mA?
> 
>      Andrew

Hi Andrew,

They represent the gradient of the current, which should be the higher the value, the greater the current.
However, there is no exact current value to determine from YT8531 datasheet.
Probably Motorcomm @Frank.sea knows what it means.

Description: Drive strenght of rx_clk pad.
3'b111: strongest;
3'b000: weakest.
Default: 0x3


Best regards,
Samin

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

[Guo Samin]

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