public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb: xhci: Fix debugfs bandwidth reporting
@ 2026-03-04 10:49 Michal Pecio
  2026-03-19 21:04 ` Mathias Nyman
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Pecio @ 2026-03-04 10:49 UTC (permalink / raw)
  To: Mathias Nyman, Greg Kroah-Hartman; +Cc: Xu Rao, linux-usb, linux-kernel

Replace kernel USB speed numbers with xHCI protocol IDs expected by HW.
They are numerically equal up to high speed, but instead of SuperSpeed
we were querying SuperSpeed+.

Gen1 hardware rejects such commands with TRB Error, which resulted in
zero available bandwidth being shown.

While at that, report command failure as IO error, not zero bandwidth.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
 drivers/usb/host/xhci-debugfs.c | 6 +++---
 drivers/usb/host/xhci.c         | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index c2080ee4df50..d45fa29a9691 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -709,7 +709,7 @@ static int xhci_ss_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, USB_SPEED_SUPER, s);
+	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_SS), s);
 	return ret;
 }
 
@@ -718,7 +718,7 @@ static int xhci_hs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, USB_SPEED_HIGH, s);
+	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_HS), s);
 	return ret;
 }
 
@@ -727,7 +727,7 @@ static int xhci_fs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, USB_SPEED_FULL, s);
+	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_FS), s);
 	return ret;
 }
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6b312966c103..2f23b796ca12 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3277,6 +3277,8 @@ int xhci_get_port_bandwidth(struct xhci_hcd *xhci, struct xhci_container_ctx *ct
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
 	wait_for_completion(cmd->completion);
+	if (cmd->status != COMP_SUCCESS)
+		ret = -EIO;
 err_out:
 	kfree(cmd->completion);
 	kfree(cmd);
-- 
2.48.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: xhci: Fix debugfs bandwidth reporting
  2026-03-04 10:49 [PATCH] usb: xhci: Fix debugfs bandwidth reporting Michal Pecio
@ 2026-03-19 21:04 ` Mathias Nyman
  2026-03-25 10:32   ` Michal Pecio
  0 siblings, 1 reply; 6+ messages in thread
From: Mathias Nyman @ 2026-03-19 21:04 UTC (permalink / raw)
  To: Michal Pecio, Mathias Nyman, Greg Kroah-Hartman
  Cc: Xu Rao, linux-usb, linux-kernel

On 3/4/26 12:49, Michal Pecio wrote:
> Replace kernel USB speed numbers with xHCI protocol IDs expected by HW.
> They are numerically equal up to high speed, but instead of SuperSpeed
> we were querying SuperSpeed+.
> 
> Gen1 hardware rejects such commands with TRB Error, which resulted in
> zero available bandwidth being shown.
> 
> While at that, report command failure as IO error, not zero bandwidth.
> 
> Signed-off-by: Michal Pecio <michal.pecio@gmail.com>

Added to queue

Thanks
Mathias

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: xhci: Fix debugfs bandwidth reporting
  2026-03-19 21:04 ` Mathias Nyman
@ 2026-03-25 10:32   ` Michal Pecio
  2026-03-25 10:33     ` [PATCH v2] " Michal Pecio
                       ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Michal Pecio @ 2026-03-25 10:32 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: Mathias Nyman, Greg Kroah-Hartman, Xu Rao, linux-usb,
	linux-kernel

On Thu, 19 Mar 2026 23:04:38 +0200, Mathias Nyman wrote:
> On 3/4/26 12:49, Michal Pecio wrote:
> > Replace kernel USB speed numbers with xHCI protocol IDs expected by HW.
> > They are numerically equal up to high speed, but instead of SuperSpeed
> > we were querying SuperSpeed+.
> > 
> > Gen1 hardware rejects such commands with TRB Error, which resulted in
> > zero available bandwidth being shown.
> > 
> > While at that, report command failure as IO error, not zero bandwidth.
> > 
> > Signed-off-by: Michal Pecio <michal.pecio@gmail.com>  
> 
> Added to queue

Hi,

Thanks for taking the patch, but can we have a last minute swap for
a v2 or optionally v3?

Problem is that returning -EIO is common (the command is optional)
and it upsets userspace: "grep -r" spams the console with errors,
"zip -R" terminates and doesn't include remaining files, etc.
So I would prefer to print an error string in this case.
"Real" errors will still be returned ordinarily.

The optional v3 also renames the new directory for consistency.
Technically it's a breaking change, but I believe it's permissible
in debugfs, particularly for an interface only added recently.

Regards,
Michal

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] usb: xhci: Fix debugfs bandwidth reporting
  2026-03-25 10:32   ` Michal Pecio
@ 2026-03-25 10:33     ` Michal Pecio
  2026-03-25 10:34     ` [PATCH v3] " Michal Pecio
  2026-03-25 12:24     ` [PATCH] " Mathias Nyman
  2 siblings, 0 replies; 6+ messages in thread
From: Michal Pecio @ 2026-03-25 10:33 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: Mathias Nyman, Greg Kroah-Hartman, Xu Rao, linux-usb,
	linux-kernel

Replace kernel USB speed numbers with xHCI protocol IDs expected by HW.
They are numerically equal up to high speed, but instead of SuperSpeed
we were querying SuperSpeed+.

Gen1 hardware rejects such commands with TRB Error, which resulted in
zero available bandwidth being shown.

While at that, report failures properly. No attempt made at "tunneling"
all possible comp codes through errno, debugfs users may inspect the
result through event-ring/trbs.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---

New in v2:
- print an error string instead of returning -EIO
- document known and sometimes unobvious error cases

 drivers/usb/host/xhci-debugfs.c | 10 +++++++---
 drivers/usb/host/xhci.c         |  9 ++++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 6cc0f60da771..da528d07b815 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -703,6 +703,10 @@ static int xhci_port_bw_show(struct xhci_hcd *xhci, u8 dev_speed,
 		seq_printf(s, "port[%d] available bw: %d%%.\n", i,
 				ctx->bytes[i]);
 err_out:
+	if (ret == -EIO) {
+		seq_puts(s, "Get Port Bandwidth failed\n");
+		ret = 0;
+	}
 	pm_runtime_put_sync(dev);
 	xhci_free_port_bw_ctx(xhci, ctx);
 	return ret;
@@ -713,7 +717,7 @@ static int xhci_ss_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, USB_SPEED_SUPER, s);
+	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_SS), s);
 	return ret;
 }
 
@@ -722,7 +726,7 @@ static int xhci_hs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, USB_SPEED_HIGH, s);
+	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_HS), s);
 	return ret;
 }
 
@@ -731,7 +735,7 @@ static int xhci_fs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, USB_SPEED_FULL, s);
+	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_FS), s);
 	return ret;
 }
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e6edafdfcdb4..c86819afdede 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3262,7 +3262,12 @@ void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
 }
 EXPORT_SYMBOL_GPL(xhci_reset_bandwidth);
 
-/* Get the available bandwidth of the ports under the xhci roothub */
+/*
+ * Get the available bandwidth of the ports under the xhci roothub.
+ * EIO means the command failed: command not implemented or unsupported
+ * speed (TRB Error), some ASMedia complete with Parameter Error when
+ * querying the root hub (slot_id = 0), or other error or timeout.
+ */
 int xhci_get_port_bandwidth(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
 			    u8 dev_speed)
 {
@@ -3291,6 +3296,8 @@ int xhci_get_port_bandwidth(struct xhci_hcd *xhci, struct xhci_container_ctx *ct
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
 	wait_for_completion(cmd->completion);
+	if (cmd->status != COMP_SUCCESS)
+		ret = -EIO;
 err_out:
 	kfree(cmd->completion);
 	kfree(cmd);
-- 
2.48.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3] usb: xhci: Fix debugfs bandwidth reporting
  2026-03-25 10:32   ` Michal Pecio
  2026-03-25 10:33     ` [PATCH v2] " Michal Pecio
@ 2026-03-25 10:34     ` Michal Pecio
  2026-03-25 12:24     ` [PATCH] " Mathias Nyman
  2 siblings, 0 replies; 6+ messages in thread
From: Michal Pecio @ 2026-03-25 10:34 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: Mathias Nyman, Greg Kroah-Hartman, Xu Rao, linux-usb,
	linux-kernel

Replace kernel USB speed numbers with xHCI protocol IDs expected by HW.
They are numerically equal up to high speed, but instead of SuperSpeed
we were querying SuperSpeed+.

Gen1 hardware rejects such commands with TRB Error, which resulted in
zero available bandwidth being shown.

While at that, report failures properly. No attempt made at "tunneling"
all possible comp codes through errno, debugfs users may inspect the
result through event-ring/trbs.

Rename port_bandwidth to port-bandwidth for consistency with others.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---

New in v3:
- rename port_bandwidth to port-bandwidth

New in v2:
- print an error string instead of returning -EIO
- document known and sometimes unobvious error cases

 drivers/usb/host/xhci-debugfs.c | 12 ++++++++----
 drivers/usb/host/xhci.c         |  9 ++++++++-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 6cc0f60da771..55778c8cab78 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -703,6 +703,10 @@ static int xhci_port_bw_show(struct xhci_hcd *xhci, u8 dev_speed,
 		seq_printf(s, "port[%d] available bw: %d%%.\n", i,
 				ctx->bytes[i]);
 err_out:
+	if (ret == -EIO) {
+		seq_puts(s, "Get Port Bandwidth failed\n");
+		ret = 0;
+	}
 	pm_runtime_put_sync(dev);
 	xhci_free_port_bw_ctx(xhci, ctx);
 	return ret;
@@ -713,7 +717,7 @@ static int xhci_ss_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, USB_SPEED_SUPER, s);
+	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_SS), s);
 	return ret;
 }
 
@@ -722,7 +726,7 @@ static int xhci_hs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, USB_SPEED_HIGH, s);
+	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_HS), s);
 	return ret;
 }
 
@@ -731,7 +735,7 @@ static int xhci_fs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, USB_SPEED_FULL, s);
+	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_FS), s);
 	return ret;
 }
 
@@ -767,7 +771,7 @@ static const struct file_operations bw_fops = {
 static void xhci_debugfs_create_bandwidth(struct xhci_hcd *xhci,
 					struct dentry *parent)
 {
-	parent = debugfs_create_dir("port_bandwidth", parent);
+	parent = debugfs_create_dir("port-bandwidth", parent);
 
 	xhci_debugfs_create_files(xhci, bw_context_files,
 			  ARRAY_SIZE(bw_context_files),
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e6edafdfcdb4..c86819afdede 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3262,7 +3262,12 @@ void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
 }
 EXPORT_SYMBOL_GPL(xhci_reset_bandwidth);
 
-/* Get the available bandwidth of the ports under the xhci roothub */
+/*
+ * Get the available bandwidth of the ports under the xhci roothub.
+ * EIO means the command failed: command not implemented or unsupported
+ * speed (TRB Error), some ASMedia complete with Parameter Error when
+ * querying the root hub (slot_id = 0), or other error or timeout.
+ */
 int xhci_get_port_bandwidth(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
 			    u8 dev_speed)
 {
@@ -3291,6 +3296,8 @@ int xhci_get_port_bandwidth(struct xhci_hcd *xhci, struct xhci_container_ctx *ct
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
 	wait_for_completion(cmd->completion);
+	if (cmd->status != COMP_SUCCESS)
+		ret = -EIO;
 err_out:
 	kfree(cmd->completion);
 	kfree(cmd);
-- 
2.48.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: xhci: Fix debugfs bandwidth reporting
  2026-03-25 10:32   ` Michal Pecio
  2026-03-25 10:33     ` [PATCH v2] " Michal Pecio
  2026-03-25 10:34     ` [PATCH v3] " Michal Pecio
@ 2026-03-25 12:24     ` Mathias Nyman
  2 siblings, 0 replies; 6+ messages in thread
From: Mathias Nyman @ 2026-03-25 12:24 UTC (permalink / raw)
  To: Michal Pecio
  Cc: Mathias Nyman, Greg Kroah-Hartman, Xu Rao, linux-usb,
	linux-kernel

On 3/25/26 12:32, Michal Pecio wrote:
> On Thu, 19 Mar 2026 23:04:38 +0200, Mathias Nyman wrote:
>> On 3/4/26 12:49, Michal Pecio wrote:
>>> Replace kernel USB speed numbers with xHCI protocol IDs expected by HW.
>>> They are numerically equal up to high speed, but instead of SuperSpeed
>>> we were querying SuperSpeed+.
>>>
>>> Gen1 hardware rejects such commands with TRB Error, which resulted in
>>> zero available bandwidth being shown.
>>>
>>> While at that, report command failure as IO error, not zero bandwidth.
>>>
>>> Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
>>
>> Added to queue
> 
> Hi,
> 
> Thanks for taking the patch, but can we have a last minute swap for
> a v2 or optionally v3?
> 

Sure, no problem.

> Problem is that returning -EIO is common (the command is optional)
> and it upsets userspace: "grep -r" spams the console with errors,
> "zip -R" terminates and doesn't include remaining files, etc.
> So I would prefer to print an error string in this case.
> "Real" errors will still be returned ordinarily.
> 
> The optional v3 also renames the new directory for consistency.
> Technically it's a breaking change, but I believe it's permissible
> in debugfs, particularly for an interface only added recently.
> 

lets stick with v2, avoiding possible breakage due to debugfs file rename.

Thanks
Mathias


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-25 12:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-04 10:49 [PATCH] usb: xhci: Fix debugfs bandwidth reporting Michal Pecio
2026-03-19 21:04 ` Mathias Nyman
2026-03-25 10:32   ` Michal Pecio
2026-03-25 10:33     ` [PATCH v2] " Michal Pecio
2026-03-25 10:34     ` [PATCH v3] " Michal Pecio
2026-03-25 12:24     ` [PATCH] " Mathias Nyman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox