On 05/03/2025 10.46, Michał Pecio wrote:
>> +debug_finding_td:
>> + xhci_err(xhci, "Transfer event %u ep %d dma %016llx not part of current TD start %016llx end %016llx\n",
>> + trb_comp_code, ep_index, (unsigned long long)ep_trb_dma,
>> + (unsigned long long)xhci_trb_virt_to_dma(td->start_seg, td->start_trb),
>> + (unsigned long long)xhci_trb_virt_to_dma(td->end_seg, td->end_trb)); +
>> + xhci_for_each_ring_seg(ep_ring->first_seg, ep_seg) {
>> + xhci_warn(xhci, "Ring seg %u trb start %016llx end %016llx\n", ep_seg->num,
>> + (unsigned long long)ep_seg->dma,
>> + (unsigned long long)(ep_seg->dma + TRB_SEGMENT_SIZE));
>> + }
>> + return -ESHUTDOWN;
>
> Cleaning up trb_in_td() is obviously the right thing to do, but one
> thing I always disliked about this message is how long and verbose it
> is. Not sure if dumping all ring segments is useful here, seg->dma can
> generally be deduced by looking at the DMA pointers involved.
>
> As far as improvements go, IMO it would be much more useful to decode
> those pointers into seg-number/trb-index pairs. I wrote a PoC and the
> result is quite encouraging, I may submit it if there is interest.
>
Agreed, printing the segment number and TRB index is a better approach.
This is my plan for the future, but in my opinion, the format for common
values should be consistent. Therefore, the change to segment number and
index should be implemented simultaneously across all xHCI driver prints,
except for some specific places. Thus, it's not changed in this patch set.
I plan to make this change after the rework of trb_in_td(). Currently,
I am testing a new trb_in_td() version suggested by Mathias, which would
make the number/index change even more logical.
Regards,
Niklas
