RE: [PATCH 02/14] net: axienet: Propagate failure of DMA descriptor setup

-----Original Message-----
From: Radhey Shyam Pandey
Sent: Friday, January 10, 2020 8:25 PM
To: Andre Przywara <andre.przywara@arm.com>; David S . Miller
[off-list ref]
Cc: Michal Simek <redacted>; Robert Hancock
[off-list ref]; netdev@vger.kernel.org; linux-arm-
kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Subject: RE: [PATCH 02/14] net: axienet: Propagate failure of DMA descriptor
setup

quoted

-----Original Message-----
From: Andre Przywara <andre.przywara@arm.com>
Sent: Friday, January 10, 2020 5:24 PM
To: David S . Miller <davem@davemloft.net>; Radhey Shyam Pandey
[off-list ref]
Cc: Michal Simek <redacted>; Robert Hancock
[off-list ref]; netdev@vger.kernel.org; linux-arm-
kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Subject: [PATCH 02/14] net: axienet: Propagate failure of DMA
descriptor setup

When we fail allocating the DMA buffers in axienet_dma_bd_init(), we
report this error, but carry on with initialisation nevertheless.

This leads to a kernel panic when the driver later wants to send a
packet, as it uses uninitialised data structures.

Make the axienet_device_reset() routine return an error value, as it
contains the DMA buffer initialisation. Make sure we propagate the
error up the chain and eventually fail the driver initialisation, to
avoid relying on non-initialised buffers.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Radhey Shyam Pandey <redacted>

quoted

---
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 25
+++++++++++++------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c

b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 20746b801959..97482cf093ce 100644

--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c

@@ -437,9 +437,10 @@ static void axienet_setoptions(struct net_device

*ndev, u32 options)
 	lp->options |= options;
 }

-static void __axienet_device_reset(struct axienet_local *lp)
+static int __axienet_device_reset(struct axienet_local *lp)
 {
 	u32 timeout;
+
 	/* Reset Axi DMA. This would reset Axi Ethernet core as well. The
reset
 	 * process of Axi DMA takes a while to complete as all pending
 	 * commands/transfers will be flushed or completed during this @@
-455,9 +456,11 @@ static void __axienet_device_reset(struct
axienet_local *lp)
 		if (--timeout == 0) {
 			netdev_err(lp->ndev, "%s: DMA reset timeout!\n",
 				   __func__);
-			break;
+			return -ETIMEDOUT;
 		}
 	}
+
+	return 0;
 }

 /**

@@ -471,12 +474,15 @@ static void __axienet_device_reset(struct

axienet_local *lp)
  * Ethernet core. No separate hardware reset is done for the Axi Ethernet
  * core.
  */

Minor nit- we need to add descripton for return value.
drivers/net/ethernet/xilinx/xilinx_axienet_main.c:491: warning: 
No description found for return value of 'axienet_device_reset'

quoted

-static void axienet_device_reset(struct net_device *ndev)
+static int axienet_device_reset(struct net_device *ndev)
 {
 	u32 axienet_status;
 	struct axienet_local *lp = netdev_priv(ndev);
+	int ret;

-	__axienet_device_reset(lp);
+	ret = __axienet_device_reset(lp);
+	if (ret)
+		return ret;

 	lp->max_frm_size = XAE_MAX_VLAN_FRAME_SIZE;
 	lp->options |= XAE_OPTION_VLAN;

@@ -491,9 +497,11 @@ static void axienet_device_reset(struct

net_device
*ndev)
 			lp->options |= XAE_OPTION_JUMBO;
 	}

-	if (axienet_dma_bd_init(ndev)) {
+	ret = axienet_dma_bd_init(ndev);
+	if (ret) {
 		netdev_err(ndev, "%s: descriptor allocation failed\n",
 			   __func__);
+		return ret;
 	}

 	axienet_status = axienet_ior(lp, XAE_RCW1_OFFSET); @@ -518,6

+526,8

quoted

@@ static void axienet_device_reset(struct net_device

*ndev)
 	axienet_setoptions(ndev, lp->options);

 	netif_trans_update(ndev);
+
+	return 0;
 }

 /**

@@ -921,8 +931,9 @@ static int axienet_open(struct net_device *ndev)
 	 */
 	mutex_lock(&lp->mii_bus->mdio_lock);
 	axienet_mdio_disable(lp);
-	axienet_device_reset(ndev);
-	ret = axienet_mdio_enable(lp);
+	ret = axienet_device_reset(ndev);
+	if (ret == 0)
+		ret = axienet_mdio_enable(lp);
 	mutex_unlock(&lp->mii_bus->mdio_lock);
 	if (ret < 0)
 		return ret;
--

2.17.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel