On 13/03/2025 10.46, Michał Pecio wrote: > On Tue, 11 Mar 2025 23:41:39 +0100, Michał Pecio wrote: >> [102711.994235] xhci_hcd 0000:02:00.0: Event dma 0x00000000ffef4a50 for ep 6 status 4 not part of TD at 00000000eb22b790 - 00000000eb22b790 >> [102711.994243] xhci_hcd 0000:02:00.0: Ring seg 0 dma 0x00000000ffef4000 >> [102711.994246] xhci_hcd 0000:02:00.0: Ring seg 1 dma 0x00000000ffeee000 >> [102711.994249] xhci_hcd 0000:02:00.0: Ring seg 2 dma 0x00000000ffc4e000 >> >> [ ... 735 lines omitted for brevity ... ] >> >> [102711.995935] xhci_hcd 0000:02:00.0: Ring seg 738 dma 0x00000000eb2e2000 >> [102711.995937] xhci_hcd 0000:02:00.0: Ring seg 739 dma 0x00000000eb22b000 > > And what are your thoughts about this noise? It's absurd to print such > long debug dumps under failure conditions (and hold a spinlock for 2ms > to do so), and I would argue that it is pointless even normally: > > 1. Almost always exactly two segments exist, and either > a. the event and the TD are in the same segment, so who cares where > the other segment is > b. they are in different segments, and you can deduce both segments > from dma pointers so the dump tells you absolutely nothing new > > 2. With more segments, the dump can tell if there were other segments > between the event and the TD, but is it really important? > > 3. It might help with finding out-of-ring events, but this is extremely > rare and should be done automatically (xhci_dma_to_trb() or similar). > > > Bottom line, the driver never printed it and no one other than Niklas > (Cc) seemed to really miss such a feature. IMO the driver used to print a long and repetitive debug message, which is why I changed it. Admittedly, my design does not handle hundreds of segments well. Before: For each segment or until the segment containing the TD end TRB: "Looking for event-dma %016llx trb-start %016llx trb-end %016llx seg-start %016llx seg-end %016llx" After: "Event dma %pad for ep %d status %d not part of TD at %016llx - %016llx" For each segment: "Ring seg %u dma %pad" Probably, would have been better to loop from TD start seg to end seg. > > I would be inclined to submit a small patch which removes this segment > dump, as I have already done so locally. Or at least make it xhci_dbg() > if somebody can present a convincing case for having it around. My patch was only meant to move the debugging out of trb_in_td() and shorten it. Before, trb_in_td() was called twice, once for its primary functionality and a second time solely for debugging purposes. This was what I wanted to remove. Otherwise, I don't object to modifying or removing the debugs. Best Regards, Niklas > > Note that debugfs exists and provides this and much more information, > at least so long as the class driver doesn't disable the endpoint. >
