Re: [PATCH 2/7] usb: xhci: use '%pad' specifier for DMA address printing

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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