On Thu, Sep 11, 2025 at 11:34:51AM +0200, Michal Pecio wrote: > On Thu, 11 Sep 2025 10:41:49 +0300, Andy Shevchenko wrote: ... > > > - xhci_err(xhci, "Event dma %pad for ep %d status %d not part of TD at %016llx - %016llx\n", > > > - &ep_trb_dma, ep_index, trb_comp_code, > > > + xhci_err(xhci, "Event dma %#08llx for ep %d status %d not part of TD at %#08llx - %#08llx\n", > > > > How is 0 will be printed with %#08x? > > Oops, thanks, this won't work indeed. > > > > + (u64) ep_trb_dma, ep_index, trb_comp_code, > > > > > > These zeros only add noise, and in many cases make difference between > > > line wrapping or not because this is longer than 99% of kernel messages > > > and some people want their terminal window not to take the whole screen. > > > > I disagree on this. The 64-bit platforms are 64-bit. If the address in use is > > _capable_ of 64-bit, it should be printed as 64-bit. Otherwise make it u32 in > > the code and then I will agree with you. > > Maybe some people unfamiliar with this driver would want to know the > width of those fields for some reason without needing to grep the code > (thuogh off the top of my head I don't know who and why). > > But when I see this line, I mainly want to know if the 1st pointer is > less than the 2nd or more than the 3rd. Padding only spreads them apart. > > I can see how padding beyond actual variable size (as in example above) > can be dangereous, because people might see it and not even think about > verifying if the code isn't truncating something. The opposite should > be less problematic. > > As for the %08llx format widespread in dynamic debug, I think it was > used in the past because it does approximately the right thing on both > types of systems and it's the only format capable of giving consistent > result on both dma_addr_t and u64, used for some DMA pointers too. The problem with it is that it can't give the proper result for the ranges that span over the 4G. Which I consider a bad thing. So, the correct use is to stick with HW register size and do appropriate specifier as it was a pointer. > If dma_addr_t really *must* be padded, then I guess it would only make > sense to also convert those u64 %08llx to %016llx. Yes. And this is the case on 64-bit platforms with device and/or main memory being resided above 4G independently if we use bounce buffers (DMA mask < address space where device or memory to transfer data is) or not. > But I see later in this series some reductions to %llx, which decision I find > puzzling. -- With Best Regards, Andy Shevchenko
