[v1] usb: xhci: print xhci->xhc_state when queue_command failed

[PATCH] usb: xhci: print xhci->xhc_state when queue_command failed

Posted by Su Hui 2 months, 1 week ago

When encounters some errors like these:
xhci_hcd 0000:4a:00.2: xHCI dying or halted, can't queue_command
xhci_hcd 0000:4a:00.2: FIXME: allocate a command ring segment
usb usb5-port6: couldn't allocate usb_device

It's hard to know whether xhc_state is dying or halted. So it's better
to print xhc_state's value which can help locate the resaon of the bug.

Signed-off-by: Su Hui <suhui@nfschina.com>
---
 drivers/usb/host/xhci-ring.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 94c9c9271658..a1a628e849c0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -4372,7 +4372,8 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
 
 	if ((xhci->xhc_state & XHCI_STATE_DYING) ||
 		(xhci->xhc_state & XHCI_STATE_HALTED)) {
-		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command\n");
+		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command. state: %u\n",
+			 xhci->xhc_state);
 		return -ESHUTDOWN;
 	}
 
-- 
2.30.2

Re: [PATCH] usb: xhci: print xhci->xhc_state when queue_command failed

Posted by Greg KH 2 months, 1 week ago

On Fri, Jul 25, 2025 at 11:13:09AM +0800, Su Hui wrote:
> When encounters some errors like these:
> xhci_hcd 0000:4a:00.2: xHCI dying or halted, can't queue_command
> xhci_hcd 0000:4a:00.2: FIXME: allocate a command ring segment
> usb usb5-port6: couldn't allocate usb_device
> 
> It's hard to know whether xhc_state is dying or halted. So it's better
> to print xhc_state's value which can help locate the resaon of the bug.
> 
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
>  drivers/usb/host/xhci-ring.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 94c9c9271658..a1a628e849c0 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -4372,7 +4372,8 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
>  
>  	if ((xhci->xhc_state & XHCI_STATE_DYING) ||
>  		(xhci->xhc_state & XHCI_STATE_HALTED)) {
> -		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command\n");
> +		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command. state: %u\n",
> +			 xhci->xhc_state);

Don't you want to see this value in hex?

thanks,

greg k-h

Re: [PATCH] usb: xhci: print xhci->xhc_state when queue_command failed

Posted by Su Hui 2 months, 1 week ago

On 2025/7/25 12:43, Greg KH wrote:
> On Fri, Jul 25, 2025 at 11:13:09AM +0800, Su Hui wrote:
>> When encounters some errors like these:
>> xhci_hcd 0000:4a:00.2: xHCI dying or halted, can't queue_command
>> xhci_hcd 0000:4a:00.2: FIXME: allocate a command ring segment
>> usb usb5-port6: couldn't allocate usb_device
>>
>> It's hard to know whether xhc_state is dying or halted. So it's better
>> to print xhc_state's value which can help locate the resaon of the bug.
>>
>> Signed-off-by: Su Hui <suhui@nfschina.com>
>> ---
>>   drivers/usb/host/xhci-ring.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index 94c9c9271658..a1a628e849c0 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -4372,7 +4372,8 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
>>   
>>   	if ((xhci->xhc_state & XHCI_STATE_DYING) ||
>>   		(xhci->xhc_state & XHCI_STATE_HALTED)) {
>> -		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command\n");
>> +		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command. state: %u\n",
>> +			 xhci->xhc_state);
> Don't you want to see this value in hex?

yes, hex is better, will update in v2 soon.

Su Hui

[PATCH v2] usb: xhci: print xhci->xhc_state when queue_command failed

Posted by Su Hui 2 months, 1 week ago

When encounters some errors like these:
xhci_hcd 0000:4a:00.2: xHCI dying or halted, can't queue_command
xhci_hcd 0000:4a:00.2: FIXME: allocate a command ring segment
usb usb5-port6: couldn't allocate usb_device

It's hard to know whether xhc_state is dying or halted. So it's better
to print xhc_state's value which can help locate the resaon of the bug.

Signed-off-by: Su Hui <suhui@nfschina.com>
---
v2:
 - Print xhci->xhc_state with hex style.

 drivers/usb/host/xhci-ring.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 94c9c9271658..131e7530ec4a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -4372,7 +4372,8 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
 
 	if ((xhci->xhc_state & XHCI_STATE_DYING) ||
 		(xhci->xhc_state & XHCI_STATE_HALTED)) {
-		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command\n");
+		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command. state: 0x%x\n",
+			 xhci->xhc_state);
 		return -ESHUTDOWN;
 	}
 
-- 
2.30.2

Re: [PATCH v2] usb: xhci: print xhci->xhc_state when queue_command failed

Posted by Michał Pecio 2 months, 1 week ago

Hi,

On Fri, 25 Jul 2025 14:01:18 +0800, Su Hui wrote:
> When encounters some errors like these:
> xhci_hcd 0000:4a:00.2: xHCI dying or halted, can't queue_command
> xhci_hcd 0000:4a:00.2: FIXME: allocate a command ring segment
> usb usb5-port6: couldn't allocate usb_device
> 
> It's hard to know whether xhc_state is dying or halted.

Is it truly a problem? This is the only place which sets
XHCI_STATE_DYING that I found in the whole drivers/ tree:

        xhci_err(xhci, "xHCI host controller not responding, assume dead\n");
        xhci->xhc_state |= XHCI_STATE_DYING;

And AFAIK such state can only be exited by unbinding the driver.
Are there really cases when it's unclear if the HC is dying or not?

> So it's better to print xhc_state's value which can help locate the
> resaon of the bug.

Hmm, any chance you came across bugs that upstream should know about?

Regards,
Michal

Re: [PATCH v2] usb: xhci: print xhci->xhc_state when queue_command failed

Posted by Su Hui 2 months, 1 week ago

On 2025/7/25 18:03, Michał Pecio wrote:
> Hi,
>
> On Fri, 25 Jul 2025 14:01:18 +0800, Su Hui wrote:
>> When encounters some errors like these:
>> xhci_hcd 0000:4a:00.2: xHCI dying or halted, can't queue_command
>> xhci_hcd 0000:4a:00.2: FIXME: allocate a command ring segment
>> usb usb5-port6: couldn't allocate usb_device
>>
>> It's hard to know whether xhc_state is dying or halted.
> Is it truly a problem? This is the only place which sets
> XHCI_STATE_DYING that I found in the whole drivers/ tree:
>
>          xhci_err(xhci, "xHCI host controller not responding, assume dead\n");
>          xhci->xhc_state |= XHCI_STATE_DYING;
>
> And AFAIK such state can only be exited by unbinding the driver.
> Are there really cases when it's unclear if the HC is dying or not?
Oh, my fault, I ignored this so obvious error message. :(.
Sorry for the noise, Maybe this patch should be removed.
>
>> So it's better to print xhc_state's value which can help locate the
>> resaon of the bug.
>>
>> Hmm, any chance you came across bugs that upstream should know about?
Actually, this bug is specific to the 5.4 version of the kernel and a 
particular USB camera. I am working
to resolve this issue. When the xhci_hcd is initialized, the driver sets 
xhc_state to "halted", but before
the xhci_hcd calls xhci_start, the hub starts Initializing. Hub 
initialization failed due to xhc_state being
halted. Perhaps this issue is caused by hardware...

Su Hui

Re: [PATCH v2] usb: xhci: print xhci->xhc_state when queue_command failed

Posted by Michał Pecio 2 months, 1 week ago

On Fri, 25 Jul 2025 19:32:37 +0800, Su Hui wrote:
> On 2025/7/25 18:03, Michał Pecio wrote:
> >> Hmm, any chance you came across bugs that upstream should know about?  
> Actually, this bug is specific to the 5.4 version of the kernel and a 
> particular USB camera. I am working
> to resolve this issue. When the xhci_hcd is initialized, the driver sets 
> xhc_state to "halted", but before
> the xhci_hcd calls xhci_start, the hub starts Initializing. Hub 
> initialization failed due to xhc_state being
> halted. Perhaps this issue is caused by hardware...

Trying to queue commands before the chip is ready sounds like a SW bug.
5.4 is ancient (and EOL soon), newer releases may have this bug fixed.

Re: [PATCH v2] usb: xhci: print xhci->xhc_state when queue_command failed

Posted by Greg KH 2 months, 1 week ago

On Fri, Jul 25, 2025 at 02:01:18PM +0800, Su Hui wrote:
> When encounters some errors like these:
> xhci_hcd 0000:4a:00.2: xHCI dying or halted, can't queue_command
> xhci_hcd 0000:4a:00.2: FIXME: allocate a command ring segment
> usb usb5-port6: couldn't allocate usb_device
> 
> It's hard to know whether xhc_state is dying or halted. So it's better
> to print xhc_state's value which can help locate the resaon of the bug.
> 
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
> v2:
>  - Print xhci->xhc_state with hex style.
> 
>  drivers/usb/host/xhci-ring.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 94c9c9271658..131e7530ec4a 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -4372,7 +4372,8 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
>  
>  	if ((xhci->xhc_state & XHCI_STATE_DYING) ||
>  		(xhci->xhc_state & XHCI_STATE_HALTED)) {
> -		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command\n");
> +		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command. state: 0x%x\n",
> +			 xhci->xhc_state);
>  		return -ESHUTDOWN;
>  	}
>  
> -- 
> 2.30.2
> 

Simple enough, let me take this now as I want to close my tree...

thanks,

greg k-h