Re: [PATCH 5/4 RFC] An alternative dma_in_range() implementation

["Neronin, Niklas" <niklas.neronin@linux.intel.com>]

On 20/02/2025 14.14, Michał Pecio wrote:
> Here's my attempt at it, the patch goes on top of the whole series. It
> compiles and passes basic testing (some URBs completed, some unlinked,
> no errors or other malfunction apparent).
> 
> The idea is to translate dma to trb* ASAP and work with that, because
> all data structures use trb* so the code gets shorter and less verbose.
> This approach is also easy to adapt to changes in handle_tx_event(),
> for example it could trivially return trb* instead of seg* or take trb*
> (with or without accompanying seg*) and return bool or even seg*.
> 
> I tried to make the common case (start_seg == end_seg) fast, both for
> hit and miss. 5 comparisons if the range wraps around, 4 if it doesn't.
> 

Thanks, I have a similar version. If it's alright with you, I'll try to
combine your version and mine into one and add it to trb_in_td-v2.

Thanks,
Niklas

> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 600842425f6d..b18e7fd7d01e 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -300,42 +300,36 @@ static struct xhci_segment *dma_in_range(struct xhci_segment *start_seg, union x
>  					 dma_addr_t dma)
>  {
>  	struct xhci_segment *seg = start_seg;
> +	union xhci_trb *trb;
>  
> -	if (start_seg == end_seg) {
> -		if (start_trb <= end_trb) {
> -			if (xhci_trb_virt_to_dma(start_seg, start_trb) <= dma &&
> -			    dma <= xhci_trb_virt_to_dma(end_seg, end_trb))
> -				return seg;
> -			return NULL;
> +	/* check if dma is a TRB in start_seg */
> +	if (in_range(dma, seg->dma, TRB_SEGMENT_SIZE)) {
> +		trb = seg->trbs + (dma - seg->dma) / sizeof(*trb);
> +
> +		if (trb >= start_trb)
> +			/* check if the range covers it and we are done */
> +			return (end_seg != seg || trb <= end_trb) ? seg : NULL;
> +
> +		/* check if the range circles back to the beginning of start_seg */
> +		return (end_seg == seg && end_trb < start_trb && trb <= end_trb) ? seg : NULL;
> +	}
> +
> +	/* stop if the range doesn't pass through any other segment */
> +	if (end_seg == seg && (end_trb >= start_trb || seg->next == seg))
> +		return NULL;
> +
> +	/* search remaining segments knowing that start_trb isn't there */
> +	do {
> +		seg = seg->next;
> +
> +		if (in_range(dma, seg->dma, TRB_SEGMENT_SIZE)) {
> +			trb = seg->trbs + (dma - seg->dma) / sizeof(*trb);
> +
> +			return (seg != end_seg || trb <= end_trb) ? seg : NULL;
>  		}
> +	} while (seg != end_seg && seg != start_seg);
>  
> -		/* Edge case, the TD wrapped around to the start segment. */
> -		if (xhci_trb_virt_to_dma(end_seg, end_trb) < dma &&
> -		    dma < xhci_trb_virt_to_dma(start_seg, start_trb))
> -			return NULL;
> -		if (seg->dma <= dma && dma <= (seg->dma + TRB_SEGMENT_SIZE))
> -			return seg;
> -		seg = seg->next;
> -	}
> -
> -	/* Loop through segment which don't contain the DMA address. */
> -	while (dma < seg->dma || (seg->dma + TRB_SEGMENT_SIZE) <= dma) {
> -		if (seg == end_seg)
> -			return NULL;
> -
> -		seg = seg->next;
> -		if (seg == start_seg)
> -			return NULL;
> -	}
> -
> -	if (seg == start_seg) {
> -		if (dma < xhci_trb_virt_to_dma(start_seg, start_trb))
> -			return NULL;
> -	} else if (seg == end_seg) {
> -		if (xhci_trb_virt_to_dma(end_seg, end_trb) < dma)
> -			return NULL;
> -	}
> -	return seg;
> +	return NULL;
>  }
>  
>  static struct xhci_segment *trb_in_td(struct xhci_td *td, dma_addr_t dma)