[PATCH net-next v5 3/6] net: xilinx: axienet: Propagate device reset failure from ndo_open

[Suraj Gupta <suraj.gupta2@amd.com>]

axienet_open() overwrites the axienet_device_reset() return value with the
phylink_fwnode_phy_connect() one before it is ever checked, so a failed
reset brings the device up anyway. Check and propagate it.

That makes a failed ndo_open reachable, which matters for axienet_resume():
it calls ndo_open directly, so unlike the netdev core it leaves the device
running when open fails. A later ndo_stop then tears down resources that
open had already unwound and never re-created. For the dmaengine flow that
is a double free of the skb rings and a second dma_release_channel(); for
the non-dmaengine flow it is napi_disable() on an already disabled NAPI,
which never returns, and free_irq() on interrupts that were never
requested.

Track whether ndo_open completed and make axienet_stop() a no-op otherwise.

axienet_resume() now propagates the error, but it still has to undo the
netif_device_detach() that axienet_suspend() did. Leaving the device
detached while it is still IFF_UP wedges the interface: ndo_stop is a
no-op, so bringing it down appears to work, while bringing it up again
fails with -ENODEV because the device is not present. Drop IFF_UP with
netif_close() before reattaching, so the interface can be brought up again
once the cause of the failure is resolved, and so the stack cannot
transmit on the datapath open has already torn down.

Clearing the error path of __axienet_device_reset() goes with this: it
jumps over the block that clears lp->reset_in_progress, and the statistics
work that used to clear it on the next run is no longer scheduled once the
reset error is propagated. Without this, axienet_stat() would keep
returning the frozen base after a reset timeout.

Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet.h  |  2 +
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 44 +++++++++++++++++--
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h
index a87cfa079502..140383749996 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
@@ -526,6 +526,7 @@ struct skbuf_dma_descriptor {
  * @stopping:   Set when we are about to stop the device: makes @dma_err_task
  *              a no-op (legacy DMA path) and fences RX descriptor
  *              resubmission in axienet_dma_rx_cb() (dmaengine path).
+ * @datapath_up: Set while the resources allocated by ndo_open are live.
  * @tx_irq:	Axidma TX IRQ number
  * @rx_irq:	Axidma RX IRQ number
  * @eth_irq:	Ethernet core IRQ number
@@ -608,6 +609,7 @@ struct axienet_local {
 
 	struct work_struct dma_err_task;
 	bool stopping;
+	bool datapath_up;
 
 	int tx_irq;
 	int rx_irq;
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 83f35effbf37..f338029cf870 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -702,6 +702,17 @@ static int __axienet_device_reset(struct axienet_local *lp)
 	}
 
 out:
+	/* On failure the block above is skipped, so clear the flag here.
+	 * axienet_open() now returns on a reset error without scheduling the
+	 * statistics work that used to clear it, which would otherwise leave
+	 * axienet_stat() returning the frozen base until a reset succeeds.
+	 */
+	if (ret && (lp->features & XAE_FEATURE_STATS)) {
+		write_seqcount_begin(&lp->hw_stats_seqcount);
+		lp->reset_in_progress = false;
+		write_seqcount_end(&lp->hw_stats_seqcount);
+	}
+
 	mutex_unlock(&lp->stats_lock);
 	return ret;
 }
@@ -1772,6 +1783,10 @@ static int axienet_open(struct net_device *ndev)
 	axienet_lock_mii(lp);
 	ret = axienet_device_reset(ndev);
 	axienet_unlock_mii(lp);
+	if (ret) {
+		dev_err(lp->dev, "axienet_device_reset() failed: %d\n", ret);
+		return ret;
+	}
 
 	ret = phylink_fwnode_phy_connect(lp->phylink, dev_fwnode(lp->dev), 0);
 	if (ret) {
@@ -1802,6 +1817,8 @@ static int axienet_open(struct net_device *ndev)
 			goto err_phy;
 	}
 
+	lp->datapath_up = true;
+
 	return 0;
 
 err_free_eth_irq:
@@ -1830,6 +1847,16 @@ static int axienet_stop(struct net_device *ndev)
 	struct axienet_local *lp = netdev_priv(ndev);
 	int i;
 
+	/* axienet_resume() calls ndo_open directly, so unlike the netdev core
+	 * it can leave the device running after a failed open. Tearing down
+	 * below would then stop a phylink that open already stopped and
+	 * disconnected, and free interrupts it never requested.
+	 */
+	if (!lp->datapath_up)
+		return 0;
+
+	lp->datapath_up = false;
+
 	if (!lp->use_dmaengine) {
 		WRITE_ONCE(lp->stopping, true);
 		flush_work(&lp->dma_err_task);
@@ -3415,17 +3442,26 @@ static int axienet_suspend(struct device *dev)
 static int axienet_resume(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);
+	int ret;
 
 	if (!netif_running(ndev))
 		return 0;
 
 	rtnl_lock();
-	axienet_open(ndev);
-	rtnl_unlock();
-
+	ret = axienet_open(ndev);
+	if (ret) {
+		netdev_err(ndev, "failed to reopen device on resume: %d\n", ret);
+		/* Drop IFF_UP before reattaching, so that the interface can be
+		 * brought up again once whatever made open fail is resolved.
+		 * Reattaching while still up would also let the stack transmit
+		 * on the datapath open has already torn down.
+		 */
+		netif_close(ndev);
+	}
 	netif_device_attach(ndev);
+	rtnl_unlock();
 
-	return 0;
+	return ret;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(axienet_pm_ops,
-- 
2.25.1