Re: [PATCH v2 03/11] usb: xhci: move command ring pointer write

[Sergey Shtylyov <sergei.shtylyov@gmail.com>]

On 4/16/25 4:45 PM, Niklas Neronin wrote:

> Move command ring pointer write from xhci_mem_init() to xhci_init(),
> and utilize the xhci_set_cmd_ring_deq() function.
> 
> The xhci_set_cmd_ring_deq() function is nearly identical to the Command
> Ring Control register code in xhci_mem_init(). The only notable change is
> the use of:
>   xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg, xhci->cmd_ring->dequeue)
> instead of:
>   xhci->cmd_ring->first_seg->dma
> but they are effectively the same in this context. The former represents
> the exact position of the dequeue pointer, while the latter is the first
> DMA in the first segment. Before use, the dequeue pointer is at the first
> DMA in the first segment.
> 
> The xhci_set_cmd_ring_deq() function is moved without modification, except
> for (long unsigned long) -> (unsigned long long) due to checkpatch.pl.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>  drivers/usb/host/xhci-mem.c | 10 ----------
>  drivers/usb/host/xhci.c     | 37 ++++++++++++++++++++-----------------
>  2 files changed, 20 insertions(+), 27 deletions(-)

[...]
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 4de51ba851d8..92c32a80acae 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -495,6 +495,23 @@ static void xhci_enable_max_dev_slots(struct xhci_hcd *xhci)
>  	writel(config_reg, &xhci->op_regs->config_reg);
>  }
>  
> +static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
> +{
> +	u64	val_64;
> +
> +	/* step 2: initialize command ring buffer */
> +	val_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
> +	val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
> +		(xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
> +					xhci->cmd_ring->dequeue) &
> +			(u64) ~CMD_RING_RSVD_BITS) |
> +		xhci->cmd_ring->cycle_state;

   Very strange alignment scheme here...

> +	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
> +			"// Setting command ring address to 0x%llx",

   I thought you're dropping those //?

> +			(unsigned long long) val_64);
> +	xhci_write_64(xhci, val_64, &xhci->op_regs->cmd_ring);
> +}
> +
>  /*
>   * Initialize memory for HCD and xHC (one-time init).
>   *
[...]
MBR, Sergey