[PATCH net-next 0/2] net: ti: icssg: Remove dedicated workqueue for ndo_set_rx_mode callback

[PATCH net-next 0/2] net: ti: icssg: Remove dedicated workqueue for ndo_set_rx_mode callback

[Kevin Hao]

While addressing an issue in the cpsw driver, I based my fix on the
implementation of the emac_ndo_set_rx_mode() function in icssg. During review,
Jakub pointed out that the dedicated workqueue for the ndo_set_rx_mode
callback might be unnecessary. I subsequently used the system workqueue for
this purpose in the cpsw driver, and it performed as expected. Therefore,
I believe the same approach can be applied to the icssg driver. These two
patches aim to remove this dedicated workqueue for icssg drivers.

Please note: I do not have access to a board that supports the CSSG PRU
Ethernet controller, so this change has only been build-tested.

---
Cc: MD Danish Anwar <danishanwar@ti.com>
Cc: Roger Quadros <rogerq@kernel.org>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Meghana Malladi <m-malladi@ti.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Cc: Himanshu Mittal <h-mittal1@ti.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Byungchul Park <byungchul@sk.com>
Cc: Markus Elfring <elfring@users.sourceforge.net>

---
Kevin Hao (2):
      net: ti: icssg-prueth-sr1: Remove unused workqueue
      net: ti: icssg-prueth: Use system default workqueue in ndo_set_rx_mode callback

 drivers/net/ethernet/ti/icssg/icssg_common.c     |  1 -
 drivers/net/ethernet/ti/icssg/icssg_prueth.c     | 13 ++++---------
 drivers/net/ethernet/ti/icssg/icssg_prueth.h     |  1 -
 drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c |  9 +--------
 4 files changed, 5 insertions(+), 19 deletions(-)
---
base-commit: 5c009020744fe129e4728e71c44a6c7816c9105e
change-id: 20260203-icssg-prueth-workqueue-f460eba72132

Best regards,
-- 
Kevin Hao <haokexin@gmail.com>

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

[Kevin Hao]

The dedicated workqueue emac->cmd_wq is not utilized in this driver and
can be safely removed.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
index 7bb4f0d850cc72d997e5ead38cf56f1dbc2355a1..b8115ca47082f9676067bb65c6eb3a441290fd7c 100644
--- 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;
-	}
 
 	INIT_DELAYED_WORK(&emac->stats_work, icssg_stats_work_handler);
 
@@ -798,7 +793,7 @@ static int prueth_netdev_init(struct prueth *prueth,
 	if (ret) {
 		dev_err(prueth->dev, "unable to get DRAM: %d\n", ret);
 		ret = -ENOMEM;
-		goto free_wq;
+		goto free_ndev;
 	}
 
 	/* SR1.0 uses a dedicated high priority channel
@@ -883,8 +878,6 @@ static int prueth_netdev_init(struct prueth *prueth,
 
 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;

-- 
2.52.0

[PATCH net-next 2/2] net: ti: icssg-prueth: Use system default workqueue in ndo_set_rx_mode callback

[Kevin Hao]

Currently, the actual processing of ndo_set_rx_mode is executed in a work.
However, there is no need to create a dedicated workqueue for such a
simple use case. To simplify the code, switch to using the system default
workqueue instead.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/net/ethernet/ti/icssg/icssg_common.c |  1 -
 drivers/net/ethernet/ti/icssg/icssg_prueth.c | 13 ++++---------
 drivers/net/ethernet/ti/icssg/icssg_prueth.h |  1 -
 3 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 090aa74d3ce7257ae52158b2ff9a2a1239bfcfc8..0cf9dfe0fa362b4ff986b6815952f95b79cb476d 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -1720,7 +1720,6 @@ void prueth_netdev_exit(struct prueth *prueth,
 	netif_napi_del(&emac->napi_rx);
 
 	pruss_release_mem_region(prueth->pruss, &emac->dram);
-	destroy_workqueue(emac->cmd_wq);
 	free_netdev(emac->ndev);
 	prueth->emac[mac] = NULL;
 }
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index f65041662173c1968f7e70dc4a3d897523f24095..0939994c932f5322eebbaa1b2fe43e20d3dca5ae 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -1099,7 +1099,7 @@ static void emac_ndo_set_rx_mode(struct net_device *ndev)
 {
 	struct prueth_emac *emac = netdev_priv(ndev);
 
-	queue_work(emac->cmd_wq, &emac->rx_mode_work);
+	schedule_work(&emac->rx_mode_work);
 }
 
 static netdev_features_t emac_ndo_fix_features(struct net_device *ndev,
@@ -1451,11 +1451,6 @@ static int prueth_netdev_init(struct prueth *prueth,
 	emac->port_id = port;
 	emac->xdp_prog = NULL;
 	emac->ndev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
-	emac->cmd_wq = create_singlethread_workqueue("icssg_cmd_wq");
-	if (!emac->cmd_wq) {
-		ret = -ENOMEM;
-		goto free_ndev;
-	}
 	INIT_WORK(&emac->rx_mode_work, emac_ndo_set_rx_mode_work);
 
 	INIT_DELAYED_WORK(&emac->stats_work, icssg_stats_work_handler);
@@ -1467,7 +1462,7 @@ static int prueth_netdev_init(struct prueth *prueth,
 	if (ret) {
 		dev_err(prueth->dev, "unable to get DRAM: %d\n", ret);
 		ret = -ENOMEM;
-		goto free_wq;
+		goto free_ndev;
 	}
 
 	emac->tx_ch_num = 1;
@@ -1566,8 +1561,6 @@ static int prueth_netdev_init(struct prueth *prueth,
 
 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;
@@ -2236,6 +2229,7 @@ static int prueth_probe(struct platform_device *pdev)
 			prueth->emac[i]->ndev->phydev = NULL;
 		}
 		unregister_netdev(prueth->registered_netdevs[i]);
+		disable_work_sync(&prueth->emac[i]->rx_mode_work);
 	}
 
 netdev_exit:
@@ -2295,6 +2289,7 @@ static void prueth_remove(struct platform_device *pdev)
 		phy_disconnect(prueth->emac[i]->ndev->phydev);
 		prueth->emac[i]->ndev->phydev = NULL;
 		unregister_netdev(prueth->registered_netdevs[i]);
+		disable_work_sync(&prueth->emac[i]->rx_mode_work);
 	}
 
 	for (i = 0; i < PRUETH_NUM_MACS; i++) {
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 10eadd35665045a98a0c0a639606b7c2691195de..3d94fa5a7ac10992d35fdb63458f8cb6eb3a5a22 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -236,7 +236,6 @@ struct prueth_emac {
 	/* Mutex to serialize access to firmware command interface */
 	struct mutex cmd_lock;
 	struct work_struct rx_mode_work;
-	struct workqueue_struct	*cmd_wq;
 
 	struct pruss_mem_region dram;
 

-- 
2.52.0

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

[Markus Elfring]

> The dedicated workqueue emac->cmd_wq is not utilized in this driver and
> can be safely removed.

Wording suggestion:
Remove the work queue “emac->cmd_wq” because it is not utilized in this driver.

Regards,
Markus

Re: [PATCH net-next 2/2] net: ti: icssg-prueth: Use system default workqueue in ndo_set_rx_mode callback

[Markus Elfring]

> Currently, the actual processing of ndo_set_rx_mode is executed in a work.

                                                                            queue?


> However, there is no need to create a dedicated workqueue for …


Regards,
Markus

Re: [EXTERNAL] [PATCH net-next 0/2] net: ti: icssg: Remove dedicated workqueue for ndo_set_rx_mode callback

[Meghana Malladi]

Hi Kevin,

On 2/4/26 08:11, Kevin Hao wrote:
> While addressing an issue in the cpsw driver, I based my fix on the 
> implementation of the emac_ndo_set_rx_mode() function in icssg. During 
> review, Jakub pointed out that the dedicated workqueue for the 
> ndo_set_rx_mode callback might be unnecessary. 
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> uxdqXRfPtm1bKoXPFrvrtsCD2dIkXHVJwEzupcwg1k-qY0e_2PUuMdZgJf- 
> SUVShc4kuXcBnqQbeqGz1dHBW-_OSdROPAjSiNpSTmAhDg8nQbc8$>
> ZjQcmQRYFpfptBannerEnd
> 
> While addressing an issue in the cpsw driver, I based my fix on the
> implementation of the emac_ndo_set_rx_mode() function in icssg. During review,
> Jakub pointed out that the dedicated workqueue for the ndo_set_rx_mode
> callback might be unnecessary. I subsequently used the system workqueue for
> this purpose in the cpsw driver, and it performed as expected. Therefore,
> I believe the same approach can be applied to the icssg driver. These two
> patches aim to remove this dedicated workqueue for icssg drivers.
> 

It would be more helpful, if you could provide link to the cpsw fix 
which you have mentioned above.

> Please note: I do not have access to a board that supports the CSSG PRU
> Ethernet controller, so this change has only been build-tested.
> 
> ---
> Cc: MD Danish Anwar <danishanwar@ti.com>
> Cc: Roger Quadros <rogerq@kernel.org>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Meghana Malladi <m-malladi@ti.com>
> Cc: Jacob Keller <jacob.e.keller@intel.com>
> Cc: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> Cc: Himanshu Mittal <h-mittal1@ti.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Byungchul Park <byungchul@sk.com>
> Cc: Markus Elfring <elfring@users.sourceforge.net>
> 
> ---
> Kevin Hao (2):
>        net: ti: icssg-prueth-sr1: Remove unused workqueue
>        net: ti: icssg-prueth: Use system default workqueue in ndo_set_rx_mode callback
> 
>   drivers/net/ethernet/ti/icssg/icssg_common.c     |  1 -
>   drivers/net/ethernet/ti/icssg/icssg_prueth.c     | 13 ++++---------
>   drivers/net/ethernet/ti/icssg/icssg_prueth.h     |  1 -
>   drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c |  9 +--------
>   4 files changed, 5 insertions(+), 19 deletions(-)
> ---
> base-commit: 5c009020744fe129e4728e71c44a6c7816c9105e
> change-id: 20260203-icssg-prueth-workqueue-f460eba72132
> 
> Best regards,
> -- 
> Kevin Hao <haokexin@gmail.com>
> 

Thanks,
Meghana

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

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

On Wed, Feb 04, 2026 at 11:38:37AM +0100, Markus Elfring wrote:
> > The dedicated workqueue emac->cmd_wq is not utilized in this driver and
> > can be safely removed.
> 
> Wording suggestion:
> Remove the work queue “emac->cmd_wq” because it is not utilized in this driver.

Will update.

Thanks,
Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH net-next 2/2] net: ti: icssg-prueth: Use system default workqueue in ndo_set_rx_mode callback

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]

On Wed, Feb 04, 2026 at 11:45:24AM +0100, Markus Elfring wrote:
> > Currently, the actual processing of ndo_set_rx_mode is executed in a work.
> 
>                                                                             queue?

Will change to workqueue.

Thanks,
Kevin

> 
> 
> > However, there is no need to create a dedicated workqueue for …
> 
> 
> Regards,
> Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [EXTERNAL] [PATCH net-next 0/2] net: ti: icssg: Remove dedicated workqueue for ndo_set_rx_mode callback

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 3419 bytes --]

On Wed, Feb 04, 2026 at 04:44:21PM +0530, Meghana Malladi wrote:
> Hi Kevin,
> 
> On 2/4/26 08:11, Kevin Hao wrote:
> > While addressing an issue in the cpsw driver, I based my fix on the
> > implementation of the emac_ndo_set_rx_mode() function in icssg. During
> > review, Jakub pointed out that the dedicated workqueue for the
> > ndo_set_rx_mode callback might be unnecessary. 
> > ZjQcmQRYFpfptBannerStart
> > This message was sent from outside of Texas Instruments.
> > Do not click links or open attachments unless you recognize the source
> > of this email and know the content is safe.
> > Report Suspicious
> > <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK!
> > uxdqXRfPtm1bKoXPFrvrtsCD2dIkXHVJwEzupcwg1k-qY0e_2PUuMdZgJf-
> > SUVShc4kuXcBnqQbeqGz1dHBW-_OSdROPAjSiNpSTmAhDg8nQbc8$>
> > ZjQcmQRYFpfptBannerEnd
> > 
> > While addressing an issue in the cpsw driver, I based my fix on the
> > implementation of the emac_ndo_set_rx_mode() function in icssg. During review,
> > Jakub pointed out that the dedicated workqueue for the ndo_set_rx_mode
> > callback might be unnecessary. I subsequently used the system workqueue for
> > this purpose in the cpsw driver, and it performed as expected. Therefore,
> > I believe the same approach can be applied to the icssg driver. These two
> > patches aim to remove this dedicated workqueue for icssg drivers.
> > 
> 
> It would be more helpful, if you could provide link to the cpsw fix which
> you have mentioned above.

The fix for cpsw is available at:
  https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=c0b5dc73a38f954e780f93a549b8fe225235c07a

Jakub's suggestion can be found here:
  https://lore.kernel.org/all/20260127190836.6a420768@kernel.org/

I will also include this information in the v2 cover letter.

Thanks,
Kevin

> 
> > Please note: I do not have access to a board that supports the CSSG PRU
> > Ethernet controller, so this change has only been build-tested.
> > 
> > ---
> > Cc: MD Danish Anwar <danishanwar@ti.com>
> > Cc: Roger Quadros <rogerq@kernel.org>
> > Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Paolo Abeni <pabeni@redhat.com>
> > Cc: Meghana Malladi <m-malladi@ti.com>
> > Cc: Jacob Keller <jacob.e.keller@intel.com>
> > Cc: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> > Cc: Himanshu Mittal <h-mittal1@ti.com>
> > Cc: Simon Horman <horms@kernel.org>
> > Cc: Byungchul Park <byungchul@sk.com>
> > Cc: Markus Elfring <elfring@users.sourceforge.net>
> > 
> > ---
> > Kevin Hao (2):
> >        net: ti: icssg-prueth-sr1: Remove unused workqueue
> >        net: ti: icssg-prueth: Use system default workqueue in ndo_set_rx_mode callback
> > 
> >   drivers/net/ethernet/ti/icssg/icssg_common.c     |  1 -
> >   drivers/net/ethernet/ti/icssg/icssg_prueth.c     | 13 ++++---------
> >   drivers/net/ethernet/ti/icssg/icssg_prueth.h     |  1 -
> >   drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c |  9 +--------
> >   4 files changed, 5 insertions(+), 19 deletions(-)
> > ---
> > base-commit: 5c009020744fe129e4728e71c44a6c7816c9105e
> > change-id: 20260203-icssg-prueth-workqueue-f460eba72132
> > 
> > Best regards,
> > -- 
> > Kevin Hao <haokexin@gmail.com>
> > 
> 
> Thanks,
> Meghana

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Jakub Kicinski]

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

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

[Jakub Kicinski]

On Wed,  4 Feb 2026 20:17:02 -0800 Jakub Kicinski wrote:
> > @@ -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?

Maybe it's not worth separating the removal into two patches if there
are surprising dependencies here? Squash them into one for v2 perhaps?

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

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 250 bytes --]

On Wed, Feb 04, 2026 at 08:19:14PM -0800, Jakub Kicinski wrote:
> Maybe it's not worth separating the removal into two patches if there
> are surprising dependencies here?

Definitely.

> Squash them into one for v2 perhaps?

Will do.

Thanks,
Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]