When multiple DWC3 controllers are being used, trace events from
different instances get mixed up making debugging difficult as
there's no way to distinguish which instance generated the trace.
Append the base address of dwc3 controller to trace events, so that
the source instance is clearly identifiable.
Example trace output,
before -> dwc3_event: event (00000101): Reset [U0]
after -> dwc3_event: 0a600000: event (00000101): Reset [U0]
Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
---
drivers/usb/dwc3/core.c | 6 ++-
drivers/usb/dwc3/core.h | 2 +
drivers/usb/dwc3/ep0.c | 2 +-
drivers/usb/dwc3/gadget.c | 2 +-
drivers/usb/dwc3/io.h | 4 +-
drivers/usb/dwc3/trace.h | 88 ++++++++++++++++++++++++---------------
6 files changed, 66 insertions(+), 38 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 670a9d4bfff2..3aa85f5f1c47 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -158,7 +158,7 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy)
dwc3_writel(dwc, DWC3_GCTL, reg);
dwc->current_dr_role = mode;
- trace_dwc3_set_prtcap(mode);
+ trace_dwc3_set_prtcap(dwc, mode);
}
EXPORT_SYMBOL_GPL(dwc3_set_prtcap);
@@ -2193,6 +2193,10 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
dwc_res = *res;
dwc_res.start += DWC3_GLOBALS_REGS_START;
+ /* Store the physical base address for logging in trace */
+ snprintf(dwc->base_addr, sizeof(dwc->base_addr), "%08llx",
+ (unsigned long long)res->start);
+
if (dev->of_node) {
struct device_node *parent = of_get_parent(dev->of_node);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 23188b910528..c16e47273ea0 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1178,6 +1178,7 @@ struct dwc3_glue_ops {
* @wakeup_pending_funcs: Indicates whether any interface has requested for
* function wakeup in bitmap format where bit position
* represents interface_id.
+ * @base_addr: The HW base address of DWC3 controller.
*/
struct dwc3 {
struct work_struct drd_work;
@@ -1412,6 +1413,7 @@ struct dwc3 {
struct dentry *debug_root;
u32 gsbuscfg0_reqinfo;
u32 wakeup_pending_funcs;
+ char base_addr[9];
};
#define INCRX_BURST_MODE 0
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a8ff8db610d3..bfe616194dfa 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -833,7 +833,7 @@ static void dwc3_ep0_inspect_setup(struct dwc3 *dwc,
if (!dwc->gadget_driver || !dwc->softconnect || !dwc->connected)
goto out;
- trace_dwc3_ctrl_req(ctrl);
+ trace_dwc3_ctrl_req(dwc, ctrl);
len = le16_to_cpu(ctrl->wLength);
if (!len) {
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a85ba5ca7912..95c05e5de5b7 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -278,7 +278,7 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned int cmd,
status = -ETIMEDOUT;
}
- trace_dwc3_gadget_generic_cmd(cmd, param, status);
+ trace_dwc3_gadget_generic_cmd(dwc, cmd, param, status);
return ret;
}
diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
index 7dd0c1e0cf74..cad9a2ae1547 100644
--- a/drivers/usb/dwc3/io.h
+++ b/drivers/usb/dwc3/io.h
@@ -33,7 +33,7 @@ static inline u32 dwc3_readl(struct dwc3 *dwc, u32 offset)
* documentation, so we revert it back to the proper addresses, the
* same way they are described on SNPS documentation
*/
- trace_dwc3_readl(base - DWC3_GLOBALS_REGS_START, offset, value);
+ trace_dwc3_readl(dwc, base - DWC3_GLOBALS_REGS_START, offset, value);
return value;
}
@@ -54,7 +54,7 @@ static inline void dwc3_writel(struct dwc3 *dwc, u32 offset, u32 value)
* documentation, so we revert it back to the proper addresses, the
* same way they are described on SNPS documentation
*/
- trace_dwc3_writel(base - DWC3_GLOBALS_REGS_START, offset, value);
+ trace_dwc3_writel(dwc, base - DWC3_GLOBALS_REGS_START, offset, value);
}
#endif /* __DRIVERS_USB_DWC3_IO_H */
diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
index b6ba984bafcd..130c21f5dd1a 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -20,63 +20,70 @@
#include "debug.h"
DECLARE_EVENT_CLASS(dwc3_log_set_prtcap,
- TP_PROTO(u32 mode),
- TP_ARGS(mode),
+ TP_PROTO(struct dwc3 *dwc, u32 mode),
+ TP_ARGS(dwc, mode),
TP_STRUCT__entry(
+ __string(base_addr, dwc->base_addr)
__field(u32, mode)
),
TP_fast_assign(
+ __assign_str(base_addr);
__entry->mode = mode;
),
- TP_printk("mode %s", dwc3_mode_string(__entry->mode))
+ TP_printk("%s: mode %s", __get_str(base_addr), dwc3_mode_string(__entry->mode))
);
DEFINE_EVENT(dwc3_log_set_prtcap, dwc3_set_prtcap,
- TP_PROTO(u32 mode),
- TP_ARGS(mode)
+ TP_PROTO(struct dwc3 *dwc, u32 mode),
+ TP_ARGS(dwc, mode)
);
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, u32 offset, u32 value),
+ TP_ARGS(dwc, base, offset, value),
TP_STRUCT__entry(
+ __string(base_addr, dwc->base_addr)
__field(void *, base)
__field(u32, offset)
__field(u32, value)
),
TP_fast_assign(
+ __assign_str(base_addr);
__entry->base = base;
__entry->offset = offset;
__entry->value = value;
),
- TP_printk("addr %p offset %04x value %08x",
+ TP_printk("%s: addr %p offset %04x value %08x",
+ __get_str(base_addr),
__entry->base + __entry->offset,
__entry->offset,
__entry->value)
);
DEFINE_EVENT(dwc3_log_io, dwc3_readl,
- TP_PROTO(void __iomem *base, u32 offset, u32 value),
- TP_ARGS(base, offset, value)
+ TP_PROTO(struct dwc3 *dwc, void __iomem *base, u32 offset, u32 value),
+ TP_ARGS(dwc, base, offset, value)
);
DEFINE_EVENT(dwc3_log_io, dwc3_writel,
- TP_PROTO(void __iomem *base, u32 offset, u32 value),
- TP_ARGS(base, offset, value)
+ TP_PROTO(struct dwc3 *dwc, void __iomem *base, u32 offset, u32 value),
+ TP_ARGS(dwc, base, offset, value)
);
DECLARE_EVENT_CLASS(dwc3_log_event,
TP_PROTO(u32 event, struct dwc3 *dwc),
TP_ARGS(event, dwc),
TP_STRUCT__entry(
+ __string(base_addr, dwc->base_addr)
__field(u32, event)
__field(u32, ep0state)
),
TP_fast_assign(
+ __assign_str(base_addr);
__entry->event = event;
__entry->ep0state = dwc->ep0state;
),
- TP_printk("event (%08x): %s", __entry->event,
+ TP_printk("%s: event (%08x): %s", __get_str(base_addr), __entry->event,
dwc3_decode_event(__get_buf(DWC3_MSG_MAX), DWC3_MSG_MAX,
__entry->event, __entry->ep0state))
);
@@ -87,9 +94,10 @@ DEFINE_EVENT(dwc3_log_event, dwc3_event,
);
DECLARE_EVENT_CLASS(dwc3_log_ctrl,
- TP_PROTO(struct usb_ctrlrequest *ctrl),
- TP_ARGS(ctrl),
+ TP_PROTO(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl),
+ TP_ARGS(dwc, ctrl),
TP_STRUCT__entry(
+ __string(base_addr, dwc->base_addr)
__field(__u8, bRequestType)
__field(__u8, bRequest)
__field(__u16, wValue)
@@ -97,13 +105,15 @@ DECLARE_EVENT_CLASS(dwc3_log_ctrl,
__field(__u16, wLength)
),
TP_fast_assign(
+ __assign_str(base_addr);
__entry->bRequestType = ctrl->bRequestType;
__entry->bRequest = ctrl->bRequest;
__entry->wValue = le16_to_cpu(ctrl->wValue);
__entry->wIndex = le16_to_cpu(ctrl->wIndex);
__entry->wLength = le16_to_cpu(ctrl->wLength);
),
- TP_printk("%s", usb_decode_ctrl(__get_buf(DWC3_MSG_MAX), DWC3_MSG_MAX,
+ TP_printk("%s: %s", __get_str(base_addr), usb_decode_ctrl(__get_buf(DWC3_MSG_MAX),
+ DWC3_MSG_MAX,
__entry->bRequestType,
__entry->bRequest, __entry->wValue,
__entry->wIndex, __entry->wLength)
@@ -111,14 +121,15 @@ DECLARE_EVENT_CLASS(dwc3_log_ctrl,
);
DEFINE_EVENT(dwc3_log_ctrl, dwc3_ctrl_req,
- TP_PROTO(struct usb_ctrlrequest *ctrl),
- TP_ARGS(ctrl)
+ TP_PROTO(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl),
+ TP_ARGS(dwc, ctrl)
);
DECLARE_EVENT_CLASS(dwc3_log_request,
TP_PROTO(struct dwc3_request *req),
TP_ARGS(req),
TP_STRUCT__entry(
+ __string(base_addr, req->dep->dwc->base_addr)
__string(name, req->dep->name)
__field(struct dwc3_request *, req)
__field(unsigned int, actual)
@@ -129,7 +140,7 @@ DECLARE_EVENT_CLASS(dwc3_log_request,
__field(int, no_interrupt)
),
TP_fast_assign(
- __assign_str(name);
+ __assign_str(base_addr);
__entry->req = req;
__entry->actual = req->request.actual;
__entry->length = req->request.length;
@@ -138,8 +149,10 @@ DECLARE_EVENT_CLASS(dwc3_log_request,
__entry->short_not_ok = req->request.short_not_ok;
__entry->no_interrupt = req->request.no_interrupt;
),
- TP_printk("%s: req %p length %u/%u %s%s%s ==> %d",
- __get_str(name), __entry->req, __entry->actual, __entry->length,
+ TP_printk("%s: %s: req %p length %u/%u %s%s%s ==> %d",
+ __get_str(base_addr),
+ __get_str(name), __entry->req,
+ __entry->actual, __entry->length,
__entry->zero ? "Z" : "z",
__entry->short_not_ok ? "S" : "s",
__entry->no_interrupt ? "i" : "I",
@@ -173,28 +186,30 @@ DEFINE_EVENT(dwc3_log_request, dwc3_gadget_giveback,
);
DECLARE_EVENT_CLASS(dwc3_log_generic_cmd,
- TP_PROTO(unsigned int cmd, u32 param, int status),
- TP_ARGS(cmd, param, status),
+ TP_PROTO(struct dwc3 *dwc, unsigned int cmd, u32 param, int status),
+ TP_ARGS(dwc, cmd, param, status),
TP_STRUCT__entry(
+ __string(base_addr, dwc->base_addr)
__field(unsigned int, cmd)
__field(u32, param)
__field(int, status)
),
TP_fast_assign(
+ __assign_str(base_addr);
__entry->cmd = cmd;
__entry->param = param;
__entry->status = status;
),
- TP_printk("cmd '%s' [%x] param %08x --> status: %s",
- dwc3_gadget_generic_cmd_string(__entry->cmd),
+ TP_printk("%s: cmd '%s' [%x] param %08x --> status: %s",
+ __get_str(base_addr), dwc3_gadget_generic_cmd_string(__entry->cmd),
__entry->cmd, __entry->param,
dwc3_gadget_generic_cmd_status_string(__entry->status)
)
);
DEFINE_EVENT(dwc3_log_generic_cmd, dwc3_gadget_generic_cmd,
- TP_PROTO(unsigned int cmd, u32 param, int status),
- TP_ARGS(cmd, param, status)
+ TP_PROTO(struct dwc3 *dwc, unsigned int cmd, u32 param, int status),
+ TP_ARGS(dwc, cmd, param, status)
);
DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd,
@@ -202,6 +217,7 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd,
struct dwc3_gadget_ep_cmd_params *params, int cmd_status),
TP_ARGS(dep, cmd, params, cmd_status),
TP_STRUCT__entry(
+ __string(base_addr, dep->dwc->base_addr)
__string(name, dep->name)
__field(unsigned int, cmd)
__field(u32, param0)
@@ -210,6 +226,7 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd,
__field(int, cmd_status)
),
TP_fast_assign(
+ __assign_str(base_addr);
__assign_str(name);
__entry->cmd = cmd;
__entry->param0 = params->param0;
@@ -217,8 +234,9 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd,
__entry->param2 = params->param2;
__entry->cmd_status = cmd_status;
),
- TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s",
- __get_str(name), dwc3_gadget_ep_cmd_string(__entry->cmd),
+ TP_printk("%s: %s: cmd '%s' [%x] params %08x %08x %08x --> status: %s",
+ __get_str(base_addr), __get_str(name),
+ dwc3_gadget_ep_cmd_string(__entry->cmd),
__entry->cmd, __entry->param0,
__entry->param1, __entry->param2,
dwc3_ep_cmd_status_string(__entry->cmd_status)
@@ -235,6 +253,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
TP_PROTO(struct dwc3_ep *dep, struct dwc3_trb *trb),
TP_ARGS(dep, trb),
TP_STRUCT__entry(
+ __string(base_addr, dep->dwc->base_addr)
__string(name, dep->name)
__field(struct dwc3_trb *, trb)
__field(u32, bpl)
@@ -246,6 +265,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
__field(u32, dequeue)
),
TP_fast_assign(
+ __assign_str(base_addr);
__assign_str(name);
__entry->trb = trb;
__entry->bpl = trb->bpl;
@@ -256,8 +276,8 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
__entry->enqueue = dep->trb_enqueue;
__entry->dequeue = dep->trb_dequeue;
),
- TP_printk("%s: trb %p (E%d:D%d) buf %08x%08x size %s%d ctrl %08x sofn %08x (%c%c%c%c:%c%c:%s)",
- __get_str(name), __entry->trb, __entry->enqueue,
+ TP_printk("%s: %s: trb %p (E%d:D%d) buf %08x%08x size %s%d ctrl %08x sofn %08x (%c%c%c%c:%c%c:%s)",
+ __get_str(base_addr), __get_str(name), __entry->trb, __entry->enqueue,
__entry->dequeue, __entry->bph, __entry->bpl,
({char *s;
int pcm = ((__entry->size >> 24) & 3) + 1;
@@ -307,6 +327,7 @@ DECLARE_EVENT_CLASS(dwc3_log_ep,
TP_PROTO(struct dwc3_ep *dep),
TP_ARGS(dep),
TP_STRUCT__entry(
+ __string(base_addr, dep->dwc->base_addr)
__string(name, dep->name)
__field(unsigned int, maxpacket)
__field(unsigned int, maxpacket_limit)
@@ -318,6 +339,7 @@ DECLARE_EVENT_CLASS(dwc3_log_ep,
__field(u8, trb_dequeue)
),
TP_fast_assign(
+ __assign_str(base_addr);
__assign_str(name);
__entry->maxpacket = dep->endpoint.maxpacket;
__entry->maxpacket_limit = dep->endpoint.maxpacket_limit;
@@ -328,8 +350,8 @@ DECLARE_EVENT_CLASS(dwc3_log_ep,
__entry->trb_enqueue = dep->trb_enqueue;
__entry->trb_dequeue = dep->trb_dequeue;
),
- TP_printk("%s: mps %d/%d streams %d burst %d ring %d/%d flags %c:%c%c%c%c:%c",
- __get_str(name), __entry->maxpacket,
+ TP_printk("%s: %s: mps %d/%d streams %d burst %d ring %d/%d flags %c:%c%c%c%c:%c",
+ __get_str(base_addr), __get_str(name), __entry->maxpacket,
__entry->maxpacket_limit, __entry->max_streams,
__entry->maxburst, __entry->trb_enqueue,
__entry->trb_dequeue,
--
2.34.1On Mon, Jan 05, 2026, Prashanth K wrote:
> When multiple DWC3 controllers are being used, trace events from
> different instances get mixed up making debugging difficult as
> there's no way to distinguish which instance generated the trace.
>
> Append the base address of dwc3 controller to trace events, so that
> the source instance is clearly identifiable.
>
> Example trace output,
> before -> dwc3_event: event (00000101): Reset [U0]
> after -> dwc3_event: 0a600000: event (00000101): Reset [U0]
>
> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> ---
> drivers/usb/dwc3/core.c | 6 ++-
> drivers/usb/dwc3/core.h | 2 +
> drivers/usb/dwc3/ep0.c | 2 +-
> drivers/usb/dwc3/gadget.c | 2 +-
> drivers/usb/dwc3/io.h | 4 +-
> drivers/usb/dwc3/trace.h | 88 ++++++++++++++++++++++++---------------
> 6 files changed, 66 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 670a9d4bfff2..3aa85f5f1c47 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -158,7 +158,7 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy)
> dwc3_writel(dwc, DWC3_GCTL, reg);
>
> dwc->current_dr_role = mode;
> - trace_dwc3_set_prtcap(mode);
> + trace_dwc3_set_prtcap(dwc, mode);
> }
> EXPORT_SYMBOL_GPL(dwc3_set_prtcap);
>
> @@ -2193,6 +2193,10 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
> dwc_res = *res;
> dwc_res.start += DWC3_GLOBALS_REGS_START;
>
> + /* Store the physical base address for logging in trace */
> + snprintf(dwc->base_addr, sizeof(dwc->base_addr), "%08llx",
> + (unsigned long long)res->start);
> +
Why string?
> if (dev->of_node) {
> struct device_node *parent = of_get_parent(dev->of_node);
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 23188b910528..c16e47273ea0 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1178,6 +1178,7 @@ struct dwc3_glue_ops {
> * @wakeup_pending_funcs: Indicates whether any interface has requested for
> * function wakeup in bitmap format where bit position
> * represents interface_id.
> + * @base_addr: The HW base address of DWC3 controller.
> */
> struct dwc3 {
> struct work_struct drd_work;
> @@ -1412,6 +1413,7 @@ struct dwc3 {
> struct dentry *debug_root;
> u32 gsbuscfg0_reqinfo;
> u32 wakeup_pending_funcs;
> + char base_addr[9];
Can this be u32?
BR,
Thinh
On 1/9/2026 6:48 AM, Thinh Nguyen wrote:
> On Mon, Jan 05, 2026, Prashanth K wrote:
>> When multiple DWC3 controllers are being used, trace events from
>> different instances get mixed up making debugging difficult as
>> there's no way to distinguish which instance generated the trace.
>>
>> Append the base address of dwc3 controller to trace events, so that
>> the source instance is clearly identifiable.
>>
>> Example trace output,
>> before -> dwc3_event: event (00000101): Reset [U0]
>> after -> dwc3_event: 0a600000: event (00000101): Reset [U0]
>>
>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
>> ---
>> drivers/usb/dwc3/core.c | 6 ++-
>> drivers/usb/dwc3/core.h | 2 +
>> drivers/usb/dwc3/ep0.c | 2 +-
>> drivers/usb/dwc3/gadget.c | 2 +-
>> drivers/usb/dwc3/io.h | 4 +-
>> drivers/usb/dwc3/trace.h | 88 ++++++++++++++++++++++++---------------
>> 6 files changed, 66 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 670a9d4bfff2..3aa85f5f1c47 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -158,7 +158,7 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy)
>> dwc3_writel(dwc, DWC3_GCTL, reg);
>>
>> dwc->current_dr_role = mode;
>> - trace_dwc3_set_prtcap(mode);
>> + trace_dwc3_set_prtcap(dwc, mode);
>> }
>> EXPORT_SYMBOL_GPL(dwc3_set_prtcap);
>>
>> @@ -2193,6 +2193,10 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
>> dwc_res = *res;
>> dwc_res.start += DWC3_GLOBALS_REGS_START;
>>
>> + /* Store the physical base address for logging in trace */
>> + snprintf(dwc->base_addr, sizeof(dwc->base_addr), "%08llx",
>> + (unsigned long long)res->start);
>> +
>
> Why string?
>
>> if (dev->of_node) {
>> struct device_node *parent = of_get_parent(dev->of_node);
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 23188b910528..c16e47273ea0 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -1178,6 +1178,7 @@ struct dwc3_glue_ops {
>> * @wakeup_pending_funcs: Indicates whether any interface has requested for
>> * function wakeup in bitmap format where bit position
>> * represents interface_id.
>> + * @base_addr: The HW base address of DWC3 controller.
>> */
>> struct dwc3 {
>> struct work_struct drd_work;
>> @@ -1412,6 +1413,7 @@ struct dwc3 {
>> struct dentry *debug_root;
>> u32 gsbuscfg0_reqinfo;
>> u32 wakeup_pending_funcs;
>> + char base_addr[9];
>
> Can this be u32?
>
Sure, will change it. And also rename it to dwc3_reg_base.
Let me know if that needs to be changed.
Regards,
Prashanth K
On Fri, Jan 09, 2026, Prashanth K wrote:
>
>
> On 1/9/2026 6:48 AM, Thinh Nguyen wrote:
> > On Mon, Jan 05, 2026, Prashanth K wrote:
> >> When multiple DWC3 controllers are being used, trace events from
> >> different instances get mixed up making debugging difficult as
> >> there's no way to distinguish which instance generated the trace.
> >>
> >> Append the base address of dwc3 controller to trace events, so that
> >> the source instance is clearly identifiable.
> >>
> >> Example trace output,
> >> before -> dwc3_event: event (00000101): Reset [U0]
> >> after -> dwc3_event: 0a600000: event (00000101): Reset [U0]
> >>
> >> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> >> ---
> >> drivers/usb/dwc3/core.c | 6 ++-
> >> drivers/usb/dwc3/core.h | 2 +
> >> drivers/usb/dwc3/ep0.c | 2 +-
> >> drivers/usb/dwc3/gadget.c | 2 +-
> >> drivers/usb/dwc3/io.h | 4 +-
> >> drivers/usb/dwc3/trace.h | 88 ++++++++++++++++++++++++---------------
> >> 6 files changed, 66 insertions(+), 38 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> >> index 670a9d4bfff2..3aa85f5f1c47 100644
> >> --- a/drivers/usb/dwc3/core.c
> >> +++ b/drivers/usb/dwc3/core.c
> >> @@ -158,7 +158,7 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy)
> >> dwc3_writel(dwc, DWC3_GCTL, reg);
> >>
> >> dwc->current_dr_role = mode;
> >> - trace_dwc3_set_prtcap(mode);
> >> + trace_dwc3_set_prtcap(dwc, mode);
> >> }
> >> EXPORT_SYMBOL_GPL(dwc3_set_prtcap);
> >>
> >> @@ -2193,6 +2193,10 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
> >> dwc_res = *res;
> >> dwc_res.start += DWC3_GLOBALS_REGS_START;
> >>
> >> + /* Store the physical base address for logging in trace */
> >> + snprintf(dwc->base_addr, sizeof(dwc->base_addr), "%08llx",
> >> + (unsigned long long)res->start);
> >> +
> >
> > Why string?
> >
> >> if (dev->of_node) {
> >> struct device_node *parent = of_get_parent(dev->of_node);
> >>
> >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> >> index 23188b910528..c16e47273ea0 100644
> >> --- a/drivers/usb/dwc3/core.h
> >> +++ b/drivers/usb/dwc3/core.h
> >> @@ -1178,6 +1178,7 @@ struct dwc3_glue_ops {
> >> * @wakeup_pending_funcs: Indicates whether any interface has requested for
> >> * function wakeup in bitmap format where bit position
> >> * represents interface_id.
> >> + * @base_addr: The HW base address of DWC3 controller.
> >> */
> >> struct dwc3 {
> >> struct work_struct drd_work;
> >> @@ -1412,6 +1413,7 @@ struct dwc3 {
> >> struct dentry *debug_root;
> >> u32 gsbuscfg0_reqinfo;
> >> u32 wakeup_pending_funcs;
> >> + char base_addr[9];
> >
> > Can this be u32?
> >
> Sure, will change it. And also rename it to dwc3_reg_base.
> Let me know if that needs to be changed.
>
If we want to rename it, perhaps just call it "address".
Thanks!
Thinh
On Mon, Jan 05, 2026 at 05:23:25PM +0530, Prashanth K wrote:
> When multiple DWC3 controllers are being used, trace events from
> different instances get mixed up making debugging difficult as
> there's no way to distinguish which instance generated the trace.
>
> Append the base address of dwc3 controller to trace events, so that
> the source instance is clearly identifiable.
>
> Example trace output,
> before -> dwc3_event: event (00000101): Reset [U0]
> after -> dwc3_event: 0a600000: event (00000101): Reset [U0]
>
> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> ---
> drivers/usb/dwc3/core.c | 6 ++-
> drivers/usb/dwc3/core.h | 2 +
> drivers/usb/dwc3/ep0.c | 2 +-
> drivers/usb/dwc3/gadget.c | 2 +-
> drivers/usb/dwc3/io.h | 4 +-
> drivers/usb/dwc3/trace.h | 88 ++++++++++++++++++++++++---------------
> 6 files changed, 66 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 670a9d4bfff2..3aa85f5f1c47 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -158,7 +158,7 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy)
> dwc3_writel(dwc, DWC3_GCTL, reg);
>
> dwc->current_dr_role = mode;
> - trace_dwc3_set_prtcap(mode);
> + trace_dwc3_set_prtcap(dwc, mode);
> }
> EXPORT_SYMBOL_GPL(dwc3_set_prtcap);
>
> @@ -2193,6 +2193,10 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
> dwc_res = *res;
> dwc_res.start += DWC3_GLOBALS_REGS_START;
>
> + /* Store the physical base address for logging in trace */
> + snprintf(dwc->base_addr, sizeof(dwc->base_addr), "%08llx",
> + (unsigned long long)res->start);
I think start is defined as resource_size_t, which is really
phys_addr_t, which is then either a u64 or u32, so why not just use u64
here?
And are you _sure_ it is ok to expose that to userspace?
> +
> if (dev->of_node) {
> struct device_node *parent = of_get_parent(dev->of_node);
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 23188b910528..c16e47273ea0 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1178,6 +1178,7 @@ struct dwc3_glue_ops {
> * @wakeup_pending_funcs: Indicates whether any interface has requested for
> * function wakeup in bitmap format where bit position
> * represents interface_id.
> + * @base_addr: The HW base address of DWC3 controller.
You already have this pointer to the resource in dwc3 somewhere, so why
are you storing this as a char string and not just always using the
"real" resource instead?
And also, document this as ONLY needed for debugging things, that's not
obvious here at all.
thanks,
greg k-h
On 1/6/2026 3:28 PM, Greg Kroah-Hartman wrote:
> On Mon, Jan 05, 2026 at 05:23:25PM +0530, Prashanth K wrote:
>> When multiple DWC3 controllers are being used, trace events from
>> different instances get mixed up making debugging difficult as
>> there's no way to distinguish which instance generated the trace.
>>
>> Append the base address of dwc3 controller to trace events, so that
>> the source instance is clearly identifiable.
>>
>> Example trace output,
>> before -> dwc3_event: event (00000101): Reset [U0]
>> after -> dwc3_event: 0a600000: event (00000101): Reset [U0]
>>
>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
>> ---
>> drivers/usb/dwc3/core.c | 6 ++-
>> drivers/usb/dwc3/core.h | 2 +
>> drivers/usb/dwc3/ep0.c | 2 +-
>> drivers/usb/dwc3/gadget.c | 2 +-
>> drivers/usb/dwc3/io.h | 4 +-
>> drivers/usb/dwc3/trace.h | 88 ++++++++++++++++++++++++---------------
>> 6 files changed, 66 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 670a9d4bfff2..3aa85f5f1c47 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -158,7 +158,7 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy)
>> dwc3_writel(dwc, DWC3_GCTL, reg);
>>
>> dwc->current_dr_role = mode;
>> - trace_dwc3_set_prtcap(mode);
>> + trace_dwc3_set_prtcap(dwc, mode);
>> }
>> EXPORT_SYMBOL_GPL(dwc3_set_prtcap);
>>
>> @@ -2193,6 +2193,10 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
>> dwc_res = *res;
>> dwc_res.start += DWC3_GLOBALS_REGS_START;
>>
>> + /* Store the physical base address for logging in trace */
>> + snprintf(dwc->base_addr, sizeof(dwc->base_addr), "%08llx",
>> + (unsigned long long)res->start);
>
> I think start is defined as resource_size_t, which is really
> phys_addr_t, which is then either a u64 or u32, so why not just use u64
> here?
>
Yes, we can substitute u64 instead of `unsigned long long`
+ snprintf(dwc->base_addr, sizeof(dwc->base_addr), "%08llx",
+ (u64)res->start);
> And are you _sure_ it is ok to expose that to userspace?
>
yes, because it's already part of devname in most cases.
>
>> +
>> if (dev->of_node) {
>> struct device_node *parent = of_get_parent(dev->of_node);
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 23188b910528..c16e47273ea0 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -1178,6 +1178,7 @@ struct dwc3_glue_ops {
>> * @wakeup_pending_funcs: Indicates whether any interface has requested for
>> * function wakeup in bitmap format where bit position
>> * represents interface_id.
>> + * @base_addr: The HW base address of DWC3 controller.
>
> You already have this pointer to the resource in dwc3 somewhere, so why
> are you storing this as a char string and not just always using the
> "real" resource instead?
>
No Greg, dwc3 struct doesn't have the resource pointer, but has res for
xhci.
> And also, document this as ONLY needed for debugging things, that's not
> obvious here at all.
>
Sure
Regards,
Prashanth K
On Wed, Jan 07, 2026 at 11:33:11AM +0530, Prashanth K wrote:
> On 1/6/2026 3:28 PM, Greg Kroah-Hartman wrote:
> + snprintf(dwc->base_addr, sizeof(dwc->base_addr), "%08llx",
> + (u64)res->start);
>
> > And are you _sure_ it is ok to expose that to userspace?
> >
> yes, because it's already part of devname in most cases.
Ah, so then why do you really need this? :)
> >> +
> >> if (dev->of_node) {
> >> struct device_node *parent = of_get_parent(dev->of_node);
> >>
> >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> >> index 23188b910528..c16e47273ea0 100644
> >> --- a/drivers/usb/dwc3/core.h
> >> +++ b/drivers/usb/dwc3/core.h
> >> @@ -1178,6 +1178,7 @@ struct dwc3_glue_ops {
> >> * @wakeup_pending_funcs: Indicates whether any interface has requested for
> >> * function wakeup in bitmap format where bit position
> >> * represents interface_id.
> >> + * @base_addr: The HW base address of DWC3 controller.
> >
> > You already have this pointer to the resource in dwc3 somewhere, so why
> > are you storing this as a char string and not just always using the
> > "real" resource instead?
> >
> No Greg, dwc3 struct doesn't have the resource pointer, but has res for
> xhci.
That resource has to be somewhere in the dwc3 structure, otherwise how
does the driver know how to talk to the device in the end? Or is that
just happening deep in the dwc3-platform-specific-code? And if so,
doesn't that imply that you can't just rely on a single resource for
this "address" as you don't know what the platform really does to talk
to the device?
So this leads me back to the "just use the device name" argument. Don't
try to encode a platform-type-thing into the name for everyone to be
using as that's just not going to work well. But if you totally insist
on it, please name this something else, as that's not what this really
is (i.e. "base_addr" is not correct.)
thanks,
greg k-h
On 1/7/2026 12:10 PM, Greg Kroah-Hartman wrote:
> On Wed, Jan 07, 2026 at 11:33:11AM +0530, Prashanth K wrote:
>> On 1/6/2026 3:28 PM, Greg Kroah-Hartman wrote:
>> + snprintf(dwc->base_addr, sizeof(dwc->base_addr), "%08llx",
>> + (u64)res->start);
>>
>>> And are you _sure_ it is ok to expose that to userspace?
>>>
>> yes, because it's already part of devname in most cases.
>
As Thinh suggested PCI based devices has different devname style, which
is difficult to map with correct controller instance from just trace logs.
> Ah, so then why do you really need this? :)
>
>>>> +
>>>> if (dev->of_node) {
>>>> struct device_node *parent = of_get_parent(dev->of_node);
>>>>
>>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>>> index 23188b910528..c16e47273ea0 100644
>>>> --- a/drivers/usb/dwc3/core.h
>>>> +++ b/drivers/usb/dwc3/core.h
>>>> @@ -1178,6 +1178,7 @@ struct dwc3_glue_ops {
>>>> * @wakeup_pending_funcs: Indicates whether any interface has requested for
>>>> * function wakeup in bitmap format where bit position
>>>> * represents interface_id.
>>>> + * @base_addr: The HW base address of DWC3 controller.
>>>
>>> You already have this pointer to the resource in dwc3 somewhere, so why
>>> are you storing this as a char string and not just always using the
>>> "real" resource instead?
>>>
>> No Greg, dwc3 struct doesn't have the resource pointer, but has res for
>> xhci.
>
> That resource has to be somewhere in the dwc3 structure, otherwise how
> does the driver know how to talk to the device in the end? Or is that
> just happening deep in the dwc3-platform-specific-code? And if so,
> doesn't that imply that you can't just rely on a single resource for
> this "address" as you don't know what the platform really does to talk
> to the device?
>
It does have the iomapped resource (stored in dwc3 struct), but doesn't
have the actual physical address.
> So this leads me back to the "just use the device name" argument. Don't
> try to encode a platform-type-thing into the name for everyone to be
> using as that's just not going to work well. But if you totally insist
> on it, please name this something else, as that's not what this really
> is (i.e. "base_addr" is not correct.)
>
Sorry, I guess there's a confusion here.
We are trying to print the physical address thats coming from DT/ACPI
(reg property). We are not adding the iomem/virtual address here.
I'll rename/document it for better clarity.
Regards,
Prashanth K
On Mon, Jan 05, 2026 at 05:23:25PM +0530, Prashanth K wrote: > When multiple DWC3 controllers are being used, trace events from > different instances get mixed up making debugging difficult as > there's no way to distinguish which instance generated the trace. > > Append the base address of dwc3 controller to trace events, so that > the source instance is clearly identifiable. > > Example trace output, > before -> dwc3_event: event (00000101): Reset [U0] > after -> dwc3_event: 0a600000: event (00000101): Reset [U0] Why not use the device name instead? That will be unique in the system. thanks, greg k-h
© 2016 - 2026 Red Hat, Inc.