102 lines
3.5 KiB
Diff
102 lines
3.5 KiB
Diff
From 06efc9b8a1360cad83cae6e71558e5458cc1fbf3 Mon Sep 17 00:00:00 2001
|
|
From: Vladimir Oltean <vladimir.oltean@nxp.com>
|
|
Date: Tue, 29 Nov 2022 16:12:14 +0200
|
|
Subject: [PATCH 07/14] net: dpaa2-eth: assign priv->mac after
|
|
dpaa2_mac_connect() call
|
|
|
|
There are 2 requirements for correct code:
|
|
|
|
- Any time the driver accesses the priv->mac pointer at runtime, it
|
|
either holds NULL to indicate a DPNI-DPNI connection (or unconnected
|
|
DPNI), or a struct dpaa2_mac whose phylink instance was fully
|
|
initialized (created and connected to the PHY). No changes are made to
|
|
priv->mac while it is being used. Currently, rtnl_lock() watches over
|
|
the call to dpaa2_eth_connect_mac(), so it serves the purpose of
|
|
serializing this with all readers of priv->mac.
|
|
|
|
- dpaa2_mac_connect() should run unlocked, because inside it are 2
|
|
phylink calls with incompatible locking requirements: phylink_create()
|
|
requires that the rtnl_mutex isn't held, and phylink_fwnode_phy_connect()
|
|
requires that the rtnl_mutex is held. The only way to solve those
|
|
contradictory requirements is to let dpaa2_mac_connect() take
|
|
rtnl_lock() when it needs to.
|
|
|
|
To solve both requirements, we need to identify the writer side of the
|
|
priv->mac pointer, which can be wrapped in a mutex private to the driver
|
|
in a future patch. The dpaa2_mac_connect() cannot be part of the writer
|
|
side critical section, because of an AB/BA deadlock with rtnl_lock().
|
|
|
|
So the strategy needs to be that where we prepare the DPMAC by calling
|
|
dpaa2_mac_connect(), and only make priv->mac point to it once it's fully
|
|
prepared. This ensures that the writer side critical section has the
|
|
absolute minimum surface it can.
|
|
|
|
The reverse strategy is adopted in the dpaa2_eth_disconnect_mac() code
|
|
path. This makes sure that priv->mac is NULL when we start tearing down
|
|
the DPMAC that we disconnected from, and concurrent code will simply not
|
|
see it.
|
|
|
|
No locking changes in this patch (concurrent code is still blocked by
|
|
the rtnl_mutex).
|
|
|
|
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
|
|
Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com>
|
|
Tested-by: Ioana Ciornei <ioana.ciornei@nxp.com>
|
|
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
---
|
|
.../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 21 +++++++++++--------
|
|
1 file changed, 12 insertions(+), 9 deletions(-)
|
|
|
|
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
|
|
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
|
|
@@ -4441,9 +4441,8 @@ static int dpaa2_eth_connect_mac(struct
|
|
err = dpaa2_mac_open(mac);
|
|
if (err)
|
|
goto err_free_mac;
|
|
- priv->mac = mac;
|
|
|
|
- if (dpaa2_eth_is_type_phy(priv)) {
|
|
+ if (dpaa2_mac_is_type_phy(mac)) {
|
|
err = dpaa2_mac_connect(mac);
|
|
if (err && err != -EPROBE_DEFER)
|
|
netdev_err(priv->net_dev, "Error connecting to the MAC endpoint: %pe",
|
|
@@ -4452,11 +4451,12 @@ static int dpaa2_eth_connect_mac(struct
|
|
goto err_close_mac;
|
|
}
|
|
|
|
+ priv->mac = mac;
|
|
+
|
|
return 0;
|
|
|
|
err_close_mac:
|
|
dpaa2_mac_close(mac);
|
|
- priv->mac = NULL;
|
|
err_free_mac:
|
|
kfree(mac);
|
|
return err;
|
|
@@ -4464,15 +4464,18 @@ err_free_mac:
|
|
|
|
static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv)
|
|
{
|
|
- if (dpaa2_eth_is_type_phy(priv))
|
|
- dpaa2_mac_disconnect(priv->mac);
|
|
+ struct dpaa2_mac *mac = priv->mac;
|
|
+
|
|
+ priv->mac = NULL;
|
|
|
|
- if (!dpaa2_eth_has_mac(priv))
|
|
+ if (!mac)
|
|
return;
|
|
|
|
- dpaa2_mac_close(priv->mac);
|
|
- kfree(priv->mac);
|
|
- priv->mac = NULL;
|
|
+ if (dpaa2_mac_is_type_phy(mac))
|
|
+ dpaa2_mac_disconnect(mac);
|
|
+
|
|
+ dpaa2_mac_close(mac);
|
|
+ kfree(mac);
|
|
}
|
|
|
|
static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg)
|