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

[Michal Pecio <michal.pecio@gmail.com>]

On Thu, 11 Sep 2025 23:13:54 +0300, Andy Shevchenko wrote:
> On Thu, Sep 11, 2025 at 11:34:51AM +0200, Michal Pecio wrote:
> > 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.

I see no reason to bother padding pointers to full variable width and
when I run 'dmesg' on my 64 bit machine I see that most of the kernel
doesn't really bother either, so xhci isn't any outlier.

(Plus: why should we stop at pointers? Integers too have a width.)

It amounts to embedding static type information in logs. Maybe there
are cases where it could be helpful for people reading the log, maybe
there aren't, but this patch doesn't even attempt to make such case,
it just talks vaguely about "correctness".

I only see one truly incorrect case fixed here, a missing (u64) cast
for %llx format, which I presume will print garbage on 32 bits.


This brings up another problem with %pad: it is unknown to compilers
so they don't type check it. I can make the above bug a build error:

drivers/usb/host/xhci-ring.c: In function ‘xhci_move_dequeue_past_td’:
drivers/usb/host/xhci-ring.c:784:45: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 4 has type ‘dma_addr_t’ {aka ‘unsigned int’} [-Werror=format=]
  784 |                        "Set TR Deq ptr 0x%llx, cycle %u", addr, new_cycle);
      |                                          ~~~^             ~~~~
      |                                             |             |
      |                                             |             dma_addr_t {aka unsigned int}
      |                                             long long unsigned int
      |                                          %x


but if I switch to %pad and later change my mind and extend 'addr' to
u64 without updating this format, the compiler will eat it up and once
again garbage will be printed on some systems.

Regards,
Michal