Re: [PATCH 5/7] usb: xhci: improve Command Ring Control register debugging

[Michal Pecio <michal.pecio@gmail.com>]

On Wed,  3 Sep 2025 19:01:25 +0200, Niklas Neronin wrote:
> Improve the debugging output for Command Ring Control registers in the xHCI
> driver. The CRCR consist of the following fields:
>  bit 0 - Ring Cycle State
>  bit 1 - Command Stop
>  bit 2 - Command Abort
>  bit 3 - Command Ring Running
>  bits 5:4 - RsvdP.
>  bits 63:4 - TR Dequeue Pointer, is 16-byte aligned.
> 
> Instead of printing the entire 64-bit register as a single block, each
> field is now printed separately using the appropriate printk() specifiers.
> This approach enhances user interpretation and ensures the DMA address
> format is displayed accurately.
> 
> Except for bit 3, reading the other bits will consistently return '0'.
> Therefore, only modified bits and bit 3 are printed.
> 
> xHCI specification, section 5.4.5.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>  drivers/usb/host/xhci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 5e5681c1eb4e..4526989169a3 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -499,7 +499,8 @@ static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
>  	crcr &= ~CMD_RING_CYCLE;
>  	crcr |= xhci->cmd_ring->cycle_state;
>  
> -	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Setting command ring address to 0x%llx", crcr);
> +	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Command ring deq %pad running %u cycle %u",
> +		       &deq_dma, crcr & CMD_RING_RUNNING, xhci->cmd_ring->cycle_state);

I think the word "Setting" was useful here.

This function should probably not be called on a running ring (the
spec says: writes to most field are ignored in this state). If there
is concern that it may happen, but not enough concern to justify some
xhci_err(), then maybe

    "... (ring%s running)", ..., RUNNING ? "" : " not");

>  	xhci_write_64(xhci, crcr, &xhci->op_regs->cmd_ring);
>  }
>  
> -- 
> 2.50.1
>