* [PATCH net 0/3] Fixes for lynx-28g PHY driver
@ 2023-10-04 11:17 Vladimir Oltean
2023-10-04 11:17 ` [PATCH net 1/3] phy: lynx-28g: cancel the CDR check work item on the remove path Vladimir Oltean
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Vladimir Oltean @ 2023-10-04 11:17 UTC (permalink / raw
To: netdev, linux-phy
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Ioana Ciornei,
Vinod Koul, Kishon Vijay Abraham I, linux-kernel
This series fixes some issues in the Lynx 28G SerDes driver, namely an
oops when unloading the module, a race between the periodic workqueue
and the PHY API, and a race between phy_set_mode_ext() calls on multiple
lanes on the same SerDes.
Ioana Ciornei (1):
phy: lynx-28g: cancel the CDR check work item on the remove path
Vladimir Oltean (2):
phy: lynx-28g: lock PHY while performing CDR lock workaround
phy: lynx-28g: serialize concurrent phy_set_mode_ext() calls to shared
registers
drivers/phy/freescale/phy-fsl-lynx-28g.c | 27 +++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
---
I have some more work pending on this driver, hopefully for this
development cycle, which is entangled with net-next. Can this series go
through the networking tree, with the PHY maintainers' Ack? The original
driver submission also went through net-next.
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net 1/3] phy: lynx-28g: cancel the CDR check work item on the remove path
2023-10-04 11:17 [PATCH net 0/3] Fixes for lynx-28g PHY driver Vladimir Oltean
@ 2023-10-04 11:17 ` Vladimir Oltean
2023-10-04 11:17 ` [PATCH net 2/3] phy: lynx-28g: lock PHY while performing CDR lock workaround Vladimir Oltean
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2023-10-04 11:17 UTC (permalink / raw
To: netdev, linux-phy
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Ioana Ciornei,
Vinod Koul, Kishon Vijay Abraham I, linux-kernel
From: Ioana Ciornei <ioana.ciornei@nxp.com>
The blamed commit added the CDR check work item but didn't cancel it on
the remove path. Fix this by adding a remove function which takes care
of it.
Fixes: 8f73b37cf3fb ("phy: add support for the Layerscape SerDes 28G")
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/phy/freescale/phy-fsl-lynx-28g.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 4f036c77284e..c6323669f119 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -604,6 +604,14 @@ static int lynx_28g_probe(struct platform_device *pdev)
return PTR_ERR_OR_ZERO(provider);
}
+static void lynx_28g_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct lynx_28g_priv *priv = dev_get_drvdata(dev);
+
+ cancel_delayed_work_sync(&priv->cdr_check);
+}
+
static const struct of_device_id lynx_28g_of_match_table[] = {
{ .compatible = "fsl,lynx-28g" },
{ },
@@ -612,6 +620,7 @@ MODULE_DEVICE_TABLE(of, lynx_28g_of_match_table);
static struct platform_driver lynx_28g_driver = {
.probe = lynx_28g_probe,
+ .remove_new = lynx_28g_remove,
.driver = {
.name = "lynx-28g",
.of_match_table = lynx_28g_of_match_table,
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net 2/3] phy: lynx-28g: lock PHY while performing CDR lock workaround
2023-10-04 11:17 [PATCH net 0/3] Fixes for lynx-28g PHY driver Vladimir Oltean
2023-10-04 11:17 ` [PATCH net 1/3] phy: lynx-28g: cancel the CDR check work item on the remove path Vladimir Oltean
@ 2023-10-04 11:17 ` Vladimir Oltean
2023-10-04 11:17 ` [PATCH net 3/3] phy: lynx-28g: serialize concurrent phy_set_mode_ext() calls to shared registers Vladimir Oltean
2023-10-06 10:10 ` [PATCH net 0/3] Fixes for lynx-28g PHY driver patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2023-10-04 11:17 UTC (permalink / raw
To: netdev, linux-phy
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Ioana Ciornei,
Vinod Koul, Kishon Vijay Abraham I, linux-kernel
lynx_28g_cdr_lock_check() runs once per second in a workqueue to reset
the lane receiver if the CDR has not locked onto bit transitions in the
RX stream. But the PHY consumer may do stuff with the PHY simultaneously,
and that isn't okay. Block concurrent generic PHY calls by holding the
PHY mutex from this workqueue.
Fixes: 8f73b37cf3fb ("phy: add support for the Layerscape SerDes 28G")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/phy/freescale/phy-fsl-lynx-28g.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index c6323669f119..d5385d36735d 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -508,11 +508,12 @@ static void lynx_28g_cdr_lock_check(struct work_struct *work)
for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
lane = &priv->lane[i];
- if (!lane->init)
- continue;
+ mutex_lock(&lane->phy->mutex);
- if (!lane->powered_up)
+ if (!lane->init || !lane->powered_up) {
+ mutex_unlock(&lane->phy->mutex);
continue;
+ }
rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
if (!(rrstctl & LYNX_28G_LNaRRSTCTL_CDR_LOCK)) {
@@ -521,6 +522,8 @@ static void lynx_28g_cdr_lock_check(struct work_struct *work)
rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
} while (!(rrstctl & LYNX_28G_LNaRRSTCTL_RST_DONE));
}
+
+ mutex_unlock(&lane->phy->mutex);
}
queue_delayed_work(system_power_efficient_wq, &priv->cdr_check,
msecs_to_jiffies(1000));
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net 3/3] phy: lynx-28g: serialize concurrent phy_set_mode_ext() calls to shared registers
2023-10-04 11:17 [PATCH net 0/3] Fixes for lynx-28g PHY driver Vladimir Oltean
2023-10-04 11:17 ` [PATCH net 1/3] phy: lynx-28g: cancel the CDR check work item on the remove path Vladimir Oltean
2023-10-04 11:17 ` [PATCH net 2/3] phy: lynx-28g: lock PHY while performing CDR lock workaround Vladimir Oltean
@ 2023-10-04 11:17 ` Vladimir Oltean
2023-10-06 10:10 ` [PATCH net 0/3] Fixes for lynx-28g PHY driver patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2023-10-04 11:17 UTC (permalink / raw
To: netdev, linux-phy
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Ioana Ciornei,
Vinod Koul, Kishon Vijay Abraham I, linux-kernel
The protocol converter configuration registers PCC8, PCCC, PCCD
(implemented by the driver), as well as others, control protocol
converters from multiple lanes (each represented as a different
struct phy). So, if there are simultaneous calls to phy_set_mode_ext()
to lanes sharing the same PCC register (either for the "old" or for the
"new" protocol), corruption of the values programmed to hardware is
possible, because lynx_28g_rmw() has no locking.
Add a spinlock in the struct lynx_28g_priv shared by all lanes, and take
the global spinlock from the phy_ops :: set_mode() implementation. There
are no other callers which modify PCC registers.
Fixes: 8f73b37cf3fb ("phy: add support for the Layerscape SerDes 28G")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/phy/freescale/phy-fsl-lynx-28g.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index d5385d36735d..e2187767ce00 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -127,6 +127,10 @@ struct lynx_28g_lane {
struct lynx_28g_priv {
void __iomem *base;
struct device *dev;
+ /* Serialize concurrent access to registers shared between lanes,
+ * like PCCn
+ */
+ spinlock_t pcc_lock;
struct lynx_28g_pll pll[LYNX_28G_NUM_PLL];
struct lynx_28g_lane lane[LYNX_28G_NUM_LANE];
@@ -397,6 +401,8 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
if (powered_up)
lynx_28g_power_off(phy);
+ spin_lock(&priv->pcc_lock);
+
switch (submode) {
case PHY_INTERFACE_MODE_SGMII:
case PHY_INTERFACE_MODE_1000BASEX:
@@ -413,6 +419,8 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
lane->interface = submode;
out:
+ spin_unlock(&priv->pcc_lock);
+
/* Power up the lane if necessary */
if (powered_up)
lynx_28g_power_on(phy);
@@ -596,6 +604,7 @@ static int lynx_28g_probe(struct platform_device *pdev)
dev_set_drvdata(dev, priv);
+ spin_lock_init(&priv->pcc_lock);
INIT_DELAYED_WORK(&priv->cdr_check, lynx_28g_cdr_lock_check);
queue_delayed_work(system_power_efficient_wq, &priv->cdr_check,
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net 0/3] Fixes for lynx-28g PHY driver
2023-10-04 11:17 [PATCH net 0/3] Fixes for lynx-28g PHY driver Vladimir Oltean
` (2 preceding siblings ...)
2023-10-04 11:17 ` [PATCH net 3/3] phy: lynx-28g: serialize concurrent phy_set_mode_ext() calls to shared registers Vladimir Oltean
@ 2023-10-06 10:10 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-10-06 10:10 UTC (permalink / raw
To: Vladimir Oltean
Cc: netdev, linux-phy, davem, kuba, pabeni, ioana.ciornei, vkoul,
kishon, linux-kernel
Hello:
This series was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:
On Wed, 4 Oct 2023 14:17:05 +0300 you wrote:
> This series fixes some issues in the Lynx 28G SerDes driver, namely an
> oops when unloading the module, a race between the periodic workqueue
> and the PHY API, and a race between phy_set_mode_ext() calls on multiple
> lanes on the same SerDes.
>
> Ioana Ciornei (1):
> phy: lynx-28g: cancel the CDR check work item on the remove path
>
> [...]
Here is the summary with links:
- [net,1/3] phy: lynx-28g: cancel the CDR check work item on the remove path
https://git.kernel.org/netdev/net/c/f200bab3756f
- [net,2/3] phy: lynx-28g: lock PHY while performing CDR lock workaround
https://git.kernel.org/netdev/net/c/0ac87fe54a17
- [net,3/3] phy: lynx-28g: serialize concurrent phy_set_mode_ext() calls to shared registers
https://git.kernel.org/netdev/net/c/139ad1143151
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-10-06 10:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-04 11:17 [PATCH net 0/3] Fixes for lynx-28g PHY driver Vladimir Oltean
2023-10-04 11:17 ` [PATCH net 1/3] phy: lynx-28g: cancel the CDR check work item on the remove path Vladimir Oltean
2023-10-04 11:17 ` [PATCH net 2/3] phy: lynx-28g: lock PHY while performing CDR lock workaround Vladimir Oltean
2023-10-04 11:17 ` [PATCH net 3/3] phy: lynx-28g: serialize concurrent phy_set_mode_ext() calls to shared registers Vladimir Oltean
2023-10-06 10:10 ` [PATCH net 0/3] Fixes for lynx-28g PHY driver patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).