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
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 <[email protected]>
---
.../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
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 | 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
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 <[email protected]>
> ---
> .../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
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 <[email protected]>
> ---
> 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;
> }
>
> > + 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
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 <[email protected]>
> ---
> .../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 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg
From: Frank Sae <[email protected]>
to: Samin Guo <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, Peter Geis <[email protected]>
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 <[email protected]>
>> ---
>> .../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
From: Frank Sae <[email protected]>
to: Samin Guo <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, Peter Geis <[email protected]>
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 <[email protected]>
>> ---
>> 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
From: Andrew Lunn <[email protected]>
to: Frank Sae <[email protected]>
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
Re: [PATCH v1 1/2] dt-bindings: net: motorcomm: Add pad driver strength cfg
From: Andrew Lunn <[email protected]>
to: Samin Guo <[email protected]>
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 <[email protected]>
>> ---
>> .../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