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

["Neronin, Niklas" <niklas.neronin@linux.intel.com>]

On 15/09/2025 13.22, Michal Pecio wrote:
> 
> The issue is how to print u64 and dma_addr_t, and the suggestion is
> to stay with ("%08llx", (u64)addr) for both. What should go wrong?

I agree that printing long 64-bit hex values is annoying. However,
"%08llx" is not the common format for printing DMA addresses,
and having inconsistencies is much more annoying.

Additionally, you made a good point in another review:
 "And what if the kernel starts hashing %pad by default?"
Now, all non-pad DMA addresses are not hashed. :/

I would agree on another format if it is consistently used
throughout the xHCI driver. I chose this format because it's the
most common format.
> 1. 'addr' is transparently widened if necessary
> 2. if 'addr' type changes later, nothing happens
> 3. missing cast is a build error on common platforms (needs patch)
> 4. wrong format (%lx, %d, %pad, %p) is a build error
> 
> With %pad used for dma_addr_t:
> 
> 1. different formats must be written manually
> 2a. u64 to dma_addr_t: manual edit
> 2b. dma_addr_t to u64: manual edit or it's a silent bug, invisible
>     to compilers, invisible on 64 bit platforms used by developers

Why is this conversion necessary when %pad is used for 'dma_addr_t'?
AFAIK, aside from printing, the xHCI driver does not convert
'dma_addr_t' to 'u64'.
> That's for type safety. And further:
> 5. rvalues work without proliferation of temp variables
> 6. same number looks same, whether stored as u64 or dma_addr_t
> 7. consistency with the rest of the kernel

How did you verify this?

> git grep -e '0x%08llx' -e '%#08llx' -e '%08llx' | wc -l
310

> git grep '%pad' | wc -l
446

> 
> Seriously, *lots* of drivers and even the PCI subsystem itself print
> addresses unpadded, using %llx or similar formats. The numbers have
> 8 digits on a PC (even 64 bit) and grow to 12 or more elsewhere.
> 
> It's first time I see somebody who appears really bothered by this.

Personally, when I see "0x7b271bb9ec," I think "hex value," but when I
see "0x0000007b271bb9ec," I think "address." This is because that is
how I have usually seen addresses represented. Otherwise I do prefer
the shorter format.

> 
>>> Reminder: this drivers handles DMAs as u64 too, so it will *never*
>>> print all DMAs as %pad. And if it tries, it will be a silent bug.  
>>
>> Yes, and the problem here is not in the printf() specifiers, the
>> problem is in the (used) data types.
> 
> And what else can be done? The driver uses dma_addr_t where applicable
> for efficiency on 32 bits, the HW uses 64 bits like 'buffer' below:
> 
> struct xhci_transfer_event {
>         /* 64-bit buffer address, or immediate data */
>         __le64  buffer;
>         __le32  transfer_len;
>         /* This field is interpreted differently based on the type of TRB */
>         __le32  flags;     
> };
> 
> Same address may be logged at various stages of the flow where it
> exists in variabes of different type. The number matters, not type.

I was working on implementing a helper function that would extract the DMA
address and validate it so that it can be returned as a 'dma_addr_t'.
This was supposed to be step 2, following this patch series.

Best Regard,
Niklas