[PATCH v2 0/4] staging: octeon: Improve initialization error handling

[PATCH v2 0/4] staging: octeon: Improve initialization error handling

[Prashant Rahul]

Hi all,

The TX and RX initialization paths currently call panic() on some
initialization failures. Instead, these failures should be reported back
to the probe function so that it can unwind any resources acquired during
initialization and fail gracefully.

This series replaces those panic() calls with proper error logging, propagates
the errors to the probe function, and adds the necessary cleanup to avoid
leaving the driver in a partially initialized state. I have not tested
these changes as I do not own this hardware.

This is my first time working with drivers, so please let me know if I
have made any mistakes.

Thank you! :3

Signed-off-by: Prashant Rahul <prashantrahul23@gmail.com>
---
Changes in v2:
- ethernet.c: remove unnecessary curly braces around single line for-loop
- ethernet.c: dont mark cvm_oct_remove_device as inline
- ethernet.c: remove unnecessary call to cvmx_pko_disable as cvmx_pko_shutdown
already does that.
- cover: specify no testing

- Link to v1: https://lore.kernel.org/r/20260628-staging-driver-octeon-panic-v1-0-fe8c34989254@gmail.com

---
Prashant Rahul (4):
      staging: octeon: factor out device removal into a helper
      staging: octeon: Propagate rx initialization failures
      staging: octeon: Propagate tx initialization failures
      staging: octeon: handle rx/tx initialization failures in probe

 drivers/staging/octeon/ethernet-rx.c | 18 ++++++----
 drivers/staging/octeon/ethernet-rx.h |  2 +-
 drivers/staging/octeon/ethernet-tx.c | 17 +++++-----
 drivers/staging/octeon/ethernet-tx.h |  2 +-
 drivers/staging/octeon/ethernet.c    | 66 ++++++++++++++++++++++++++++--------
 5 files changed, 74 insertions(+), 31 deletions(-)
---
base-commit: 4e5dfb7c84012007c3c7061126491bbc92d71bf1
change-id: 20260625-staging-driver-octeon-panic-2602ed8d5d67

Best regards,
-- 
Prashant Rahul <prashantrahul23@gmail.com>

[PATCH v2 1/4] staging: octeon: factor out device removal into a helper

[Prashant Rahul]

Factor out the common device removal sequence into a helper in
preparation for adding cleanup to the probe error path.

Signed-off-by: Prashant Rahul <prashantrahul23@gmail.com>
---
 drivers/staging/octeon/ethernet.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index f3fa221f452e2..21f3e27aabf2c 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -104,6 +104,21 @@ struct net_device *cvm_oct_device[TOTAL_NUMBER_OF_PORTS];
 
 u64 cvm_oct_tx_poll_interval;
 
+static void cvm_oct_remove_device(int port)
+{
+	if (cvm_oct_device[port]) {
+		struct net_device *dev = cvm_oct_device[port];
+		struct octeon_ethernet *priv = netdev_priv(dev);
+
+		cancel_delayed_work_sync(&priv->port_periodic_work);
+
+		cvm_oct_tx_shutdown_dev(dev);
+		unregister_netdev(dev);
+		free_netdev(dev);
+		cvm_oct_device[port] = NULL;
+	}
+}
+
 static void cvm_oct_rx_refill_worker(struct work_struct *work)
 {
 	struct octeon_ethernet_platform *plat = container_of(work,
@@ -948,19 +963,8 @@ static void cvm_oct_remove(struct platform_device *pdev)
 	cvmx_pko_disable();
 
 	/* Free the ethernet devices */
-	for (port = 0; port < TOTAL_NUMBER_OF_PORTS; port++) {
-		if (cvm_oct_device[port]) {
-			struct net_device *dev = cvm_oct_device[port];
-			struct octeon_ethernet *priv = netdev_priv(dev);
-
-			cancel_delayed_work_sync(&priv->port_periodic_work);
-
-			cvm_oct_tx_shutdown_dev(dev);
-			unregister_netdev(dev);
-			free_netdev(dev);
-			cvm_oct_device[port] = NULL;
-		}
-	}
+	for (port = 0; port < TOTAL_NUMBER_OF_PORTS; port++)
+		cvm_oct_remove_device(port);
 
 	cvmx_pko_shutdown();
 

-- 
2.54.0

[PATCH v2 2/4] staging: octeon: Propagate rx initialization failures

[Prashant Rahul]

Instead of calling panic(), log and propagate the error upwards for
better error handling.

Signed-off-by: Prashant Rahul <prashantrahul23@gmail.com>
---
 drivers/staging/octeon/ethernet-rx.c | 18 ++++++++++++------
 drivers/staging/octeon/ethernet-rx.h |  2 +-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index cd36b5ba6f6c2..66e7253d484de 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -5,6 +5,8 @@
  * Copyright (c) 2003-2010 Cavium Networks
  */
 
+#include <asm-generic/errno-base.h>
+#include <linux/dev_printk.h>
 #include <linux/platform_device.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -445,22 +447,24 @@ void cvm_oct_poll_controller(struct net_device *dev)
 }
 #endif
 
-void cvm_oct_rx_initialize(struct platform_device *pdev)
+int cvm_oct_rx_initialize(struct platform_device *pdev)
 {
 	int i;
 	struct net_device *dev_for_napi = NULL;
 	struct octeon_ethernet_platform *plat = platform_get_drvdata(pdev);
 	struct oct_rx_group *rx_group = plat->rx_group;
 
 	for (i = 0; i < TOTAL_NUMBER_OF_PORTS; i++) {
 		if (cvm_oct_device[i]) {
 			dev_for_napi = cvm_oct_device[i];
 			break;
 		}
 	}
 
-	if (!dev_for_napi)
-		panic("No net_devices were allocated.");
+	if (!dev_for_napi) {
+		dev_err(&pdev->dev, "No net_devices were allocated.");
+		return -ENODEV;
+	}
 
 	for (i = 0; i < ARRAY_SIZE(plat->rx_group); i++) {
 		int ret;
@@ -479,9 +483,10 @@ void cvm_oct_rx_initialize(struct platform_device *pdev)
 		/* Register an IRQ handler to receive POW interrupts */
 		ret = request_irq(rx_group[i].irq, cvm_oct_do_interrupt, 0,
 				  "Ethernet", &rx_group[i].napi);
-		if (ret)
-			panic("Could not acquire Ethernet IRQ %d\n",
-			      rx_group[i].irq);
+		if (ret) {
+			dev_err(&pdev->dev, "Could not acquire Ethernet IRQ %d\n", rx_group[i].irq);
+			return ret;
+		}
 
 		disable_irq_nosync(rx_group[i].irq);
 
@@ -518,6 +523,7 @@ void cvm_oct_rx_initialize(struct platform_device *pdev)
 		napi_schedule(&rx_group[i].napi);
 	}
 	atomic_inc(&oct_rx_ready);
+	return 0;
 }
 
 void cvm_oct_rx_shutdown(struct platform_device *pdev)
diff --git a/drivers/staging/octeon/ethernet-rx.h b/drivers/staging/octeon/ethernet-rx.h
index 6093694326cb6..ff3ed3d784a4b 100644
--- a/drivers/staging/octeon/ethernet-rx.h
+++ b/drivers/staging/octeon/ethernet-rx.h
@@ -8,7 +8,7 @@
 struct platform_device;
 
 void cvm_oct_poll_controller(struct net_device *dev);
-void cvm_oct_rx_initialize(struct platform_device *pdev);
+int cvm_oct_rx_initialize(struct platform_device *pdev);
 void cvm_oct_rx_shutdown(struct platform_device *pdev);
 
 static inline void cvm_oct_rx_refill_pool(struct platform_device *pdev,

-- 
2.54.0

[PATCH v2 3/4] staging: octeon: Propagate tx initialization failures

[Prashant Rahul]

Instead of calling panic(), log and propagate the error upwards for
better error handling.

Pass platform_device to tx initializer to match the rx
initializer interface and to log device specific failures.

Signed-off-by: Prashant Rahul <prashantrahul23@gmail.com>
---
 drivers/staging/octeon/ethernet-tx.c | 17 +++++++++--------
 drivers/staging/octeon/ethernet-tx.h |  2 +-
 drivers/staging/octeon/ethernet.c    |  2 +-
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-tx.c b/drivers/staging/octeon/ethernet-tx.c
index 14d10659bce71..0f3b1f34ff2a3 100644
--- a/drivers/staging/octeon/ethernet-tx.c
+++ b/drivers/staging/octeon/ethernet-tx.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2010 Cavium Networks
  */
 
+#include <linux/platform_device.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
@@ -649,19 +650,19 @@ static irqreturn_t cvm_oct_tx_cleanup_watchdog(int cpl, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-void cvm_oct_tx_initialize(void)
+int cvm_oct_tx_initialize(struct platform_device *pdev)
 {
-	int i;
+	int ret;
 
 	/* Disable the interrupt.  */
 	cvmx_write_csr(CVMX_CIU_TIMX(1), 0);
 	/* Register an IRQ handler to receive CIU_TIMX(1) interrupts */
-	i = request_irq(OCTEON_IRQ_TIMER1,
-			cvm_oct_tx_cleanup_watchdog, 0,
-			"Ethernet", cvm_oct_device);
-
-	if (i)
-		panic("Could not acquire Ethernet IRQ %d\n", OCTEON_IRQ_TIMER1);
+	ret = request_irq(OCTEON_IRQ_TIMER1, cvm_oct_tx_cleanup_watchdog, 0,
+			  "Ethernet", cvm_oct_device);
+	if (ret)
+		dev_warn(&pdev->dev, "Could not acquire Ethernet IRQ %d\n",
+			 OCTEON_IRQ_TIMER1);
+	return ret;
 }
 
 void cvm_oct_tx_shutdown(void)
diff --git a/drivers/staging/octeon/ethernet-tx.h b/drivers/staging/octeon/ethernet-tx.h
index 6c524668f65a5..b03d56b0d2b65 100644
--- a/drivers/staging/octeon/ethernet-tx.h
+++ b/drivers/staging/octeon/ethernet-tx.h
@@ -9,6 +9,6 @@ netdev_tx_t cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev);
 netdev_tx_t cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev);
 int cvm_oct_transmit_qos(struct net_device *dev, void *work_queue_entry,
 			 int do_free, int qos);
-void cvm_oct_tx_initialize(void);
+int cvm_oct_tx_initialize(struct platform_device *pdev);
 void cvm_oct_tx_shutdown(void);
 void cvm_oct_tx_shutdown_dev(struct net_device *dev);
diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index 21f3e27aabf2c..56ea848187531 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -934,7 +934,7 @@ static int cvm_oct_probe(struct platform_device *pdev)
 		}
 	}
 
-	cvm_oct_tx_initialize();
+	cvm_oct_tx_initialize(pdev);
 	cvm_oct_rx_initialize(pdev);
 
 	/*

-- 
2.54.0

[PATCH v2 4/4] staging: octeon: handle rx/tx initialization failures in probe

[Prashant Rahul]

Check return value of rx/tx initialization functions and abort probing
if either one fails.

Add error handling labels to deallocate resources before returning the
error.

Signed-off-by: Prashant Rahul <prashantrahul23@gmail.com>
---
 drivers/staging/octeon/ethernet.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index 56ea848187531..42ec4b8324a83 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -688,6 +688,7 @@ static int cvm_oct_probe(struct platform_device *pdev)
 {
 	int num_interfaces;
 	int interface;
+	int ret = 0;
 	int fau = FAU_NUM_PACKET_BUFFERS_TO_FREE;
 	int qos;
 	struct device_node *pip;
@@ -934,17 +935,48 @@ static int cvm_oct_probe(struct platform_device *pdev)
 		}
 	}
 
-	cvm_oct_tx_initialize(pdev);
-	cvm_oct_rx_initialize(pdev);
+	ret = cvm_oct_tx_initialize(pdev);
+	if (ret)
+		goto err_tx;
+
+	ret = cvm_oct_rx_initialize(pdev);
+	if (ret)
+		goto err_rx;
 
 	/*
 	 * 150 uS: about 10 1500-byte packets at 1GE.
 	 */
 	cvm_oct_tx_poll_interval = 150 * (octeon_get_clock_rate() / 1000000);
 
 	schedule_delayed_work(&plat->rx_refill_work, HZ);
 
 	return 0;
+
+err_rx:
+	cvm_oct_tx_shutdown();
+err_tx:
+	cvmx_ipd_disable();
+
+	atomic_inc_return(&cvm_oct_poll_queue_stopping);
+
+	/* Free the ethernet devices */
+	for (int port = 0; port < TOTAL_NUMBER_OF_PORTS; port++)
+		cvm_oct_remove_device(port);
+
+	cvmx_pko_shutdown();
+
+	cvmx_ipd_free_ptr();
+
+	/* Free the HW pools */
+	cvm_oct_mem_empty_fpa(pdev, CVMX_FPA_PACKET_POOL, CVMX_FPA_PACKET_POOL_SIZE,
+			      num_packet_buffers);
+	cvm_oct_mem_empty_fpa(pdev, CVMX_FPA_WQE_POOL, CVMX_FPA_WQE_POOL_SIZE,
+			      num_packet_buffers);
+	if (CVMX_FPA_OUTPUT_BUFFER_POOL != CVMX_FPA_PACKET_POOL)
+		cvm_oct_mem_empty_fpa(pdev, CVMX_FPA_OUTPUT_BUFFER_POOL,
+				      CVMX_FPA_OUTPUT_BUFFER_POOL_SIZE, 128);
+
+	return ret;
 }
 
 static void cvm_oct_remove(struct platform_device *pdev)

-- 
2.54.0