2021-06-29 10:05:57

by Harini Katakam

[permalink] [raw]
Subject: [PATCH 0/3] Add support for VSC8531_02 PHY and DT RGMII tuning

Add support for VSC8531_02 PHY ID.
Also provide an option to tune RGMII delay value via devicetree.
The default delays are retained in the driver.

Harini Katakam (3):
include: dt-bindings: Add mscc-vsc8531 RGMII clock delay definitions
dt-bindings: mscc: Add RGMII RX and TX delay tuning
phy: mscc: Add support for VSC8531_02 with RGMII tuning

.../bindings/net/mscc-phy-vsc8531.txt | 9 +++-
drivers/net/phy/mscc/mscc.h | 3 ++
drivers/net/phy/mscc/mscc_main.c | 41 ++++++++++++++++++-
include/dt-bindings/net/mscc-phy-vsc8531.h | 9 ++++
4 files changed, 59 insertions(+), 3 deletions(-)

--
2.17.1


2021-06-29 10:08:39

by Harini Katakam

[permalink] [raw]
Subject: [PATCH 3/3] phy: mscc: Add support for VSC8531_02 with RGMII tuning

Add support for VSC8531_02 (Rev 2) device.
Add support for optional RGMII RX and TX delay tuning via devicetree.
The hierarchy is:
- Retain the defaul 0.2ns delay when RGMII tuning is not set.
- Retain the default 2ns delay when RGMII tuning is set and DT delay
property is NOT specified.
- Use the DT delay value when RGMII tuning is set and a DT delay
property is specified.

Signed-off-by: Harini Katakam <[email protected]>
Signed-off-by: Radhey Shyam Pandey <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
---
drivers/net/phy/mscc/mscc.h | 3 +++
drivers/net/phy/mscc/mscc_main.c | 41 ++++++++++++++++++++++++++++++--
2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index a50235fdf7d9..5a26eba0ace0 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -281,6 +281,7 @@ enum rgmii_clock_delay {
#define PHY_ID_VSC8514 0x00070670
#define PHY_ID_VSC8530 0x00070560
#define PHY_ID_VSC8531 0x00070570
+#define PHY_ID_VSC8531_02 0x00070572
#define PHY_ID_VSC8540 0x00070760
#define PHY_ID_VSC8541 0x00070770
#define PHY_ID_VSC8552 0x000704e0
@@ -373,6 +374,8 @@ struct vsc8531_private {
* package.
*/
unsigned int base_addr;
+ u32 rx_delay;
+ u32 tx_delay;

#if IS_ENABLED(CONFIG_MACSEC)
/* MACsec fields:
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 6e32da28e138..2b501824c802 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -535,15 +535,16 @@ static int vsc85xx_rgmii_set_skews(struct phy_device *phydev, u32 rgmii_cntl,
u16 rgmii_tx_delay_pos = ffs(rgmii_tx_delay_mask) - 1;
u16 reg_val = 0;
int rc;
+ struct vsc8531_private *vsc8531 = phydev->priv;

mutex_lock(&phydev->lock);

if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
- reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_rx_delay_pos;
+ reg_val |= vsc8531->rx_delay << rgmii_rx_delay_pos;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
- reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_tx_delay_pos;
+ reg_val |= vsc8531->tx_delay << rgmii_tx_delay_pos;

rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
rgmii_cntl,
@@ -1820,6 +1821,17 @@ static int vsc85xx_config_init(struct phy_device *phydev)
{
int rc, i, phy_id;
struct vsc8531_private *vsc8531 = phydev->priv;
+ struct device_node *of_node = phydev->mdio.dev.of_node;
+
+ rc = of_property_read_u32(of_node, "vsc8531,rx-delay",
+ &vsc8531->rx_delay);
+ if (rc < 0)
+ vsc8531->rx_delay = RGMII_CLK_DELAY_2_0_NS;
+
+ rc = of_property_read_u32(of_node, "vsc8531,tx-delay",
+ &vsc8531->tx_delay);
+ if (rc < 0)
+ vsc8531->tx_delay = RGMII_CLK_DELAY_2_0_NS;

rc = vsc85xx_default_config(phydev);
if (rc)
@@ -2444,6 +2456,30 @@ static struct phy_driver vsc85xx_driver[] = {
.get_strings = &vsc85xx_get_strings,
.get_stats = &vsc85xx_get_stats,
},
+{
+ .phy_id = PHY_ID_VSC8531_02,
+ .name = "Microsemi VSC8531-02",
+ .phy_id_mask = 0xfffffff0,
+ /* PHY_GBIT_FEATURES */
+ .soft_reset = &genphy_soft_reset,
+ .config_init = &vsc85xx_config_init,
+ .config_aneg = &vsc85xx_config_aneg,
+ .read_status = &vsc85xx_read_status,
+ .handle_interrupt = vsc85xx_handle_interrupt,
+ .config_intr = &vsc85xx_config_intr,
+ .suspend = &genphy_suspend,
+ .resume = &genphy_resume,
+ .probe = &vsc85xx_probe,
+ .set_wol = &vsc85xx_wol_set,
+ .get_wol = &vsc85xx_wol_get,
+ .get_tunable = &vsc85xx_get_tunable,
+ .set_tunable = &vsc85xx_set_tunable,
+ .read_page = &vsc85xx_phy_read_page,
+ .write_page = &vsc85xx_phy_write_page,
+ .get_sset_count = &vsc85xx_get_sset_count,
+ .get_strings = &vsc85xx_get_strings,
+ .get_stats = &vsc85xx_get_stats,
+},
{
.phy_id = PHY_ID_VSC8540,
.name = "Microsemi FE VSC8540 SyncE",
@@ -2668,6 +2704,7 @@ static struct mdio_device_id __maybe_unused vsc85xx_tbl[] = {
{ PHY_ID_VSC8514, 0xfffffff0, },
{ PHY_ID_VSC8530, 0xfffffff0, },
{ PHY_ID_VSC8531, 0xfffffff0, },
+ { PHY_ID_VSC8531_02, 0xfffffff0, },
{ PHY_ID_VSC8540, 0xfffffff0, },
{ PHY_ID_VSC8541, 0xfffffff0, },
{ PHY_ID_VSC8552, 0xfffffff0, },
--
2.17.1

2021-06-29 10:45:38

by Heiner Kallweit

[permalink] [raw]
Subject: Re: [PATCH 3/3] phy: mscc: Add support for VSC8531_02 with RGMII tuning

On 29.06.2021 11:40, Harini Katakam wrote:
> Add support for VSC8531_02 (Rev 2) device.
> Add support for optional RGMII RX and TX delay tuning via devicetree.
> The hierarchy is:
> - Retain the defaul 0.2ns delay when RGMII tuning is not set.
> - Retain the default 2ns delay when RGMII tuning is set and DT delay
> property is NOT specified.
> - Use the DT delay value when RGMII tuning is set and a DT delay
> property is specified.
>
> Signed-off-by: Harini Katakam <[email protected]>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>
> ---
> drivers/net/phy/mscc/mscc.h | 3 +++
> drivers/net/phy/mscc/mscc_main.c | 41 ++++++++++++++++++++++++++++++--
> 2 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
> index a50235fdf7d9..5a26eba0ace0 100644
> --- a/drivers/net/phy/mscc/mscc.h
> +++ b/drivers/net/phy/mscc/mscc.h
> @@ -281,6 +281,7 @@ enum rgmii_clock_delay {
> #define PHY_ID_VSC8514 0x00070670
> #define PHY_ID_VSC8530 0x00070560
> #define PHY_ID_VSC8531 0x00070570
> +#define PHY_ID_VSC8531_02 0x00070572
> #define PHY_ID_VSC8540 0x00070760
> #define PHY_ID_VSC8541 0x00070770
> #define PHY_ID_VSC8552 0x000704e0
> @@ -373,6 +374,8 @@ struct vsc8531_private {
> * package.
> */
> unsigned int base_addr;
> + u32 rx_delay;
> + u32 tx_delay;
>
> #if IS_ENABLED(CONFIG_MACSEC)
> /* MACsec fields:
> diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
> index 6e32da28e138..2b501824c802 100644
> --- a/drivers/net/phy/mscc/mscc_main.c
> +++ b/drivers/net/phy/mscc/mscc_main.c
> @@ -535,15 +535,16 @@ static int vsc85xx_rgmii_set_skews(struct phy_device *phydev, u32 rgmii_cntl,
> u16 rgmii_tx_delay_pos = ffs(rgmii_tx_delay_mask) - 1;
> u16 reg_val = 0;
> int rc;
> + struct vsc8531_private *vsc8531 = phydev->priv;
>
> mutex_lock(&phydev->lock);
>
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> - reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_rx_delay_pos;
> + reg_val |= vsc8531->rx_delay << rgmii_rx_delay_pos;
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> - reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_tx_delay_pos;
> + reg_val |= vsc8531->tx_delay << rgmii_tx_delay_pos;
>
> rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
> rgmii_cntl,
> @@ -1820,6 +1821,17 @@ static int vsc85xx_config_init(struct phy_device *phydev)
> {
> int rc, i, phy_id;
> struct vsc8531_private *vsc8531 = phydev->priv;
> + struct device_node *of_node = phydev->mdio.dev.of_node;
> +
> + rc = of_property_read_u32(of_node, "vsc8531,rx-delay",
> + &vsc8531->rx_delay);
> + if (rc < 0)
> + vsc8531->rx_delay = RGMII_CLK_DELAY_2_0_NS;
> +
> + rc = of_property_read_u32(of_node, "vsc8531,tx-delay",
> + &vsc8531->tx_delay);
> + if (rc < 0)
> + vsc8531->tx_delay = RGMII_CLK_DELAY_2_0_NS;
>
> rc = vsc85xx_default_config(phydev);
> if (rc)
> @@ -2444,6 +2456,30 @@ static struct phy_driver vsc85xx_driver[] = {
> .get_strings = &vsc85xx_get_strings,
> .get_stats = &vsc85xx_get_stats,
> },
> +{
> + .phy_id = PHY_ID_VSC8531_02,
> + .name = "Microsemi VSC8531-02",
> + .phy_id_mask = 0xfffffff0,

VSC8531 and VSC8531-02 share the same model number, therefore you
would have to switch both PHY drivers to exact match.
phy_id_mask = 0xffffffff, or better use macro PHY_ID_MATCH_EXACT().


> + /* PHY_GBIT_FEATURES */
> + .soft_reset = &genphy_soft_reset,
> + .config_init = &vsc85xx_config_init,
> + .config_aneg = &vsc85xx_config_aneg,
> + .read_status = &vsc85xx_read_status,
> + .handle_interrupt = vsc85xx_handle_interrupt,
> + .config_intr = &vsc85xx_config_intr,
> + .suspend = &genphy_suspend,
> + .resume = &genphy_resume,
> + .probe = &vsc85xx_probe,
> + .set_wol = &vsc85xx_wol_set,
> + .get_wol = &vsc85xx_wol_get,
> + .get_tunable = &vsc85xx_get_tunable,
> + .set_tunable = &vsc85xx_set_tunable,
> + .read_page = &vsc85xx_phy_read_page,
> + .write_page = &vsc85xx_phy_write_page,
> + .get_sset_count = &vsc85xx_get_sset_count,
> + .get_strings = &vsc85xx_get_strings,
> + .get_stats = &vsc85xx_get_stats,
> +},
> {
> .phy_id = PHY_ID_VSC8540,
> .name = "Microsemi FE VSC8540 SyncE",
> @@ -2668,6 +2704,7 @@ static struct mdio_device_id __maybe_unused vsc85xx_tbl[] = {
> { PHY_ID_VSC8514, 0xfffffff0, },
> { PHY_ID_VSC8530, 0xfffffff0, },
> { PHY_ID_VSC8531, 0xfffffff0, },
> + { PHY_ID_VSC8531_02, 0xfffffff0, },

Effectively this is the same as the line before. Maybe it would make sense
to change this table in a follow-up patch to just one entry covering all
PHY ID's with the vendor part being Microsemi, e.g. using macro
PHY_ID_MATCH_VENDOR().

> { PHY_ID_VSC8540, 0xfffffff0, },
> { PHY_ID_VSC8541, 0xfffffff0, },
> { PHY_ID_VSC8552, 0xfffffff0, },
>

2021-06-29 11:48:28

by Harini Katakam

[permalink] [raw]
Subject: [PATCH 2/3] dt-bindings: mscc: Add RGMII RX and TX delay tuning

Add optional properties to tune RGMII RX and TX delay.

Signed-off-by: Harini Katakam <[email protected]>
Signed-off-by: Radhey Shyam Pandey <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
---
.../devicetree/bindings/net/mscc-phy-vsc8531.txt | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
index 87a27d775d48..e201d24d8e27 100644
--- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
+++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
@@ -33,7 +33,14 @@ Optional properties:
VSC8531_DUPLEX_COLLISION (8).
- load-save-gpios : GPIO used for the load/save operation of the PTP
hardware clock (PHC).
-
+- vsc8531,rx-delay : RGMII RX delay. Allowed values are defined in
+ "include/dt-bindings/net/mscc-phy-vsc8531.h".
+ Default value, set by the driver is
+ VSC8531_RGMII_CLK_DELAY_2_0_NS.
+- vsc8531,tx-delay : RGMII TX delay. Allowed values are defined in
+ "include/dt-bindings/net/mscc-phy-vsc8531.h".
+ Default value, set by the driver is
+ VSC8531_RGMII_CLK_DELAY_2_0_NS.

Table: 1 - Edge rate change
----------------------------------------------------------------|
--
2.17.1

2021-06-29 14:08:49

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: mscc: Add RGMII RX and TX delay tuning

On Tue, Jun 29, 2021 at 03:10:37PM +0530, Harini Katakam wrote:
> Add optional properties to tune RGMII RX and TX delay.
>
> Signed-off-by: Harini Katakam <[email protected]>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>
> ---
> .../devicetree/bindings/net/mscc-phy-vsc8531.txt | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> index 87a27d775d48..e201d24d8e27 100644
> --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> @@ -33,7 +33,14 @@ Optional properties:
> VSC8531_DUPLEX_COLLISION (8).
> - load-save-gpios : GPIO used for the load/save operation of the PTP
> hardware clock (PHC).
> -
> +- vsc8531,rx-delay : RGMII RX delay. Allowed values are defined in
> + "include/dt-bindings/net/mscc-phy-vsc8531.h".
> + Default value, set by the driver is
> + VSC8531_RGMII_CLK_DELAY_2_0_NS.
> +- vsc8531,tx-delay : RGMII TX delay. Allowed values are defined in
> + "include/dt-bindings/net/mscc-phy-vsc8531.h".
> + Default value, set by the driver is
> + VSC8531_RGMII_CLK_DELAY_2_0_NS.

The default values need better explanation. So you are saying they are
only used when 'rgmii' is not used. And they replace the default 2ns
delay, they don't add to the default 2ns delay.

Andrew

2021-06-29 14:15:57

by Harini Katakam

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: mscc: Add RGMII RX and TX delay tuning

Hi Andrew,

On Tue, Jun 29, 2021 at 7:37 PM Andrew Lunn <[email protected]> wrote:
>
> On Tue, Jun 29, 2021 at 03:10:37PM +0530, Harini Katakam wrote:
> > Add optional properties to tune RGMII RX and TX delay.
> >
> > Signed-off-by: Harini Katakam <[email protected]>
> > Signed-off-by: Radhey Shyam Pandey <[email protected]>
> > Signed-off-by: Michal Simek <[email protected]>
> > ---
> > .../devicetree/bindings/net/mscc-phy-vsc8531.txt | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> > index 87a27d775d48..e201d24d8e27 100644
> > --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> > +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> > @@ -33,7 +33,14 @@ Optional properties:
> > VSC8531_DUPLEX_COLLISION (8).
> > - load-save-gpios : GPIO used for the load/save operation of the PTP
> > hardware clock (PHC).
> > -
> > +- vsc8531,rx-delay : RGMII RX delay. Allowed values are defined in
> > + "include/dt-bindings/net/mscc-phy-vsc8531.h".
> > + Default value, set by the driver is
> > + VSC8531_RGMII_CLK_DELAY_2_0_NS.
> > +- vsc8531,tx-delay : RGMII TX delay. Allowed values are defined in
> > + "include/dt-bindings/net/mscc-phy-vsc8531.h".
> > + Default value, set by the driver is
> > + VSC8531_RGMII_CLK_DELAY_2_0_NS.
>
> The default values need better explanation. So you are saying they are
> only used when 'rgmii' is not used. And they replace the default 2ns
> delay, they don't add to the default 2ns delay.

Thanks for the review. Yes, I'm saying that they replace the default 2ns delay.
But they only come in when rgmii or rgmii-id is used. When that's not used,
the default 0.2ns in the driver is retained. I'll update the DT description here
to be clear.

Regards,
Harini

2021-06-29 14:16:28

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 3/3] phy: mscc: Add support for VSC8531_02 with RGMII tuning

> @@ -535,15 +535,16 @@ static int vsc85xx_rgmii_set_skews(struct phy_device *phydev, u32 rgmii_cntl,
> u16 rgmii_tx_delay_pos = ffs(rgmii_tx_delay_mask) - 1;
> u16 reg_val = 0;
> int rc;
> + struct vsc8531_private *vsc8531 = phydev->priv;

reverse christmass tree.


> mutex_lock(&phydev->lock);
>
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> - reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_rx_delay_pos;
> + reg_val |= vsc8531->rx_delay << rgmii_rx_delay_pos;
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> - reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_tx_delay_pos;
> + reg_val |= vsc8531->tx_delay << rgmii_tx_delay_pos;
>
> rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
> rgmii_cntl,
> @@ -1820,6 +1821,17 @@ static int vsc85xx_config_init(struct phy_device *phydev)
> {
> int rc, i, phy_id;
> struct vsc8531_private *vsc8531 = phydev->priv;
> + struct device_node *of_node = phydev->mdio.dev.of_node;
> +
> + rc = of_property_read_u32(of_node, "vsc8531,rx-delay",
> + &vsc8531->rx_delay);
> + if (rc < 0)
> + vsc8531->rx_delay = RGMII_CLK_DELAY_2_0_NS;

of_property_read_u32() is guaranteed not to touch the result value, if
it is not in device tree. So you can simplify this to:

vsc8531->rx_delay = RGMII_CLK_DELAY_2_0_NS;
of_property_read_u32(of_node, "vsc8531,rx-delay", &vsc8531->rx_delay);

2021-06-29 14:22:35

by Harini Katakam

[permalink] [raw]
Subject: Re: [PATCH 3/3] phy: mscc: Add support for VSC8531_02 with RGMII tuning

Hi Heiner,

On Tue, Jun 29, 2021 at 4:11 PM Heiner Kallweit <[email protected]> wrote:
>
> On 29.06.2021 11:40, Harini Katakam wrote:
> > Add support for VSC8531_02 (Rev 2) device.
> > Add support for optional RGMII RX and TX delay tuning via devicetree.
> > The hierarchy is:
> > - Retain the defaul 0.2ns delay when RGMII tuning is not set.
> > - Retain the default 2ns delay when RGMII tuning is set and DT delay
> > property is NOT specified.
> > - Use the DT delay value when RGMII tuning is set and a DT delay
> > property is specified.
> >
> > Signed-off-by: Harini Katakam <[email protected]>
> > Signed-off-by: Radhey Shyam Pandey <[email protected]>
> > Signed-off-by: Michal Simek <[email protected]>
> > ---
> > drivers/net/phy/mscc/mscc.h | 3 +++
> > drivers/net/phy/mscc/mscc_main.c | 41 ++++++++++++++++++++++++++++++--
> > 2 files changed, 42 insertions(+), 2 deletions(-)
> >
<snip>
> {
> > .phy_id = PHY_ID_VSC8540,
> > .name = "Microsemi FE VSC8540 SyncE",
> > @@ -2668,6 +2704,7 @@ static struct mdio_device_id __maybe_unused vsc85xx_tbl[] = {
> > { PHY_ID_VSC8514, 0xfffffff0, },
> > { PHY_ID_VSC8530, 0xfffffff0, },
> > { PHY_ID_VSC8531, 0xfffffff0, },
> > + { PHY_ID_VSC8531_02, 0xfffffff0, },
>
> Effectively this is the same as the line before. Maybe it would make sense
> to change this table in a follow-up patch to just one entry covering all
> PHY ID's with the vendor part being Microsemi, e.g. using macro
> PHY_ID_MATCH_VENDOR().

Thanks for the review. Let me check and test - I have a couple of these parts.
Will try to include the patch in this series.

Regards,
Harini

2021-07-14 21:10:28

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: mscc: Add RGMII RX and TX delay tuning

On Tue, Jun 29, 2021 at 03:10:37PM +0530, Harini Katakam wrote:
> Add optional properties to tune RGMII RX and TX delay.
>
> Signed-off-by: Harini Katakam <[email protected]>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>

Your S-o-b should be last as the last one to touch this.

> ---
> .../devicetree/bindings/net/mscc-phy-vsc8531.txt | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> index 87a27d775d48..e201d24d8e27 100644
> --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> @@ -33,7 +33,14 @@ Optional properties:
> VSC8531_DUPLEX_COLLISION (8).
> - load-save-gpios : GPIO used for the load/save operation of the PTP
> hardware clock (PHC).
> -
> +- vsc8531,rx-delay : RGMII RX delay. Allowed values are defined in

'vsc8531' is not a vendor. The form is <vendor>,<propname>.

> + "include/dt-bindings/net/mscc-phy-vsc8531.h".
> + Default value, set by the driver is
> + VSC8531_RGMII_CLK_DELAY_2_0_NS.
> +- vsc8531,tx-delay : RGMII TX delay. Allowed values are defined in
> + "include/dt-bindings/net/mscc-phy-vsc8531.h".
> + Default value, set by the driver is
> + VSC8531_RGMII_CLK_DELAY_2_0_NS.
>
> Table: 1 - Edge rate change
> ----------------------------------------------------------------|
> --
> 2.17.1
>
>

2021-08-17 10:19:19

by Harini Katakam

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: mscc: Add RGMII RX and TX delay tuning

Hi Rob,

Thanks for the review and sorry I din't reply earlier.
<snip>
> > +- vsc8531,rx-delay : RGMII RX delay. Allowed values are defined in
>
> 'vsc8531' is not a vendor. The form is <vendor>,<propname>.

I was just aligning it with existing optional property names in this document.
Could you please let me know if mscc,rx-delay is more appropriate?

Regards,
Harini