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

[Dan Carpenter]

On Sun, Jun 28, 2026 at 12:31:27PM +0530, Prashant Rahul wrote:
> 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@xxxxxxxxx>
> ---
>  drivers/staging/octeon/ethernet.c | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> index 1d3bf3c79598d..41be73b2ded91 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,50 @@ 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);
> +
> +	cvmx_pko_disable();
> +
> +	/* Free the ethernet devices */
> +	for (int port = 0; port < TOTAL_NUMBER_OF_PORTS; port++)
> +		cvm_oct_remove_device(port);
> +
> +	cvmx_pko_shutdown();

This is all copied from the shutdown and it looks basically
reasonable to me.  This is staging code so of course it's a
bit wonky.  cvmx_pko_shutdown() has a built in call to
cvmx_pko_disable() so why are we calling that separately?

Also I feel like probe() should fail if alloc_netdev() fails
or register_netdev(), but that's not introduced by this patch
either.  And I have no way to test this.  Even reviewing MIPS
code is a pain in the behind because I don't have the cross
compiler set up.

Reviewed-by: Dan Carpenter <error27@xxxxxxxxx>

regards,
dan carpenter