[PATCH net] net: phy: dp83867: add w/a for packet errors seen with short cables

[PATCH net] net: phy: dp83867: add w/a for packet errors seen with short cables

[Siddharth Vadapalli]

From: Grygorii Strashko <grygorii.strashko@ti.com>

Introduce the W/A for packet errors seen with short cables (<1m) between
two DP83867 PHYs.

The W/A recommended by DM requires FFE Equalizer Configuration tuning by
writing value 0x0E81 to DSP_FFE_CFG register (0x012C), surrounded by hard
and soft resets as follows:

write_reg(0x001F, 0x8000); //hard reset
write_reg(DSP_FFE_CFG, 0x0E81);
write_reg(0x001F, 0x4000); //soft reset

Since  DP83867 PHY DM says "Changing this register to 0x0E81, will not
affect Long Cable performance.", enable the W/A by default.

Fixes: 2a10154abcb7 ("net: phy: dp83867: Add TI dp83867 phy")
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

RFC patch at:
https://lore.kernel.org/r/20230425054429.3956535-2-s-vadapalli@ti.com/

Changes since RFC patch:
- Change patch subject to PATCH net.
- Add Fixes tag.
- Check return value of phy_write_mmd().

 drivers/net/phy/dp83867.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index d75f526a20a4..4eecc9531a6b 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -44,6 +44,7 @@
 #define DP83867_STRAP_STS1	0x006E
 #define DP83867_STRAP_STS2	0x006f
 #define DP83867_RGMIIDCTL	0x0086
+#define DP83867_DSP_FFE_CFG	0X012C
 #define DP83867_RXFCFG		0x0134
 #define DP83867_RXFPMD1	0x0136
 #define DP83867_RXFPMD2	0x0137
@@ -941,8 +942,22 @@ static int dp83867_phy_reset(struct phy_device *phydev)
 
 	usleep_range(10, 20);
 
-	return phy_modify(phydev, MII_DP83867_PHYCTRL,
+	err = phy_modify(phydev, MII_DP83867_PHYCTRL,
 			 DP83867_PHYCR_FORCE_LINK_GOOD, 0);
+	if (err < 0)
+		return err;
+
+	err = phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_DSP_FFE_CFG, 0X0E81);
+	if (err < 0)
+		return err;
+
+	err = phy_write(phydev, DP83867_CTRL, DP83867_SW_RESTART);
+	if (err < 0)
+		return err;
+
+	usleep_range(10, 20);
+
+	return 0;
 }
 
 static void dp83867_link_change_notify(struct phy_device *phydev)
-- 
2.25.1

Re: [PATCH net] net: phy: dp83867: add w/a for packet errors seen with short cables

[Jakub Kicinski]

Thanks for the patch! Some nit picks below..

On Mon, 8 May 2023 12:30:19 +0530 Siddharth Vadapalli wrote:
> +	err = phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_DSP_FFE_CFG, 0X0E81);

Pleas wrap this line at 80 characters, there's no reason for it 
to be this long.

And 0x prefix should not be in upper case, here and in the new define
you're adding.
-- 
pw-bot: cr

Re: [PATCH net] net: phy: dp83867: add w/a for packet errors seen with short cables

[Siddharth Vadapalli]

On 09/05/23 07:30, Jakub Kicinski wrote:
> Thanks for the patch! Some nit picks below..
> 
> On Mon, 8 May 2023 12:30:19 +0530 Siddharth Vadapalli wrote:
>> +	err = phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_DSP_FFE_CFG, 0X0E81);
> 
> Pleas wrap this line at 80 characters, there's no reason for it 
> to be this long.
> 
> And 0x prefix should not be in upper case, here and in the new define
> you're adding.

Thank you for pointing out the issues. I will fix them and post the v2 patch.

-- 
Regards,
Siddharth.