[PATCH net-next, v3] net: mana: Implement ndo_tx_timeout and serialize queue resets per port.

[PATCH net-next, v3] net: mana: Implement ndo_tx_timeout and serialize queue resets per port.

[Dipayaan Roy]

Implement .ndo_tx_timeout for MANA so any stalled TX queue can be detected
and a device-controlled port reset for all queues can be scheduled to a
ordered workqueue. The reset for all queues on stall detection is
recomended by hardware team.

The change introduces a single ordered workqueue
("mana_per_port_queue_reset_wq") with WQ_UNBOUND | WQ_MEM_RECLAIM and
queues exactly one work_struct per port onto it.

Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
---
Changes in v3:
  -Fixed commit meesage, removed rtnl_trylock and added
   disable_work_sync, fixed mana_queue_reset_work, and few
   cosmetics.
Changes in v2:
  -Fixed cosmetic changes.
---
---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 78 ++++++++++++++++++-
 include/net/mana/gdma.h                       |  7 +-
 include/net/mana/mana.h                       |  7 ++
 3 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index cccd5b63cee6..636df3b066c5 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -298,6 +298,42 @@ static int mana_get_gso_hs(struct sk_buff *skb)
 	return gso_hs;
 }
 
+static void mana_per_port_queue_reset_work_handler(struct work_struct *work)
+{
+	struct mana_queue_reset_work *reset_queue_work =
+			container_of(work, struct mana_queue_reset_work, work);
+
+	struct mana_port_context *apc = container_of(reset_queue_work,
+						     struct mana_port_context,
+						     queue_reset_work);
+	struct net_device *ndev = apc->ndev;
+	int err;
+
+	rtnl_lock();
+
+	/* Pre-allocate buffers to prevent failure in mana_attach later */
+	err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
+	if (err) {
+		netdev_err(ndev, "Insufficient memory for reset post tx stall detection\n");
+		goto out;
+	}
+
+	err = mana_detach(ndev, false);
+	if (err) {
+		netdev_err(ndev, "mana_detach failed: %d\n", err);
+		goto dealloc_pre_rxbufs;
+	}
+
+	err = mana_attach(ndev);
+	if (err)
+		netdev_err(ndev, "mana_attach failed: %d\n", err);
+
+dealloc_pre_rxbufs:
+	mana_pre_dealloc_rxbufs(apc);
+out:
+	rtnl_unlock();
+}
+
 netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	enum mana_tx_pkt_format pkt_fmt = MANA_SHORT_PKT_FMT;
@@ -802,6 +838,23 @@ static int mana_change_mtu(struct net_device *ndev, int new_mtu)
 	return err;
 }
 
+static void mana_tx_timeout(struct net_device *netdev, unsigned int txqueue)
+{
+	struct mana_port_context *apc = netdev_priv(netdev);
+	struct mana_context *ac = apc->ac;
+	struct gdma_context *gc = ac->gdma_dev->gdma_context;
+
+	/* Already in service, hence tx queue reset is not required.*/
+	if (gc->in_service)
+		return;
+
+	/* Note: If there are pending queue reset work for this port(apc),
+	 * subsequent request queued up from here are ignored. This is because
+	 * we are using the same work instance per port(apc).
+	 */
+	queue_work(ac->per_port_queue_reset_wq, &apc->queue_reset_work.work);
+}
+
 static int mana_shaper_set(struct net_shaper_binding *binding,
 			   const struct net_shaper *shaper,
 			   struct netlink_ext_ack *extack)
@@ -884,7 +937,9 @@ static const struct net_device_ops mana_devops = {
 	.ndo_bpf		= mana_bpf,
 	.ndo_xdp_xmit		= mana_xdp_xmit,
 	.ndo_change_mtu		= mana_change_mtu,
-	.net_shaper_ops         = &mana_shaper_ops,
+	.ndo_tx_timeout		= mana_tx_timeout,
+	.net_shaper_ops		= &mana_shaper_ops,
+
 };
 
 static void mana_cleanup_port_context(struct mana_port_context *apc)
@@ -3244,6 +3299,7 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
 	ndev->min_mtu = ETH_MIN_MTU;
 	ndev->needed_headroom = MANA_HEADROOM;
 	ndev->dev_port = port_idx;
+	ndev->watchdog_timeo = 15 * HZ;
 	SET_NETDEV_DEV(ndev, gc->dev);
 
 	netif_set_tso_max_size(ndev, GSO_MAX_SIZE);
@@ -3283,6 +3339,10 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
 
 	debugfs_create_u32("current_speed", 0400, apc->mana_port_debugfs, &apc->speed);
 
+	/* Initialize the per port queue reset work.*/
+	INIT_WORK(&apc->queue_reset_work.work,
+		  mana_per_port_queue_reset_work_handler);
+
 	return 0;
 
 free_indir:
@@ -3488,6 +3548,15 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
 	if (ac->num_ports > MAX_PORTS_IN_MANA_DEV)
 		ac->num_ports = MAX_PORTS_IN_MANA_DEV;
 
+	ac->per_port_queue_reset_wq =
+			alloc_ordered_workqueue("mana_per_port_queue_reset_wq",
+						WQ_UNBOUND | WQ_MEM_RECLAIM);
+	if (!ac->per_port_queue_reset_wq) {
+		dev_err(dev, "Failed to allocate per port queue reset workqueue\n");
+		err = -ENOMEM;
+		goto out;
+	}
+
 	if (!resuming) {
 		for (i = 0; i < ac->num_ports; i++) {
 			err = mana_probe_port(ac, i, &ac->ports[i]);
@@ -3557,6 +3626,8 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
 			goto out;
 		}
 
+		disable_work_sync(&apc->queue_reset_work.work);
+
 		/* All cleanup actions should stay after rtnl_lock(), otherwise
 		 * other functions may access partially cleaned up data.
 		 */
@@ -3581,6 +3652,11 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
 		free_netdev(ndev);
 	}
 
+	if (ac->per_port_queue_reset_wq) {
+		destroy_workqueue(ac->per_port_queue_reset_wq);
+		ac->per_port_queue_reset_wq = NULL;
+	}
+
 	mana_destroy_eq(ac);
 out:
 	mana_gd_deregister_device(gd);
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 637f42485dba..b892296705de 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -590,6 +590,10 @@ enum {
 
 /* Driver can self reset on FPGA Reconfig EQE notification */
 #define GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE BIT(17)
+
+/* Driver detects stalled send queues and recovers them */
+#define GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY BIT(18)
+
 #define GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE BIT(6)
 
 #define GDMA_DRV_CAP_FLAGS1 \
@@ -601,7 +605,8 @@ enum {
 	 GDMA_DRV_CAP_FLAG_1_DYNAMIC_IRQ_ALLOC_SUPPORT | \
 	 GDMA_DRV_CAP_FLAG_1_SELF_RESET_ON_EQE | \
 	 GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE | \
-	 GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE)
+	 GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE |\
+	 GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY)
 
 #define GDMA_DRV_CAP_FLAGS2 0
 
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 8906901535f5..095f5f1cb2ad 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -475,6 +475,7 @@ struct mana_context {
 
 	struct mana_eq *eqs;
 	struct dentry *mana_eqs_debugfs;
+	struct workqueue_struct *per_port_queue_reset_wq;
 
 	struct net_device *ports[MAX_PORTS_IN_MANA_DEV];
 
@@ -483,9 +484,15 @@ struct mana_context {
 	u32 link_event;
 };
 
+struct mana_queue_reset_work {
+	/* Work structure */
+	struct work_struct work;
+};
+
 struct mana_port_context {
 	struct mana_context *ac;
 	struct net_device *ndev;
+	struct mana_queue_reset_work queue_reset_work;
 
 	u8 mac_addr[ETH_ALEN];
 
-- 
2.43.0

Re: [PATCH net-next, v3] net: mana: Implement ndo_tx_timeout and serialize queue resets per port.

[Simon Horman]

On Mon, Nov 10, 2025 at 02:35:41AM -0800, Dipayaan Roy wrote:
> Implement .ndo_tx_timeout for MANA so any stalled TX queue can be detected
> and a device-controlled port reset for all queues can be scheduled to a
> ordered workqueue. The reset for all queues on stall detection is
> recomended by hardware team.
> 
> The change introduces a single ordered workqueue
> ("mana_per_port_queue_reset_wq") with WQ_UNBOUND | WQ_MEM_RECLAIM and
> queues exactly one work_struct per port onto it.

I see that this goes some way to addressing Jakub's feedback
on the commit message in his review of v2. But I this paragraph
isn't adding much in it's current form. It seems to me some
explanation of why why WQ_UNBOUND and WQ_MEM_RECLAIM are used is
appropriate.

[1] https://lore.kernel.org/all/20251029182233.59aea2d3@kernel.org/

> Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> ---
> Changes in v3:
>   -Fixed commit meesage, removed rtnl_trylock and added
>    disable_work_sync, fixed mana_queue_reset_work, and few
>    cosmetics.
> Changes in v2:
>   -Fixed cosmetic changes.
> ---
> ---
>  drivers/net/ethernet/microsoft/mana/mana_en.c | 78 ++++++++++++++++++-
>  include/net/mana/gdma.h                       |  7 +-
>  include/net/mana/mana.h                       |  7 ++
>  3 files changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index cccd5b63cee6..636df3b066c5 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -298,6 +298,42 @@ static int mana_get_gso_hs(struct sk_buff *skb)
>  	return gso_hs;
>  }
>  
> +static void mana_per_port_queue_reset_work_handler(struct work_struct *work)
> +{
> +	struct mana_queue_reset_work *reset_queue_work =
> +			container_of(work, struct mana_queue_reset_work, work);
> +
> +	struct mana_port_context *apc = container_of(reset_queue_work,
> +						     struct mana_port_context,
> +						     queue_reset_work);
> +	struct net_device *ndev = apc->ndev;
> +	int err;
> +
> +	rtnl_lock();
> +
> +	/* Pre-allocate buffers to prevent failure in mana_attach later */
> +	err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
> +	if (err) {
> +		netdev_err(ndev, "Insufficient memory for reset post tx stall detection\n");
> +		goto out;
> +	}
> +
> +	err = mana_detach(ndev, false);
> +	if (err) {
> +		netdev_err(ndev, "mana_detach failed: %d\n", err);
> +		goto dealloc_pre_rxbufs;
> +	}
> +
> +	err = mana_attach(ndev);
> +	if (err)
> +		netdev_err(ndev, "mana_attach failed: %d\n", err);
> +
> +dealloc_pre_rxbufs:
> +	mana_pre_dealloc_rxbufs(apc);
> +out:
> +	rtnl_unlock();
> +}
> +
>  netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  {
>  	enum mana_tx_pkt_format pkt_fmt = MANA_SHORT_PKT_FMT;
> @@ -802,6 +838,23 @@ static int mana_change_mtu(struct net_device *ndev, int new_mtu)
>  	return err;
>  }
>  
> +static void mana_tx_timeout(struct net_device *netdev, unsigned int txqueue)
> +{
> +	struct mana_port_context *apc = netdev_priv(netdev);
> +	struct mana_context *ac = apc->ac;
> +	struct gdma_context *gc = ac->gdma_dev->gdma_context;
> +
> +	/* Already in service, hence tx queue reset is not required.*/
> +	if (gc->in_service)
> +		return;
> +
> +	/* Note: If there are pending queue reset work for this port(apc),
> +	 * subsequent request queued up from here are ignored. This is because
> +	 * we are using the same work instance per port(apc).
> +	 */
> +	queue_work(ac->per_port_queue_reset_wq, &apc->queue_reset_work.work);
> +}
> +
>  static int mana_shaper_set(struct net_shaper_binding *binding,
>  			   const struct net_shaper *shaper,
>  			   struct netlink_ext_ack *extack)
> @@ -884,7 +937,9 @@ static const struct net_device_ops mana_devops = {
>  	.ndo_bpf		= mana_bpf,
>  	.ndo_xdp_xmit		= mana_xdp_xmit,
>  	.ndo_change_mtu		= mana_change_mtu,
> -	.net_shaper_ops         = &mana_shaper_ops,
> +	.ndo_tx_timeout		= mana_tx_timeout,
> +	.net_shaper_ops		= &mana_shaper_ops,
> +
>  };
>  
>  static void mana_cleanup_port_context(struct mana_port_context *apc)
> @@ -3244,6 +3299,7 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
>  	ndev->min_mtu = ETH_MIN_MTU;
>  	ndev->needed_headroom = MANA_HEADROOM;
>  	ndev->dev_port = port_idx;
> +	ndev->watchdog_timeo = 15 * HZ;
>  	SET_NETDEV_DEV(ndev, gc->dev);
>  
>  	netif_set_tso_max_size(ndev, GSO_MAX_SIZE);
> @@ -3283,6 +3339,10 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
>  
>  	debugfs_create_u32("current_speed", 0400, apc->mana_port_debugfs, &apc->speed);
>  
> +	/* Initialize the per port queue reset work.*/
> +	INIT_WORK(&apc->queue_reset_work.work,
> +		  mana_per_port_queue_reset_work_handler);
> +

I think it would make more sense to move this to before the call to
register_netdev(), which is a few lines above this hunk.

I suppose that because a watchdog timeout is involved, it won't happen in
practice, but in theory could fire ndo_tx_timeout before INIT_WORK is
called, resulting in access to the work queue before it is initialised.

>  	return 0;
>  
>  free_indir:
> @@ -3488,6 +3548,15 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
>  	if (ac->num_ports > MAX_PORTS_IN_MANA_DEV)
>  		ac->num_ports = MAX_PORTS_IN_MANA_DEV;
>  
> +	ac->per_port_queue_reset_wq =
> +			alloc_ordered_workqueue("mana_per_port_queue_reset_wq",
> +						WQ_UNBOUND | WQ_MEM_RECLAIM);
> +	if (!ac->per_port_queue_reset_wq) {
> +		dev_err(dev, "Failed to allocate per port queue reset workqueue\n");
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
>  	if (!resuming) {
>  		for (i = 0; i < ac->num_ports; i++) {
>  			err = mana_probe_port(ac, i, &ac->ports[i]);

It is not strictly related to this patch, but the lines above the hunk
below look like this:

		apc = netdev_priv(ndev);
		if (!ndev) {
			if (i == 0)
				dev_err(dev, "No net device to remove\n");

If ndev is null then the call to netdev_priv() will result in a
NULL pointer dereference. So I think it should be moved
to after the check for !ndev.

> @@ -3557,6 +3626,8 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
>  			goto out;
>  		}
>  
> +		disable_work_sync(&apc->queue_reset_work.work);
> +
>  		/* All cleanup actions should stay after rtnl_lock(), otherwise
>  		 * other functions may access partially cleaned up data.
>  		 */

Comments on code flagged by Claude Code with
https://github.com/masoncl/review-prompts/

Re: [PATCH net-next, v3] net: mana: Implement ndo_tx_timeout and serialize queue resets per port.

[Dipayaan Roy]

On Tue, Nov 11, 2025 at 05:02:21PM +0000, Simon Horman wrote:
> On Mon, Nov 10, 2025 at 02:35:41AM -0800, Dipayaan Roy wrote:
> > Implement .ndo_tx_timeout for MANA so any stalled TX queue can be detected
> > and a device-controlled port reset for all queues can be scheduled to a
> > ordered workqueue. The reset for all queues on stall detection is
> > recomended by hardware team.
> > 
> > The change introduces a single ordered workqueue
> > ("mana_per_port_queue_reset_wq") with WQ_UNBOUND | WQ_MEM_RECLAIM and
> > queues exactly one work_struct per port onto it.
> 
> I see that this goes some way to addressing Jakub's feedback
> on the commit message in his review of v2. But I this paragraph
> isn't adding much in it's current form. It seems to me some
> explanation of why why WQ_UNBOUND and WQ_MEM_RECLAIM are used is
> appropriate.
> 
> [1] https://lore.kernel.org/all/20251029182233.59aea2d3@kernel.org/
>
Sure, I can add short explanation on the flag usage.
 
> > Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> > ---
> > Changes in v3:
> >   -Fixed commit meesage, removed rtnl_trylock and added
> >    disable_work_sync, fixed mana_queue_reset_work, and few
> >    cosmetics.
> > Changes in v2:
> >   -Fixed cosmetic changes.
> > ---
> > ---
> >  drivers/net/ethernet/microsoft/mana/mana_en.c | 78 ++++++++++++++++++-
> >  include/net/mana/gdma.h                       |  7 +-
> >  include/net/mana/mana.h                       |  7 ++
> >  3 files changed, 90 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > index cccd5b63cee6..636df3b066c5 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > @@ -298,6 +298,42 @@ static int mana_get_gso_hs(struct sk_buff *skb)
> >  	return gso_hs;
> >  }
> >  
> > +static void mana_per_port_queue_reset_work_handler(struct work_struct *work)
> > +{
> > +	struct mana_queue_reset_work *reset_queue_work =
> > +			container_of(work, struct mana_queue_reset_work, work);
> > +
> > +	struct mana_port_context *apc = container_of(reset_queue_work,
> > +						     struct mana_port_context,
> > +						     queue_reset_work);
> > +	struct net_device *ndev = apc->ndev;
> > +	int err;
> > +
> > +	rtnl_lock();
> > +
> > +	/* Pre-allocate buffers to prevent failure in mana_attach later */
> > +	err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
> > +	if (err) {
> > +		netdev_err(ndev, "Insufficient memory for reset post tx stall detection\n");
> > +		goto out;
> > +	}
> > +
> > +	err = mana_detach(ndev, false);
> > +	if (err) {
> > +		netdev_err(ndev, "mana_detach failed: %d\n", err);
> > +		goto dealloc_pre_rxbufs;
> > +	}
> > +
> > +	err = mana_attach(ndev);
> > +	if (err)
> > +		netdev_err(ndev, "mana_attach failed: %d\n", err);
> > +
> > +dealloc_pre_rxbufs:
> > +	mana_pre_dealloc_rxbufs(apc);
> > +out:
> > +	rtnl_unlock();
> > +}
> > +
> >  netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> >  {
> >  	enum mana_tx_pkt_format pkt_fmt = MANA_SHORT_PKT_FMT;
> > @@ -802,6 +838,23 @@ static int mana_change_mtu(struct net_device *ndev, int new_mtu)
> >  	return err;
> >  }
> >  
> > +static void mana_tx_timeout(struct net_device *netdev, unsigned int txqueue)
> > +{
> > +	struct mana_port_context *apc = netdev_priv(netdev);
> > +	struct mana_context *ac = apc->ac;
> > +	struct gdma_context *gc = ac->gdma_dev->gdma_context;
> > +
> > +	/* Already in service, hence tx queue reset is not required.*/
> > +	if (gc->in_service)
> > +		return;
> > +
> > +	/* Note: If there are pending queue reset work for this port(apc),
> > +	 * subsequent request queued up from here are ignored. This is because
> > +	 * we are using the same work instance per port(apc).
> > +	 */
> > +	queue_work(ac->per_port_queue_reset_wq, &apc->queue_reset_work.work);
> > +}
> > +
> >  static int mana_shaper_set(struct net_shaper_binding *binding,
> >  			   const struct net_shaper *shaper,
> >  			   struct netlink_ext_ack *extack)
> > @@ -884,7 +937,9 @@ static const struct net_device_ops mana_devops = {
> >  	.ndo_bpf		= mana_bpf,
> >  	.ndo_xdp_xmit		= mana_xdp_xmit,
> >  	.ndo_change_mtu		= mana_change_mtu,
> > -	.net_shaper_ops         = &mana_shaper_ops,
> > +	.ndo_tx_timeout		= mana_tx_timeout,
> > +	.net_shaper_ops		= &mana_shaper_ops,
> > +
> >  };
> >  
> >  static void mana_cleanup_port_context(struct mana_port_context *apc)
> > @@ -3244,6 +3299,7 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
> >  	ndev->min_mtu = ETH_MIN_MTU;
> >  	ndev->needed_headroom = MANA_HEADROOM;
> >  	ndev->dev_port = port_idx;
> > +	ndev->watchdog_timeo = 15 * HZ;
> >  	SET_NETDEV_DEV(ndev, gc->dev);
> >  
> >  	netif_set_tso_max_size(ndev, GSO_MAX_SIZE);
> > @@ -3283,6 +3339,10 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
> >  
> >  	debugfs_create_u32("current_speed", 0400, apc->mana_port_debugfs, &apc->speed);
> >  
> > +	/* Initialize the per port queue reset work.*/
> > +	INIT_WORK(&apc->queue_reset_work.work,
> > +		  mana_per_port_queue_reset_work_handler);
> > +
> 
> I think it would make more sense to move this to before the call to
> register_netdev(), which is a few lines above this hunk.
> 
> I suppose that because a watchdog timeout is involved, it won't happen in
> practice, but in theory could fire ndo_tx_timeout before INIT_WORK is
> called, resulting in access to the work queue before it is initialised.
>
Sure, will address this in next version. 
> >  	return 0;
> >  
> >  free_indir:
> > @@ -3488,6 +3548,15 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
> >  	if (ac->num_ports > MAX_PORTS_IN_MANA_DEV)
> >  		ac->num_ports = MAX_PORTS_IN_MANA_DEV;
> >  
> > +	ac->per_port_queue_reset_wq =
> > +			alloc_ordered_workqueue("mana_per_port_queue_reset_wq",
> > +						WQ_UNBOUND | WQ_MEM_RECLAIM);
> > +	if (!ac->per_port_queue_reset_wq) {
> > +		dev_err(dev, "Failed to allocate per port queue reset workqueue\n");
> > +		err = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> >  	if (!resuming) {
> >  		for (i = 0; i < ac->num_ports; i++) {
> >  			err = mana_probe_port(ac, i, &ac->ports[i]);
> 
> It is not strictly related to this patch, but the lines above the hunk
> below look like this:
> 
> 		apc = netdev_priv(ndev);
> 		if (!ndev) {
> 			if (i == 0)
> 				dev_err(dev, "No net device to remove\n");
> 
> If ndev is null then the call to netdev_priv() will result in a
> NULL pointer dereference. So I think it should be moved
> to after the check for !ndev.
> 
Thanks for pointing this, I will fix this along with my next version.
> > @@ -3557,6 +3626,8 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
> >  			goto out;
> >  		}
> >  
> > +		disable_work_sync(&apc->queue_reset_work.work);
> > +
> >  		/* All cleanup actions should stay after rtnl_lock(), otherwise
> >  		 * other functions may access partially cleaned up data.
> >  		 */
> 
> Comments on code flagged by Claude Code with
> https://github.com/masoncl/review-prompts/

Thanks Simon for the review. I will send the rebased next version with all the
comments addressed.


Regards