All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Michal Pecio <michal.pecio@gmail.com>
Cc: Niklas Neronin <niklas.neronin@linux.intel.com>,
	mathias.nyman@linux.intel.com, linux-usb@vger.kernel.org
Subject: Re: [PATCH 2/7] usb: xhci: use '%pad' specifier for DMA address printing
Date: Thu, 11 Sep 2025 10:41:49 +0300	[thread overview]
Message-ID: <aMJ9PbOxn3CCuaYJ@smile.fi.intel.com> (raw)
In-Reply-To: <20250910075630.0389536f.michal.pecio@gmail.com>

On Wed, Sep 10, 2025 at 07:56:30AM +0200, Michal Pecio wrote:
> On Tue, 9 Sep 2025 22:44:16 +0200, Michal Pecio wrote:
> > On Tue, 9 Sep 2025 14:29:20 +0300, Andy Shevchenko wrote:
> > > Not really. It prints unnecessary long values on 32-bit machines
> > > making an impression that something works somewhere in 64-bit
> > > address space.  
> > 
> > The %016llx format you are alluding to is used in two error messages
> > actually seen by users, that's an issue. My crazy personal preference
> > would be %08llx, but I concede it's unprofessional, so %pad it seems.
> 
> Actually, I take this back.
> 
> I think that leading zeros are evil and I agree this message is bad.
> But I don't understand why 64 bit users should put up with this:
> 
> [  140.106751] xhci_hcd 0000:07:00.0: Event dma 0x00000000ffeec7f0 for ep 2 status 13 not part of TD at 00000000ffeec800 - 00000000ffeec800
> [  140.476573] xhci_hcd 0000:07:00.0: Event dma 0x00000000ffeec1a0 for ep 2 status 13 not part of TD at 00000000ffeec1b0 - 00000000ffeec1b0
> [  140.502855] xhci_hcd 0000:07:00.0: Event dma 0x00000000ffeecd60 for ep 2 status 13 not part of TD at 00000000ffeecd70 - 00000000ffeecd70
> [  141.225300] xhci_hcd 0000:07:00.0: Event dma 0x00000000ffeeb970 for ep 2 status 13 not part of TD at 00000000ffeeb980 - 00000000ffeeb980
> 
> when we can have this:
> 
> [  419.967755] xhci_hcd 0000:07:00.0: Event dma 0xffc34760 for ep 2 status 13 not part of TD at 0xffc34770 - 0xffc34770
> [  420.100611] xhci_hcd 0000:07:00.0: Event dma 0xffc34bc0 for ep 2 status 13 not part of TD at 0xffc34bd0 - 0xffc34bd0
> [  420.360917] xhci_hcd 0000:07:00.0: Event dma 0xffc34e70 for ep 2 status 13 not part of TD at 0xffc34e80 - 0xffc34e80
> [  421.719530] xhci_hcd 0000:07:00.0: Event dma 0xffc35770 for ep 2 status 13 not part of TD at 0xffc35780 - 0xffc35780
> 
> with a simple change (anything wrong with u64 cast here?):
> 
> -       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?

> +                (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.

> The main thing we care about here are the last 3-4 digits and we could
> have made it little more than (ep_trb_dma & 0xffff) long ago, but then
> Niklas asked "what about correlation with tracing/debugfs/dyndbg?", so
> it was left the way it is.

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2025-09-11  7:41 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-03 17:01 [PATCH 0/7] usb: xhci: enhancements to address printing Niklas Neronin
2025-09-03 17:01 ` [PATCH 1/7] usb: xhci-dbgcap: correct DMA address handling Niklas Neronin
2025-09-09 10:13   ` Michal Pecio
2025-09-10  8:00     ` Neronin, Niklas
2025-09-10  8:15       ` Michal Pecio
2025-09-03 17:01 ` [PATCH 2/7] usb: xhci: use '%pad' specifier for DMA address printing Niklas Neronin
2025-09-09  9:59   ` Michal Pecio
2025-09-09 11:29     ` Andy Shevchenko
2025-09-09 20:44       ` Michal Pecio
2025-09-10  5:56         ` Michal Pecio
2025-09-11  7:41           ` Andy Shevchenko [this message]
2025-09-11  9:34             ` Michal Pecio
2025-09-11 20:13               ` Andy Shevchenko
2025-09-12  9:46                 ` Michal Pecio
2025-09-12 18:02                   ` Andy Shevchenko
2025-09-13  8:12                     ` Michal Pecio
2025-09-15  7:20                       ` Andy Shevchenko
2025-09-15 10:22                         ` Michal Pecio
2025-09-15 12:32                           ` Neronin, Niklas
2025-09-16  9:32                             ` Michal Pecio
2025-09-16  9:36                               ` Michal Pecio
2025-09-15 14:22                           ` Andy Shevchenko
2025-09-10  9:04   ` Michal Pecio
2025-09-10  9:17     ` Michal Pecio
2025-09-03 17:01 ` [PATCH 3/7] usb: xhci: improve Stream Context register debugging Niklas Neronin
2025-09-09  9:23   ` Michal Pecio
2025-09-03 17:01 ` [PATCH 4/7] usb: xhci: improve Endpoint " Niklas Neronin
2025-09-09  9:20   ` Michal Pecio
2025-09-09 10:24     ` Michal Pecio
2025-09-15 12:45     ` Neronin, Niklas
2025-09-03 17:01 ` [PATCH 5/7] usb: xhci: improve Command Ring Control " Niklas Neronin
2025-09-09  9:17   ` Michal Pecio
2025-09-09 10:20     ` Michal Pecio
2025-09-03 17:01 ` [PATCH 6/7] usb: xhci: improve Event Ring Dequeue Pointer Register debugging Niklas Neronin
2025-09-03 17:01 ` [PATCH 7/7] usb: xhci: standardize address format Niklas Neronin
2025-09-09  9:06   ` Michal Pecio
2025-09-15 13:24     ` Neronin, Niklas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aMJ9PbOxn3CCuaYJ@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=michal.pecio@gmail.com \
    --cc=niklas.neronin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.