Re: [net-next,1/2] net: ti: icssg-prueth-sr1: Remove unused workqueue

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: ti: icssg-prueth-sr1: Remove unused workqueue

This commit removes the dedicated workqueue emac->cmd_wq from
icssg_prueth_sr1.c since it is not utilized in this driver.

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> @@ -783,11 +783,6 @@ static int prueth_netdev_init(struct prueth *prueth,
>  	emac->prueth = prueth;
>  	emac->ndev = ndev;
>  	emac->port_id = port;
> -	emac->cmd_wq = create_singlethread_workqueue("icssg_cmd_wq");
> -	if (!emac->cmd_wq) {
> -		ret = -ENOMEM;
> -		goto free_ndev;
> -	}

[ ... ]

> @@ -883,8 +878,6 @@
>
>  free:
>  	pruss_release_mem_region(prueth->pruss, &emac->dram);
> -free_wq:
> -	destroy_workqueue(emac->cmd_wq);
>  free_ndev:
>  	emac->ndev = NULL;
>  	prueth->emac[mac] = NULL;

Removing the workqueue creation here is fine for the SR1 driver's own error
path, but doesn't the shared cleanup function prueth_netdev_exit() in
icssg_common.c still call destroy_workqueue(emac->cmd_wq) unconditionally?

void prueth_netdev_exit(struct prueth *prueth, ...)
{
    ...
    pruss_release_mem_region(prueth->pruss, &emac->dram);
    destroy_workqueue(emac->cmd_wq);
    free_netdev(emac->ndev);
    ...
}

Since emac is zero-initialized from alloc_netdev_mqs(), emac->cmd_wq will be
NULL after this patch. Can this cause a NULL pointer dereference in
destroy_workqueue() on module unload, since destroy_workqueue() does not
check for NULL?
-- 
pw-bot: cr