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

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.

Changes since v1:
Patch 1:
- Renamed "rx-xxx-driver-strength" to "motorcomm,rx-xxx-driver-strength" (by Frank Sae)
Patch 2:
- Added default values for rxc/rxd driver strength (by Frank Sea/Andrew Lunn)
- Added range checking when val is in DT (by Frank Sea/Andrew Lunn)

Previous versions:
v1 - https://patchwork.kernel.org/project/netdevbpf/cover/20230426063541.15378-1-samin.guo@starfivetech.com

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        | 12 +++++
 drivers/net/phy/motorcomm.c                   | 46 +++++++++++++++++++
 2 files changed, 58 insertions(+)


base-commit: d3e1ee0e67e7603d36f4fa2fec6b881c01aabe89

-- 
2.17.1

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 <redacted>
---
 .../devicetree/bindings/net/motorcomm,yt8xxx.yaml    | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
index 157e3bbcaf6f..29a1997a1577 100644
--- a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
+++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml

@@ -52,6 +52,18 @@ properties:
       for a timer.
     type: boolean
 
+  motorcomm,rx-clk-driver-strength:
+    description: drive strength of rx_clk pad.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
+    default: 3
+
+  motorcomm,rx-data-driver-strength:
+    description: drive strength of rx_data/rx_ctl rgmii pad.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
+    default: 3
+
   motorcomm,tx-clk-adj-enabled:
     description: |
       This configuration is mainly to adapt to VF2 with JH7110 SoC.

-- 
2.17.1

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 <redacted>
---
 drivers/net/phy/motorcomm.c | 46 +++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 2fa5a90e073b..191650bb1454 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c

@@ -236,6 +236,7 @@
  */
 #define YTPHY_WCR_TYPE_PULSE			BIT(0)
 
+#define YTPHY_PAD_DRIVE_STRENGTH_REG		0xA010
 #define YTPHY_SYNCE_CFG_REG			0xA012
 #define YT8521_SCR_SYNCE_ENABLE			BIT(5)
 /* 1b0 output 25m clock

@@ -260,6 +261,14 @@
 #define YT8531_SCR_CLK_SRC_REF_25M		4
 #define YT8531_SCR_CLK_SRC_SSC_25M		5
 
+#define YT8531_RGMII_RXC_DS_DEFAULT		0x3
+#define YT8531_RGMII_RXC_DS_MAX			0x7
+#define YT8531_RGMII_RXC_DS			GENMASK(15, 13)
+#define YT8531_RGMII_RXD_DS_DEFAULT		0x3
+#define YT8531_RGMII_RXD_DS_MAX			0x7
+#define YT8531_RGMII_RXD_DS_LOW			GENMASK(5, 4) /* Bit 1/0 of rxd_ds */
+#define YT8531_RGMII_RXD_DS_HI			BIT(12) /* Bit 2 of rxd_ds */
+
 /* Extended Register  end */
 
 #define YTPHY_DTS_OUTPUT_CLK_DIS		0

@@ -1495,6 +1504,7 @@ static int yt8531_config_init(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
 	int ret;
+	u32 ds, val;
 
 	ret = ytphy_rgmii_clk_delay_config_with_lock(phydev);
 	if (ret < 0)

@@ -1518,6 +1528,42 @@ static int yt8531_config_init(struct phy_device *phydev)
 			return ret;
 	}
 
+	ds = YT8531_RGMII_RXC_DS_DEFAULT;
+	if (!of_property_read_u32(node, "motorcomm,rx-clk-driver-strength", &val)) {
+		if (val > YT8531_RGMII_RXC_DS_MAX)
+			return -EINVAL;
+
+		ds = val;
+	}
+
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YTPHY_PAD_DRIVE_STRENGTH_REG,
+					 YT8531_RGMII_RXC_DS,
+					 FIELD_PREP(YT8531_RGMII_RXC_DS, ds));
+	if (ret < 0)
+		return ret;
+
+	ds = FIELD_PREP(YT8531_RGMII_RXD_DS_LOW, YT8531_RGMII_RXD_DS_DEFAULT);
+	if (!of_property_read_u32(node, "motorcomm,rx-data-driver-strength", &val)) {
+		if (val > YT8531_RGMII_RXD_DS_MAX)
+			return -EINVAL;
+
+		if (val > FIELD_MAX(YT8531_RGMII_RXD_DS_LOW)) {
+			ds = val & FIELD_MAX(YT8531_RGMII_RXD_DS_LOW);
+			ds = FIELD_PREP(YT8531_RGMII_RXD_DS_LOW, ds);
+			ds |= YT8531_RGMII_RXD_DS_HI;
+		} else {
+			ds = FIELD_PREP(YT8531_RGMII_RXD_DS_LOW, val);
+		}
+	}
+
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YTPHY_PAD_DRIVE_STRENGTH_REG,
+					 YT8531_RGMII_RXD_DS_LOW | YT8531_RGMII_RXD_DS_HI,
+					 ds);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }
 

-- 
2.17.1

Reverse Christmas tree.  Sort these longest first, shortest last.

Otherwise this looks O.K.

The only open question is if real unit should be used, uA, not some
magic numbers. Lets see what the DT Maintainers say.

      Andrew

On 05/05/2023 11:05, Samin Guo wrote:

No improvements after Andrew's comment.

Best regards,
Krzysztof

On 2023/5/5 17:05, Samin Guo wrote:

YT8531_RGMII_xxx is bit define for YTPHY_PAD_DRIVE_STRENGTH_REG, so it is better to put it under the define of YTPHY_PAD_DRIVE_STRENGTH_REG.

YT8531_RGMII_xxx bit define as reverse order:
#define YTPHY_PAD_DRIVE_STRENGTH_REG		0xA010
#define YT8531_RGMII_RXC_DS			GENMASK(15, 13)
#define YT8531_RGMII_RXD_DS_HI			BIT(12) /* Bit 2 of rxd_ds */     <-------
#define YT8531_RGMII_RXD_DS_LOW			GENMASK(5, 4) /* Bit 1/0 of rxd_ds */
...

data: Re: [PATCH v2 2/2] net: phy: motorcomm: Add pad drive strength cfg support
From: Frank Sae <Frank.Sae@motor-comm.com>
to: Samin Guo <redacted>, <redacted>, <redacted>, <redacted>, Peter Geis <redacted>
data: 2023/5/6

On 2023/5/5 17:05, Samin Guo wrote:

quoted

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 <redacted>
---
 drivers/net/phy/motorcomm.c | 46 +++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 2fa5a90e073b..191650bb1454 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c

@@ -236,6 +236,7 @@
  */
 #define YTPHY_WCR_TYPE_PULSE			BIT(0)
 
+#define YTPHY_PAD_DRIVE_STRENGTH_REG		0xA010
 #define YTPHY_SYNCE_CFG_REG			0xA012
 #define YT8521_SCR_SYNCE_ENABLE			BIT(5)
 /* 1b0 output 25m clock

@@ -260,6 +261,14 @@
 #define YT8531_SCR_CLK_SRC_REF_25M		4
 #define YT8531_SCR_CLK_SRC_SSC_25M		5
 
+#define YT8531_RGMII_RXC_DS_DEFAULT		0x3
+#define YT8531_RGMII_RXC_DS_MAX			0x7
+#define YT8531_RGMII_RXC_DS			GENMASK(15, 13)
+#define YT8531_RGMII_RXD_DS_DEFAULT		0x3
+#define YT8531_RGMII_RXD_DS_MAX			0x7
+#define YT8531_RGMII_RXD_DS_LOW			GENMASK(5, 4) /* Bit 1/0 of rxd_ds */
+#define YT8531_RGMII_RXD_DS_HI			BIT(12) /* Bit 2 of rxd_ds */

YT8531_RGMII_xxx is bit define for YTPHY_PAD_DRIVE_STRENGTH_REG, so it is better to put it under the define of YTPHY_PAD_DRIVE_STRENGTH_REG.

YT8531_RGMII_xxx bit define as reverse order:
#define YTPHY_PAD_DRIVE_STRENGTH_REG		0xA010
#define YT8531_RGMII_RXC_DS			GENMASK(15, 13)
#define YT8531_RGMII_RXD_DS_HI			BIT(12) /* Bit 2 of rxd_ds */     <-------
#define YT8531_RGMII_RXD_DS_LOW			GENMASK(5, 4) /* Bit 1/0 of rxd_ds */
...

Hi Frank,

Ok, will fix it next version.
btw, do you have any information you can provide about Andrew's mention of using real unit uA/mA  instead of magic numbers?
(I couldn't find any information about current in the YT8531's datasheet other than the magic numbers.)


Below is all the relevant information I found：

Pad Drive Strength Cfg (EXT_0xA010)

Bit   |  Symbol           |  Access |  Default |  Description
15:13 |  Rgmii_sw_dr_rx   |  RW     |  0x3     |  Drive strenght of rx_clk pad.
                                               |  3'b111: strongest; 3'b000: weakest.

12    |  Rgmii_sw_dr[2]   |  RW     |  0x0     |  Bit 2 of Rgmii_sw_dr[2:0], refer to ext A010[5:4]

5:4   |  Rgmii_sw_dr[1:0] |  RW     |  0x3     |  Bit 1 and 0 of Rgmii_sw_dr, Drive strenght of rxd/rx_ctl rgmii pad.
                                               |  3'b111: strongest; 3'b000: weakest


Best regards,
Samin

quoted

+
 /* Extended Register  end */
 
 #define YTPHY_DTS_OUTPUT_CLK_DIS		0

@@ -1495,6 +1504,7 @@ static int yt8531_config_init(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
 	int ret;
+	u32 ds, val;
 
 	ret = ytphy_rgmii_clk_delay_config_with_lock(phydev);
 	if (ret < 0)

@@ -1518,6 +1528,42 @@ static int yt8531_config_init(struct phy_device *phydev)
 			return ret;
 	}
 
+	ds = YT8531_RGMII_RXC_DS_DEFAULT;
+	if (!of_property_read_u32(node, "motorcomm,rx-clk-driver-strength", &val)) {
+		if (val > YT8531_RGMII_RXC_DS_MAX)
+			return -EINVAL;
+
+		ds = val;
+	}
+
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YTPHY_PAD_DRIVE_STRENGTH_REG,
+					 YT8531_RGMII_RXC_DS,
+					 FIELD_PREP(YT8531_RGMII_RXC_DS, ds));
+	if (ret < 0)
+		return ret;
+
+	ds = FIELD_PREP(YT8531_RGMII_RXD_DS_LOW, YT8531_RGMII_RXD_DS_DEFAULT);
+	if (!of_property_read_u32(node, "motorcomm,rx-data-driver-strength", &val)) {
+		if (val > YT8531_RGMII_RXD_DS_MAX)
+			return -EINVAL;
+
+		if (val > FIELD_MAX(YT8531_RGMII_RXD_DS_LOW)) {
+			ds = val & FIELD_MAX(YT8531_RGMII_RXD_DS_LOW);
+			ds = FIELD_PREP(YT8531_RGMII_RXD_DS_LOW, ds);
+			ds |= YT8531_RGMII_RXD_DS_HI;
+		} else {
+			ds = FIELD_PREP(YT8531_RGMII_RXD_DS_LOW, val);
+		}
+	}
+
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YTPHY_PAD_DRIVE_STRENGTH_REG,
+					 YT8531_RGMII_RXD_DS_LOW | YT8531_RGMII_RXD_DS_HI,
+					 ds);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }
 

Re: [PATCH v2 2/2] net: phy: motorcomm: Add pad drive strength cfg support
From: Andrew Lunn <andrew@lunn.ch>
to: Samin Guo <redacted>
data: 2023/5/5

quoted

 #define YTPHY_DTS_OUTPUT_CLK_DIS		0

@@ -1495,6 +1504,7 @@ static int yt8531_config_init(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
 	int ret;
+	u32 ds, val;

Reverse Christmas tree.  Sort these longest first, shortest last.

Thanks, will fix.

Otherwise this looks O.K.

The only open question is if real unit should be used, uA, not some
magic numbers. Lets see what the DT Maintainers say.

      Andrew

Hi Andrew,

As I communicated with Frank, Motorcomm doesn't give specific units on their datasheet, except for magic numbers.
Tried to ask Motorcomm last week, but it seems that they themselves do not know what the unit is and have no response so far.


Below is all the relevant information I found：

Pad Drive Strength Cfg (EXT_0xA010)

Bit   |  Symbol           |  Access |  Default |  Description
15:13 |  Rgmii_sw_dr_rx   |  RW     |  0x3     |  Drive strenght of rx_clk pad.
                                               |  3'b111: strongest; 3'b000: weakest.

12    |  Rgmii_sw_dr[2]   |  RW     |  0x0     |  Bit 2 of Rgmii_sw_dr[2:0], refer to ext A010[5:4]

5:4   |  Rgmii_sw_dr[1:0] |  RW     |  0x3     |  Bit 1 and 0 of Rgmii_sw_dr, Drive strenght of rxd/rx_ctl rgmii pad.
                                               |  3'b111: strongest; 3'b000: weakest



Best regards,
Samin

Re: [PATCH v2 2/2] net: phy: motorcomm: Add pad drive strength cfg support
From: Guo Samin <redacted>
to: Andrew Lunn <andrew@lunn.ch>
data: 2023/5/6

Re: [PATCH v2 2/2] net: phy: motorcomm: Add pad drive strength cfg support
From: Andrew Lunn <andrew@lunn.ch>
to: Samin Guo <redacted>
data: 2023/5/5

quoted

quoted

 #define YTPHY_DTS_OUTPUT_CLK_DIS		0

@@ -1495,6 +1504,7 @@ static int yt8531_config_init(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
 	int ret;
+	u32 ds, val;

Reverse Christmas tree.  Sort these longest first, shortest last.

Thanks, will fix.

quoted

Otherwise this looks O.K.

The only open question is if real unit should be used, uA, not some
magic numbers. Lets see what the DT Maintainers say.

      Andrew

Hi Andrew,

As I communicated with Frank, Motorcomm doesn't give specific units on their datasheet, except for magic numbers.
Tried to ask Motorcomm last week, but it seems that they themselves do not know what the unit is and have no response so far.


Below is all the relevant information I found：

Pad Drive Strength Cfg (EXT_0xA010)

Bit   |  Symbol           |  Access |  Default |  Description
15:13 |  Rgmii_sw_dr_rx   |  RW     |  0x3     |  Drive strenght of rx_clk pad.
                                               |  3'b111: strongest; 3'b000: weakest.

12    |  Rgmii_sw_dr[2]   |  RW     |  0x0     |  Bit 2 of Rgmii_sw_dr[2:0], refer to ext A010[5:4]

5:4   |  Rgmii_sw_dr[1:0] |  RW     |  0x3     |  Bit 1 and 0 of Rgmii_sw_dr, Drive strenght of rxd/rx_ctl rgmii pad.
                                               |  3'b111: strongest; 3'b000: weakest



Best regards,
Samin

Hi Andrew,

We tried contacting motorcomm again, but so far we haven't been able to get any more information about unit.
Also, I found a similar configuration in Documentation/devicetree/bindings/net/qca,ar803x.yaml, and they also
used the 'magic numbers':

  qca,clk-out-strength:
    description: Clock output driver strength.
    $ref: /schemas/types.yaml#/definitions/uint32
    enum: [0, 1, 2]


Best regards,
Samin