[PATCH 0/5] usb: xhci: bandwidth accounting fixes and cleanup

[PATCH 0/5] usb: xhci: bandwidth accounting fixes and cleanup

[Niklas Neronin]

This series contains a small collection of xhci fixes and cleanups.

The main fix addresses a regression where software tracked bandwidth
information could survive hibernation resume after the S4 resume path was
optimized. This could result in incorrect bandwidth accounting when devices
were rediscovered after resume.

While investigating bandwidth accounting, I found a few additional issues:
* Fix endpoint resource accounting on virtual device allocation failure
  for controllers using XHCI_EP_LIMIT_QUIRK.
* Remove a redundant TT active endpoint update during virtual device
  cleanup.
* Correct an endianness conversion for a 32-bit slot context field.
* Remove two unused xhci structures.

Niklas Neronin (5):
  usb: xhci: clear stale bandwidth data after hibernation
  usb: xhci: correct num_active_eps accounting on allocation failure
  usb: xhci: remove redundant TT active EP update from
    xhci_free_virt_device()
  usb: xhci: correct variable size conversion
  usb: xhci: remove unused structs

 drivers/usb/host/xhci-mem.c   | 22 +++++++++++++++-------
 drivers/usb/host/xhci-trace.h |  2 +-
 drivers/usb/host/xhci.c       |  6 ++++++
 drivers/usb/host/xhci.h       | 12 ------------
 4 files changed, 22 insertions(+), 20 deletions(-)

-- 
2.50.1

[PATCH 1/5] usb: xhci: clear stale bandwidth data after hibernation

[Niklas Neronin]

Controllers using software managed bandwidth accounting maintain
periodic endpoint bandwidth information in struct 'xhci_interval_bw_table'.

Each root hub port owns a bandwidth table and a list of TT bandwidth
domains, where each TT has its own bandwidth table. Virtual devices
reference the bandwidth table of their current bandwidth domain through
'bw_table' or 'tt_info'.

Historically, resume from S4 re-allocated the entire xHCI driver state,
which implicitly cleared all bandwidth accounting data. After the
hibernation resume path was optimized to preserve parts of the driver
state, virtual devices and TT bandwidth information were freed and
recreated, but the root hub bandwidth tables were left intact.

As a result, stale bandwidth accounting data could remain in the root
hub bandwidth tables across hibernation resume, leading to incorrect
bandwidth calculations after devices were rediscovered.

Fix this by resetting all software bandwidth accounting state in
xhci_rh_bw_cleanup() so that resume starts with a clean bandwidth state.
This includes 'xhci->num_active_eps', which must remain consistent with
the cleared bandwidth tables.

Fixes: <2a70e5dc0301> ("usb: xhci: optimize resuming from S4 (suspend-to-disk)")
Reported-by: Harald Judt <h.judt@gmx.at>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=222071
Suggested-by: Lovekesh Solanki <lovekeshsolanki00@gmail.com>
Tested-by: Harald Judt <h.judt@gmx.at>
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 16 +++++++++++++++-
 drivers/usb/host/xhci.c     |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index af8d4b74c4ba..75577f441cdd 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1903,24 +1903,38 @@ EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
 void xhci_rh_bw_cleanup(struct xhci_hcd *xhci)
 {
 	struct xhci_root_port_bw_info *rh_bw;
+	struct xhci_interval_bw_table *bw_table;
+	struct xhci_interval_bw *interval_bw;
 	struct xhci_tt_bw_info *tt_info, *tt_next;
 	struct list_head *eps, *ep, *ep_next;
 
 	for (int i = 0; i < xhci->max_ports; i++) {
 		rh_bw = &xhci->rh_bw[i];
 
+		rh_bw->num_active_tts = 0;
 		/* Clear and free all TT bandwidth entries */
 		list_for_each_entry_safe(tt_info, tt_next, &rh_bw->tts, tt_list) {
 			list_del(&tt_info->tt_list);
 			kfree(tt_info);
 		}
 
+		bw_table = &rh_bw->bw_table;
+		bw_table->interval0_esit_payload = 0;
+		bw_table->bw_used = 0;
+		bw_table->ss_bw_in = 0;
+		bw_table->ss_bw_out = 0;
+
 		/* Clear per-interval endpoint lists */
 		for (int j = 0; j < XHCI_MAX_INTERVAL; j++) {
-			eps = &rh_bw->bw_table.interval_bw[j].endpoints;
+			interval_bw = &bw_table->interval_bw[j];
+			eps = &interval_bw->endpoints;
 
+			interval_bw->num_packets = 0;
 			list_for_each_safe(ep, ep_next, eps)
 				list_del_init(ep);
+			interval_bw->overhead[LS_OVERHEAD_TYPE] = 0;
+			interval_bw->overhead[FS_OVERHEAD_TYPE] = 0;
+			interval_bw->overhead[HS_OVERHEAD_TYPE] = 0;
 		}
 	}
 }
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a9e47e178c28..4708fabba84a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1185,6 +1185,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
 		for (int i = xhci->max_slots; i > 0; i--)
 			xhci_free_virt_devices_depth_first(xhci, i);
 
+		xhci->num_active_eps = 0;
 		xhci_rh_bw_cleanup(xhci);
 
 		xhci->cmd_ring_reserved_trbs = 0;
-- 
2.50.1

Re: [PATCH 1/5] usb: xhci: clear stale bandwidth data after hibernation

[Michal Pecio]

On Tue,  6 Oct 2026 17:24:23 +0200, Niklas Neronin wrote:
> Controllers using software managed bandwidth accounting maintain
> periodic endpoint bandwidth information in struct 'xhci_interval_bw_table'.
> 
> Each root hub port owns a bandwidth table and a list of TT bandwidth
> domains, where each TT has its own bandwidth table. Virtual devices
> reference the bandwidth table of their current bandwidth domain through
> 'bw_table' or 'tt_info'.
> 
> Historically, resume from S4 re-allocated the entire xHCI driver state,
> which implicitly cleared all bandwidth accounting data. After the
> hibernation resume path was optimized to preserve parts of the driver
> state, virtual devices and TT bandwidth information were freed and
> recreated, but the root hub bandwidth tables were left intact.
> 
> As a result, stale bandwidth accounting data could remain in the root
> hub bandwidth tables across hibernation resume, leading to incorrect
> bandwidth calculations after devices were rediscovered.
> 
> Fix this by resetting all software bandwidth accounting state in
> xhci_rh_bw_cleanup() so that resume starts with a clean bandwidth state.
> This includes 'xhci->num_active_eps', which must remain consistent with
> the cleared bandwidth tables.

If that's the case then it would make sense to put this inside
xhci_rh_bw_cleanup() rather than xhci_resume(), wouldn't it?

> Fixes: <2a70e5dc0301> ("usb: xhci: optimize resuming from S4 (suspend-to-disk)")
> Reported-by: Harald Judt <h.judt@gmx.at>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=222071
> Suggested-by: Lovekesh Solanki <lovekeshsolanki00@gmail.com>
> Tested-by: Harald Judt <h.judt@gmx.at>
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>  drivers/usb/host/xhci-mem.c | 16 +++++++++++++++-
>  drivers/usb/host/xhci.c     |  1 +
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index af8d4b74c4ba..75577f441cdd 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1903,24 +1903,38 @@ EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
>  void xhci_rh_bw_cleanup(struct xhci_hcd *xhci)
>  {
>  	struct xhci_root_port_bw_info *rh_bw;
> +	struct xhci_interval_bw_table *bw_table;
> +	struct xhci_interval_bw *interval_bw;
>  	struct xhci_tt_bw_info *tt_info, *tt_next;
>  	struct list_head *eps, *ep, *ep_next;
>  
>  	for (int i = 0; i < xhci->max_ports; i++) {
>  		rh_bw = &xhci->rh_bw[i];
>  
> +		rh_bw->num_active_tts = 0;
>  		/* Clear and free all TT bandwidth entries */
>  		list_for_each_entry_safe(tt_info, tt_next, &rh_bw->tts, tt_list) {
>  			list_del(&tt_info->tt_list);
>  			kfree(tt_info);
>  		}
>  
> +		bw_table = &rh_bw->bw_table;
> +		bw_table->interval0_esit_payload = 0;
> +		bw_table->bw_used = 0;
> +		bw_table->ss_bw_in = 0;
> +		bw_table->ss_bw_out = 0;
> +
>  		/* Clear per-interval endpoint lists */
>  		for (int j = 0; j < XHCI_MAX_INTERVAL; j++) {
> -			eps = &rh_bw->bw_table.interval_bw[j].endpoints;
> +			interval_bw = &bw_table->interval_bw[j];
> +			eps = &interval_bw->endpoints;
>  
> +			interval_bw->num_packets = 0;
>  			list_for_each_safe(ep, ep_next, eps)
>  				list_del_init(ep);
> +			interval_bw->overhead[LS_OVERHEAD_TYPE] = 0;
> +			interval_bw->overhead[FS_OVERHEAD_TYPE] = 0;
> +			interval_bw->overhead[HS_OVERHEAD_TYPE] = 0;
>  		}
>  	}
>  }
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index a9e47e178c28..4708fabba84a 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1185,6 +1185,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
>  		for (int i = xhci->max_slots; i > 0; i--)
>  			xhci_free_virt_devices_depth_first(xhci, i);
>  
> +		xhci->num_active_eps = 0;
>  		xhci_rh_bw_cleanup(xhci);
>  
>  		xhci->cmd_ring_reserved_trbs = 0;
> -- 
> 2.50.1
> 

Re: [PATCH 1/5] usb: xhci: clear stale bandwidth data after hibernation

[sashiko-bot]

> Controllers using software managed bandwidth accounting maintain
> periodic endpoint bandwidth information in struct 'xhci_interval_bw_table'.
> 
> Each root hub port owns a bandwidth table and a list of TT bandwidth
> domains, where each TT has its own bandwidth table. Virtual devices
> reference the bandwidth table of their current bandwidth domain through
> 'bw_table' or 'tt_info'.
> 
> Historically, resume from S4 re-allocated the entire xHCI driver state,
> which implicitly cleared all bandwidth accounting data. After the
> hibernation resume path was optimized to preserve parts of the driver
> state, virtual devices and TT bandwidth information were freed and
> recreated, but the root hub bandwidth tables were left intact.
> 
> As a result, stale bandwidth accounting data could remain in the root
> hub bandwidth tables across hibernation resume, leading to incorrect
> bandwidth calculations after devices were rediscovered.
> 
> Fix this by resetting all software bandwidth accounting state in
> xhci_rh_bw_cleanup() so that resume starts with a clean bandwidth state.
> This includes 'xhci->num_active_eps', which must remain consistent with
> the cleared bandwidth tables.
> 
> Fixes: <2a70e5dc0301> ("usb: xhci: optimize resuming from S4 (suspend-to-disk)")
> Reported-by: Harald Judt <h.judt@gmx.at>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=222071
> Suggested-by: Lovekesh Solanki <lovekeshsolanki00@gmail.com>
> Tested-by: Harald Judt <h.judt@gmx.at>
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006152427.3735383-1-niklas.neronin@linux.intel.com?part=1

[PATCH 2/5] usb: xhci: correct num_active_eps accounting on allocation failure

[Niklas Neronin]

Some host controllers have a global endpoint limit across all slots,
tracked by 'xhci->num_active_eps' when XHCI_EP_LIMIT_QUIRK is set.

During device allocation, EP0 resources are reserved before
xhci_alloc_virt_device() is called, and 'num_active_eps' is incremented
accordingly. If xhci_alloc_virt_device() subsequently fails, the slot is
disabled but the reserved endpoint resource is not released, leaving
'num_active_eps' permanently increased.

Decreasing 'num_active_eps' happens in xhci_handle_cmd_disable_slot(),
which is called upon a Disable Slot completion command.

This causes the driver to gradually lose available endpoint resources
after allocation failures and may eventually prevent new endpoints from
being allocated.

Fix this by decrementing 'num_active_eps' when virtual device allocation
fails.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4708fabba84a..ca56e0b69415 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4279,6 +4279,11 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 	 */
 	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
 		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
+		if (xhci->quirks & XHCI_EP_LIMIT_QUIRK) {
+			spin_lock_irqsave(&xhci->lock, flags);
+			xhci->num_active_eps -= 1;
+			spin_unlock_irqrestore(&xhci->lock, flags);
+		}
 		goto disable_slot;
 	}
 	vdev = xhci->devs[slot_id];
-- 
2.50.1

Re: [PATCH 2/5] usb: xhci: correct num_active_eps accounting on allocation failure

[sashiko-bot]

> Some host controllers have a global endpoint limit across all slots,
> tracked by 'xhci->num_active_eps' when XHCI_EP_LIMIT_QUIRK is set.
> 
> During device allocation, EP0 resources are reserved before
> xhci_alloc_virt_device() is called, and 'num_active_eps' is incremented
> accordingly. If xhci_alloc_virt_device() subsequently fails, the slot is
> disabled but the reserved endpoint resource is not released, leaving
> 'num_active_eps' permanently increased.
> 
> Decreasing 'num_active_eps' happens in xhci_handle_cmd_disable_slot(),
> which is called upon a Disable Slot completion command.
> 
> This causes the driver to gradually lose available endpoint resources
> after allocation failures and may eventually prevent new endpoints from
> being allocated.
> 
> Fix this by decrementing 'num_active_eps' when virtual device allocation
> fails.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006152427.3735383-1-niklas.neronin@linux.intel.com?part=2

Re: [PATCH 2/5] usb: xhci: correct num_active_eps accounting on allocation failure

[Michal Pecio]

On Tue,  6 Oct 2026 17:24:24 +0200, Niklas Neronin wrote:
> Some host controllers have a global endpoint limit across all slots,
> tracked by 'xhci->num_active_eps' when XHCI_EP_LIMIT_QUIRK is set.
> 
> During device allocation, EP0 resources are reserved before
> xhci_alloc_virt_device() is called, and 'num_active_eps' is incremented
> accordingly. If xhci_alloc_virt_device() subsequently fails, the slot is
> disabled but the reserved endpoint resource is not released, leaving
> 'num_active_eps' permanently increased.
> 
> Decreasing 'num_active_eps' happens in xhci_handle_cmd_disable_slot(),
> which is called upon a Disable Slot completion command.

Why is 'num_active_eps' not decremented if the slot is disabled and
Disable Slot completion handler is supposed to decrement it?

Does this bug really exist? Can you reproduce it by forcing the quirk
with module parameter and simulating vdev allocation failure here?

And it makes sense that Disable Slot should be queued, because this
code only seems to run after successful Enable Slot and we don't want
to leak the HW slot on allocation error.

> This causes the driver to gradually lose available endpoint resources
> after allocation failures and may eventually prevent new endpoints from
> being allocated.
>
> Fix this by decrementing 'num_active_eps' when virtual device allocation
> fails.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>  drivers/usb/host/xhci.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 4708fabba84a..ca56e0b69415 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -4279,6 +4279,11 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
>  	 */
>  	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
>  		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
> +		if (xhci->quirks & XHCI_EP_LIMIT_QUIRK) {
> +			spin_lock_irqsave(&xhci->lock, flags);
> +			xhci->num_active_eps -= 1;
> +			spin_unlock_irqrestore(&xhci->lock, flags);
> +		}
>  		goto disable_slot;
>  	}
>  	vdev = xhci->devs[slot_id];
> -- 
> 2.50.1
> 

Re: [PATCH 2/5] usb: xhci: correct num_active_eps accounting on allocation failure

[Michal Pecio]

On Tue, 6 Oct 2026 18:33:01 +0200, Michal Pecio wrote:
> On Tue,  6 Oct 2026 17:24:24 +0200, Niklas Neronin wrote:
> > Some host controllers have a global endpoint limit across all slots,
> > tracked by 'xhci->num_active_eps' when XHCI_EP_LIMIT_QUIRK is set.
> > 
> > During device allocation, EP0 resources are reserved before
> > xhci_alloc_virt_device() is called, and 'num_active_eps' is
> > incremented accordingly. If xhci_alloc_virt_device() subsequently
> > fails, the slot is disabled but the reserved endpoint resource is
> > not released, leaving 'num_active_eps' permanently increased.
> > 
> > Decreasing 'num_active_eps' happens in
> > xhci_handle_cmd_disable_slot(), which is called upon a Disable Slot
> > completion command.  
> 
> Why is 'num_active_eps' not decremented if the slot is disabled and
> Disable Slot completion handler is supposed to decrement it?
> 
> Does this bug really exist? Can you reproduce it by forcing the quirk
> with module parameter and simulating vdev allocation failure here?

Never mind, the bug is real and reproducible. Still, commit message
could include a few words of explanation to prevent such questions.

I think it would be cleaner to fix this by allocating vdev before
ep0 reservation, because allocation is easier to undo:

1. we have a helper for this, no need to dig in xhci members manually
2. we are the only user of this slot_id, so locking is not required

Downside: larger diff. But less code in the end.

And there is another bug: disable_slot uses udev->slot_id, which is
uninitialized in this path. Should use the local slot_id variable.

So overall,

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e5b39be04b92..9beb8b4b485f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4308,27 +4308,29 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 
 	xhci_free_command(xhci, command);
 
+	/* Use GFP_NOIO, since this function can be called from
+	 * xhci_discover_or_reset_device(), which may be called as part of
+	 * mass storage driver error handling.
+	 */
+	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
+		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
+		goto disable_slot;
+	}
+	vdev = xhci->devs[slot_id];
+
 	if ((xhci->quirks & XHCI_EP_LIMIT_QUIRK)) {
 		spin_lock_irqsave(&xhci->lock, flags);
 		ret = xhci_reserve_host_control_ep_resources(xhci);
+		spin_unlock_irqrestore(&xhci->lock, flags);
 		if (ret) {
-			spin_unlock_irqrestore(&xhci->lock, flags);
 			xhci_warn(xhci, "Not enough host resources, "
 					"active endpoint contexts = %u\n",
 					xhci->num_active_eps);
+			xhci_free_virt_device(xhci, vdev, slot_id);
+			/* without vdev disable slot won't free ep0 resources */
 			goto disable_slot;
 		}
-		spin_unlock_irqrestore(&xhci->lock, flags);
 	}
-	/* Use GFP_NOIO, since this function can be called from
-	 * xhci_discover_or_reset_device(), which may be called as part of
-	 * mass storage driver error handling.
-	 */
-	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
-		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
-		goto disable_slot;
-	}
-	vdev = xhci->devs[slot_id];
 	slot_ctx = xhci_get_slot_ctx(xhci, vdev->out_ctx);
 	trace_xhci_alloc_dev(slot_ctx);
 
@@ -4348,7 +4350,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 	return 1;
 
 disable_slot:
-	xhci_disable_and_free_slot(xhci, udev->slot_id);
+	xhci_disable_slot(xhci, slot_id);
 
 	return 0;
 }

Re: [PATCH 2/5] usb: xhci: correct num_active_eps accounting on allocation failure

[Neronin, Niklas]

On 07/10/2026 0.02, Michal Pecio wrote:
> On Tue, 6 Oct 2026 18:33:01 +0200, Michal Pecio wrote:
>> On Tue,  6 Oct 2026 17:24:24 +0200, Niklas Neronin wrote:
>>> Some host controllers have a global endpoint limit across all slots,
>>> tracked by 'xhci->num_active_eps' when XHCI_EP_LIMIT_QUIRK is set.
>>>
>>> During device allocation, EP0 resources are reserved before
>>> xhci_alloc_virt_device() is called, and 'num_active_eps' is
>>> incremented accordingly. If xhci_alloc_virt_device() subsequently
>>> fails, the slot is disabled but the reserved endpoint resource is
>>> not released, leaving 'num_active_eps' permanently increased.
>>>
>>> Decreasing 'num_active_eps' happens in
>>> xhci_handle_cmd_disable_slot(), which is called upon a Disable Slot
>>> completion command.  
>>
>> Why is 'num_active_eps' not decremented if the slot is disabled and
>> Disable Slot completion handler is supposed to decrement it?
>>
>> Does this bug really exist? Can you reproduce it by forcing the quirk
>> with module parameter and simulating vdev allocation failure here?
> 
> Never mind, the bug is real and reproducible. Still, commit message
> could include a few words of explanation to prevent such questions.
> 
> I think it would be cleaner to fix this by allocating vdev before
> ep0 reservation, because allocation is easier to undo:

That's a good point. I found this while working on a separate series
that splits 'vdev' allocation and slot enabling. That series was not
ready for this merge window, so I decided to submit only the simpler
fixes first to keep the future series smaller and less cluttered.
As you suggested, that series moves 'vdev' allocation before EP
reservation.

> 
> 1. we have a helper for this, no need to dig in xhci members manually
> 2. we are the only user of this slot_id, so locking is not required
> 
> Downside: larger diff. But less code in the end.
> 
> And there is another bug: disable_slot uses udev->slot_id, which is
> uninitialized in this path. Should use the local slot_id variable.

Good catch. I found the same issue but apparently forgot to include the
fix in this patch set.

Probably best if this patch is not added to this merge window,
instead I'll re-submit it with all the virtual device changes.

Thanks,
Niklas

> 
> So overall,
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index e5b39be04b92..9beb8b4b485f 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -4308,27 +4308,29 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
>  
>  	xhci_free_command(xhci, command);
>  
> +	/* Use GFP_NOIO, since this function can be called from
> +	 * xhci_discover_or_reset_device(), which may be called as part of
> +	 * mass storage driver error handling.
> +	 */
> +	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
> +		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
> +		goto disable_slot;
> +	}
> +	vdev = xhci->devs[slot_id];
> +
>  	if ((xhci->quirks & XHCI_EP_LIMIT_QUIRK)) {
>  		spin_lock_irqsave(&xhci->lock, flags);
>  		ret = xhci_reserve_host_control_ep_resources(xhci);
> +		spin_unlock_irqrestore(&xhci->lock, flags);
>  		if (ret) {
> -			spin_unlock_irqrestore(&xhci->lock, flags);
>  			xhci_warn(xhci, "Not enough host resources, "
>  					"active endpoint contexts = %u\n",
>  					xhci->num_active_eps);
> +			xhci_free_virt_device(xhci, vdev, slot_id);
> +			/* without vdev disable slot won't free ep0 resources */
>  			goto disable_slot;
>  		}
> -		spin_unlock_irqrestore(&xhci->lock, flags);
>  	}
> -	/* Use GFP_NOIO, since this function can be called from
> -	 * xhci_discover_or_reset_device(), which may be called as part of
> -	 * mass storage driver error handling.
> -	 */
> -	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
> -		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
> -		goto disable_slot;
> -	}
> -	vdev = xhci->devs[slot_id];
>  	slot_ctx = xhci_get_slot_ctx(xhci, vdev->out_ctx);
>  	trace_xhci_alloc_dev(slot_ctx);
>  
> @@ -4348,7 +4350,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
>  	return 1;
>  
>  disable_slot:
> -	xhci_disable_and_free_slot(xhci, udev->slot_id);
> +	xhci_disable_slot(xhci, slot_id);
>  
>  	return 0;
>  }

[PATCH 3/5] usb: xhci: remove redundant TT active EP update from xhci_free_virt_device()

[Niklas Neronin]

Function xhci_free_virt_device() saves 'tt_info->active_eps' and later
calls xhci_update_tt_active_eps().

However, xhci_update_tt_active_eps() only updates TT accounting when the
number of active endpoints changes. xhci_free_virt_device() does not add
or remove endpoints and therefore leaves 'tt_info->active_eps' unchanged.
As a result, the call to xhci_update_tt_active_eps() never modifies any
state.

Remove the unused active endpoint tracking and the redundant call.

In cases where virtual devices are freed without first dropping their
endpoints, xhci_rh_bw_cleanup() resets all bandwidth accounting data,
making the update unnecessary.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 75577f441cdd..d665cad22c10 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -870,7 +870,6 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev,
 		int slot_id)
 {
 	int i;
-	int old_active_eps = 0;
 
 	/* Slot ID 0 is reserved */
 	if (slot_id == 0 || !dev)
@@ -883,9 +882,6 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev,
 
 	trace_xhci_free_virt_device(dev);
 
-	if (dev->tt_info)
-		old_active_eps = dev->tt_info->active_eps;
-
 	for (i = 0; i < 31; i++) {
 		if (dev->eps[i].ring)
 			xhci_ring_free(xhci, dev->eps[i].ring);
@@ -908,8 +904,6 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev,
 	}
 	/* If this is a hub, free the TT(s) from the TT list */
 	xhci_free_tt_info(xhci, dev, slot_id);
-	/* If necessary, update the number of active TTs on this root port */
-	xhci_update_tt_active_eps(xhci, dev, old_active_eps);
 
 	if (dev->in_ctx)
 		xhci_free_container_ctx(xhci, dev->in_ctx);
-- 
2.50.1

Re: [PATCH 3/5] usb: xhci: remove redundant TT active EP update from xhci_free_virt_device()

[sashiko-bot]

> Function xhci_free_virt_device() saves 'tt_info->active_eps' and later
> calls xhci_update_tt_active_eps().
> 
> However, xhci_update_tt_active_eps() only updates TT accounting when the
> number of active endpoints changes. xhci_free_virt_device() does not add
> or remove endpoints and therefore leaves 'tt_info->active_eps' unchanged.
> As a result, the call to xhci_update_tt_active_eps() never modifies any
> state.
> 
> Remove the unused active endpoint tracking and the redundant call.
> 
> In cases where virtual devices are freed without first dropping their
> endpoints, xhci_rh_bw_cleanup() resets all bandwidth accounting data,
> making the update unnecessary.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006152427.3735383-1-niklas.neronin@linux.intel.com?part=3

[PATCH 4/5] usb: xhci: correct variable size conversion

[Niklas Neronin]

Variable 'ctx->tt_info' is defined as a le32 within the 'xhci_slot_ctx'
structure. Correct the conversion by using le32_to_cpu(), ensuring
proper handling of the 32-bit 'tt_info' variable.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 724cba2dbb78..f88f320811a3 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -392,7 +392,7 @@ DECLARE_EVENT_CLASS(xhci_log_slot_ctx,
 	TP_fast_assign(
 		__entry->info = le32_to_cpu(ctx->dev_info);
 		__entry->info2 = le32_to_cpu(ctx->dev_info2);
-		__entry->tt_info = le64_to_cpu(ctx->tt_info);
+		__entry->tt_info = le32_to_cpu(ctx->tt_info);
 		__entry->state = le32_to_cpu(ctx->dev_state);
 	),
 	TP_printk("%s", xhci_decode_slot_context(__get_buf(XHCI_MSG_MAX),
-- 
2.50.1

Re: [PATCH 4/5] usb: xhci: correct variable size conversion

[sashiko-bot]

> Variable 'ctx->tt_info' is defined as a le32 within the 'xhci_slot_ctx'
> structure. Correct the conversion by using le32_to_cpu(), ensuring
> proper handling of the 32-bit 'tt_info' variable.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006152427.3735383-1-niklas.neronin@linux.intel.com?part=4

[PATCH 5/5] usb: xhci: remove unused structs

[Niklas Neronin]

Neither structure has any remaining users in the xHCI driver, so
remove them as dead code.

Last time 'xhci_cd' was used [1] in 2014.
Last time 'dev_info' was used [2] in 2013.

Link: https://git.kernel.org/torvalds/c/c311e391a7ef [1]
Link: https://git.kernel.org/torvalds/c/de68bab4fa96 [2]
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci.h | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index c7bfa7f028d3..20abcc5a7acc 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1322,12 +1322,6 @@ struct xhci_td {
  */
 #define XHCI_CMD_DEFAULT_TIMEOUT	5000
 
-/* command descriptor */
-struct xhci_cd {
-	struct xhci_command	*command;
-	union xhci_trb		*cmd_trb;
-};
-
 enum xhci_ring_type {
 	TYPE_CTRL = 0,
 	TYPE_ISOC,
@@ -1424,12 +1418,6 @@ struct s3_save {
 	u32	config_reg;
 };
 
-/* Use for lpm */
-struct dev_info {
-	u32			dev_id;
-	struct	list_head	list;
-};
-
 struct xhci_bus_state {
 	unsigned long		bus_suspended;
 	unsigned long		next_statechange;
-- 
2.50.1

Re: [PATCH 5/5] usb: xhci: remove unused structs

[sashiko-bot]

> Neither structure has any remaining users in the xHCI driver, so
> remove them as dead code.
> 
> Last time 'xhci_cd' was used [1] in 2014.
> Last time 'dev_info' was used [2] in 2013.
> 
> Link: https://git.kernel.org/torvalds/c/c311e391a7ef [1]
> Link: https://git.kernel.org/torvalds/c/de68bab4fa96 [2]
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006152427.3735383-1-niklas.neronin@linux.intel.com?part=5