Re: [PATCH v4 3/3] usb: dwc3: Log dwc3 address in traces

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Fri, Jan 16, 2026, Prashanth K wrote:
> 
> 
> On 1/15/2026 9:52 PM, Thinh Nguyen wrote:
> > On Thu, Jan 15, 2026, Greg Kroah-Hartman wrote:
> >> On Wed, Jan 14, 2026 at 11:54:03PM +0000, Thinh Nguyen wrote:
> >>> On Wed, Jan 14, 2026, Thinh Nguyen wrote:
> >>>> On Wed, Jan 14, 2026, Greg Kroah-Hartman wrote:
> >>>>> On Wed, Jan 14, 2026 at 03:37:48PM +0530, Prashanth K wrote:
> >>>>>> + * @address: Cached lower 32-bit base address to be used for logging.
> >>>>>
> >>>>> Why are 32bits enough / ok?  Why not use the full 64 that you really
> >>>>> have?  What happens if you have 2 devices with just the upper 32 bits
> >>>>> different?
> >>>>>
> >>>>> This is a resource value, so why not use the proper type for it?
> >>>>>
> >>>>
> >>>> This is only intented to be used for logging, so I suggested to use u32.
> >>>> I want to avoid treating this struct member as a phys_addr_t where it
> >>>> may be misused.
> >>>>
> >>>> As for the reason to capture only the lower 32-bit, it's just base on
> >>>> what I've seen so far. That I have not seen designs where the 2 or more
> >>>> instances are placed that far apart and share the same lower 32-bit.
> >>>> It's a bit nicer to shorten the address print at the start of a
> >>>> tracepoint. But if it's insufficient, there's no problem with using
> >>>> 64-bit.
> >>>>
> >>>
> >>> Or we can just remove this and print the address from
> >>> dwc->xhci_resources[0].start.
> >>
> >> I thought I asked for that a few revisions ago :)
> > 
> > Ah, I missed that.
> > 
> >>
> >> I'd prefer that, instead of saving off a value that you can look up if
> >> you need it.
> >>
> > 
> > Yes, this is better.
> > 
> > Hi Prashanth, can we just use dwc->xhci_resources[0].start instead?
> > 
> 
> While its true that we can avoid adding new variable into dwc3 struct,
> using 'xhci_resources[0].start' in DWC3 core traces can be confusing for
> someones reading code, since all of the traces are related to dwc3 core
> and gadget.
> 

We can name the fast assign field in tracing to base_address. For those
who do not have access to the databook to know that that's where base
address is, if needed, we can also add a comment there.

Would something like this work for you?

diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
index b6ba984bafcd..8e5d474fd54a 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -37,19 +37,22 @@ DEFINE_EVENT(dwc3_log_set_prtcap, dwc3_set_prtcap,
 );
 
 DECLARE_EVENT_CLASS(dwc3_log_io,
-	TP_PROTO(void *base, u32 offset, u32 value),
-	TP_ARGS(base, offset, value),
+	TP_PROTO(struct dwc3 *dwc, void *base, u31 offset, u32 value),
+	TP_ARGS(dwc, base, offset, value),
 	TP_STRUCT__entry(
+		__field(phy_addr_t, base_address)
 		__field(void *, base)
 		__field(u32, offset)
 		__field(u32, value)
 	),
 	TP_fast_assign(
+		__entry->base_address = dwc->xhci_resources[0].start;
 		__entry->base = base;
 		__entry->offset = offset;
 		__entry->value = value;
 	),
-	TP_printk("addr %p offset %04x value %08x",
+	TP_printk("%pa: addr %p offset %04x value %08x",
+		dwc->base_address,
 		__entry->base + __entry->offset,
 		__entry->offset,
 		__entry->value)

Thanks,
Thinh