[PATCH net v2] net: phy: marvell: Fix RGMII Tx/Rx delays setting in 88e1121-compatible PHYs
It is mandatory for a software to issue a reset upon modifying RGMII Receive Timing Control and RGMII Transmit Timing Control bit fields of MAC Specific Control register 2 (page 2, register 21) otherwise the changes won't be perceived by the PHY (the same is applicable for a lot of other registers). Not setting the RGMII delays on the platforms that imply it' being done on the PHY side will consequently cause the traffic loss. We discovered that the denoted soft-reset is missing in the m88e1121_config_aneg() method for the case if the RGMII delays are modified but the MDIx polarity isn't changed or the auto-negotiation is left enabled, thus causing the traffic loss on our platform with Marvell Alaska 88E1510 installed. Let's fix that by issuing the soft-reset if the delays have been actually set in the m88e1121_config_aneg_rgmii_delays() method.
Fixes: d6ab93364734 ("net: phy: marvell: Avoid unnecessary soft reset") Signed-off-by: Pavel Parkhomenko Pavel.Parkhomenko@baikalelectronics.ru Reviewed-by: Russell King (Oracle) rmk+kernel@armlinux.org.uk Reviewed-by: Serge Semin fancer.lancer@gmail.com Cc: stable@vger.kernel.org
---
Link: https://lore.kernel.org/netdev/96759fee7240fd095cb9cc1f6eaf2d9113b57cf0.came... Changelog v2: - Add "net" suffix into the PATCH-clause of the subject. - Cc the patch to the stable tree list. - Rebase onto the latset netdev/net branch with the top commit 59085208e4a2 ("net: mscc: ocelot: fix all IP traffic getting trapped to CPU with PTP over IP") --- drivers/net/phy/marvell.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index fa71fb7a66b5..e2fd1252be48 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -553,9 +553,9 @@ static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev) else mscr = 0;
- return phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE, - MII_88E1121_PHY_MSCR_REG, - MII_88E1121_PHY_MSCR_DELAY_MASK, mscr); + return phy_modify_paged_changed(phydev, MII_MARVELL_MSCR_PAGE, + MII_88E1121_PHY_MSCR_REG, + MII_88E1121_PHY_MSCR_DELAY_MASK, mscr); }
static int m88e1121_config_aneg(struct phy_device *phydev) @@ -569,11 +569,13 @@ static int m88e1121_config_aneg(struct phy_device *phydev) return err; }
+ changed = err; + err = marvell_set_polarity(phydev, phydev->mdix_ctrl); if (err < 0) return err;
- changed = err; + changed |= err;
err = genphy_config_aneg(phydev); if (err < 0)
On Sat, 5 Feb 2022 23:39:32 +0300 Pavel Parkhomenko wrote:
It is mandatory for a software to issue a reset upon modifying RGMII Receive Timing Control and RGMII Transmit Timing Control bit fields of MAC Specific Control register 2 (page 2, register 21) otherwise the changes won't be perceived by the PHY (the same is applicable for a lot of other registers). Not setting the RGMII delays on the platforms that imply it' being done on the PHY side will consequently cause the traffic loss. We discovered that the denoted soft-reset is missing in the m88e1121_config_aneg() method for the case if the RGMII delays are modified but the MDIx polarity isn't changed or the auto-negotiation is left enabled, thus causing the traffic loss on our platform with Marvell Alaska 88E1510 installed. Let's fix that by issuing the soft-reset if the delays have been actually set in the m88e1121_config_aneg_rgmii_delays() method.
Fixes: d6ab93364734 ("net: phy: marvell: Avoid unnecessary soft reset") Signed-off-by: Pavel Parkhomenko Pavel.Parkhomenko@baikalelectronics.ru Reviewed-by: Russell King (Oracle) rmk+kernel@armlinux.org.uk Reviewed-by: Serge Semin fancer.lancer@gmail.com Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/netdev/96759fee7240fd095cb9cc1f6eaf2d9113b57cf0.came... Changelog v2:
- Add "net" suffix into the PATCH-clause of the subject.
- Cc the patch to the stable tree list.
- Rebase onto the latset netdev/net branch with the top commit 59085208e4a2
("net: mscc: ocelot: fix all IP traffic getting trapped to CPU with PTP over IP")
This patch is valid and waiting to be reviewed & applied, right? I see it's marked as Superseded in patchwork, but can't track down a v3.
Hello Jakub
On Mon, Feb 07, 2022 at 09:40:39AM -0800, Jakub Kicinski wrote:
On Sat, 5 Feb 2022 23:39:32 +0300 Pavel Parkhomenko wrote:
It is mandatory for a software to issue a reset upon modifying RGMII Receive Timing Control and RGMII Transmit Timing Control bit fields of MAC Specific Control register 2 (page 2, register 21) otherwise the changes won't be perceived by the PHY (the same is applicable for a lot of other registers). Not setting the RGMII delays on the platforms that imply it' being done on the PHY side will consequently cause the traffic loss. We discovered that the denoted soft-reset is missing in the m88e1121_config_aneg() method for the case if the RGMII delays are modified but the MDIx polarity isn't changed or the auto-negotiation is left enabled, thus causing the traffic loss on our platform with Marvell Alaska 88E1510 installed. Let's fix that by issuing the soft-reset if the delays have been actually set in the m88e1121_config_aneg_rgmii_delays() method.
Fixes: d6ab93364734 ("net: phy: marvell: Avoid unnecessary soft reset") Signed-off-by: Pavel Parkhomenko Pavel.Parkhomenko@baikalelectronics.ru Reviewed-by: Russell King (Oracle) rmk+kernel@armlinux.org.uk Reviewed-by: Serge Semin fancer.lancer@gmail.com Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/netdev/96759fee7240fd095cb9cc1f6eaf2d9113b57cf0.came... Changelog v2:
- Add "net" suffix into the PATCH-clause of the subject.
- Cc the patch to the stable tree list.
- Rebase onto the latset netdev/net branch with the top commit 59085208e4a2
("net: mscc: ocelot: fix all IP traffic getting trapped to CPU with PTP over IP")
This patch is valid and waiting to be reviewed & applied, right?
Right.
I see it's marked as Superseded in patchwork, but can't track down a v3.
We had accidentally sent out a temporal v2 version before submitting this one. The failed patch is here Link: https://lore.kernel.org/stable/20220205190814.20282-1-Pavel.Parkhomenko@baik... But the message was sent to Russel and to the stable mailing list only with no netdev list being in Cc. I thought if the right v2 was sent out after the failed one, then even if patchwork somehow gets to catch both of the messages, the former patch would have at least superseded the later one. It appears I was wrong. Sorry about that. Do you want us to resend this patch as v3 to have a proper patchwork status?
-Sergey
On Mon, 7 Feb 2022 21:33:19 +0300 Serge Semin wrote:
I see it's marked as Superseded in patchwork, but can't track down a v3.
We had accidentally sent out a temporal v2 version before submitting this one. The failed patch is here Link: https://lore.kernel.org/stable/20220205190814.20282-1-Pavel.Parkhomenko@baik... But the message was sent to Russel and to the stable mailing list only with no netdev list being in Cc. I thought if the right v2 was sent out after the failed one, then even if patchwork somehow gets to catch both of the messages, the former patch would have at least superseded the later one. It appears I was wrong. Sorry about that. Do you want us to resend this patch as v3 to have a proper patchwork status?
No need, I set it back to New, thanks!
On Mon, 7 Feb 2022 11:22:39 -0800 Jakub Kicinski wrote:
On Mon, 7 Feb 2022 21:33:19 +0300 Serge Semin wrote:
I see it's marked as Superseded in patchwork, but can't track down a v3.
We had accidentally sent out a temporal v2 version before submitting this one. The failed patch is here Link: https://lore.kernel.org/stable/20220205190814.20282-1-Pavel.Parkhomenko@baik... But the message was sent to Russel and to the stable mailing list only with no netdev list being in Cc. I thought if the right v2 was sent out after the failed one, then even if patchwork somehow gets to catch both of the messages, the former patch would have at least superseded the later one. It appears I was wrong. Sorry about that. Do you want us to resend this patch as v3 to have a proper patchwork status?
No need, I set it back to New, thanks!
Applied now, commit fe4f57bf7b58 ("net: phy: marvell: Fix RGMII Tx/Rx delays setting in 88e1121-compatible PHYs"), thanks!
Hello:
This patch was applied to netdev/net.git (master) by Jakub Kicinski kuba@kernel.org:
On Sat, 5 Feb 2022 23:39:32 +0300 you wrote:
It is mandatory for a software to issue a reset upon modifying RGMII Receive Timing Control and RGMII Transmit Timing Control bit fields of MAC Specific Control register 2 (page 2, register 21) otherwise the changes won't be perceived by the PHY (the same is applicable for a lot of other registers). Not setting the RGMII delays on the platforms that imply it' being done on the PHY side will consequently cause the traffic loss. We discovered that the denoted soft-reset is missing in the m88e1121_config_aneg() method for the case if the RGMII delays are modified but the MDIx polarity isn't changed or the auto-negotiation is left enabled, thus causing the traffic loss on our platform with Marvell Alaska 88E1510 installed. Let's fix that by issuing the soft-reset if the delays have been actually set in the m88e1121_config_aneg_rgmii_delays() method.
[...]
Here is the summary with links: - [net,v2] net: phy: marvell: Fix RGMII Tx/Rx delays setting in 88e1121-compatible PHYs https://git.kernel.org/netdev/net/c/fe4f57bf7b58
You are awesome, thank you!
linux-stable-mirror@lists.linaro.org
-
Jakub Kicinski -
patchwork-bot+netdevbpf＠kernel.org -
Pavel Parkhomenko -
Serge Semin