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/[email protected]
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, 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 <[email protected]>
---
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
> #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.
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 2023/5/5 17:05, 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 <[email protected]>
> ---
> 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 */
...
> +
> /* 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;
> }
>
data: Re: [PATCH v2 2/2] net: phy: motorcomm: Add pad drive strength cfg support
From: Frank Sae <[email protected]>
to: Samin Guo <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, Peter Geis <[email protected]>
data: 2023/5/6
>
>
> On 2023/5/5 17:05, 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 <[email protected]>
>> ---
>> 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
>> +
>> /* 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 <[email protected]>
to: Samin Guo <[email protected]>
data: 2023/5/5
>> #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 <[email protected]>
to: Andrew Lunn <[email protected]>
data: 2023/5/6
>
> Re: [PATCH v2 2/2] net: phy: motorcomm: Add pad drive strength cfg support
> From: Andrew Lunn <[email protected]>
> to: Samin Guo <[email protected]>
> data: 2023/5/5
>
>>> #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
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