[PATCH] usb: dwc3: Log dwc3 instance name in traces

[PATCH] usb: dwc3: Log dwc3 instance name in traces

[Prashanth K]

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 device name to trace events to clearly identify the
source instance.

Example trace output,
before ->  dwc3_event: event (00000101): Reset [U0]
after  ->  dwc3_event: a600000.usb: event (00000101): Reset [U0]

Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
---
 drivers/usb/dwc3/ep0.c    |  2 +-
 drivers/usb/dwc3/gadget.c |  2 +-
 drivers/usb/dwc3/gadget.h |  1 +
 drivers/usb/dwc3/io.h     | 12 ++++---
 drivers/usb/dwc3/trace.h  | 76 ++++++++++++++++++++++++---------------
 5 files changed, 59 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 666ac432f52d..b814bbba18ac 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -830,7 +830,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 25db36c63951..e3621cc318ea 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -271,7 +271,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/gadget.h b/drivers/usb/dwc3/gadget.h
index d73e735e4081..dc9985523ed8 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -131,6 +131,7 @@ int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index);
 static inline void dwc3_gadget_ep_get_transfer_index(struct dwc3_ep *dep)
 {
 	u32			res_id;
+	struct dwc3		*dwc = dep->dwc;
 
 	res_id = dwc3_readl(dep->regs, DWC3_DEPCMD);
 	dep->resource_index = DWC3_DEPCMD_GET_RSC_IDX(res_id);
diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
index 1e96ea339d48..8e8eb3265676 100644
--- a/drivers/usb/dwc3/io.h
+++ b/drivers/usb/dwc3/io.h
@@ -16,7 +16,11 @@
 #include "debug.h"
 #include "core.h"
 
-static inline u32 dwc3_readl(void __iomem *base, u32 offset)
+/* Note: Caller must have a reference to dwc3 structure */
+#define dwc3_readl(b, o) __dwc3_readl(dwc, b, o)
+#define dwc3_writel(b, o, v) __dwc3_writel(dwc, b, o, v)
+
+static inline u32 __dwc3_readl(struct dwc3 *dwc, void __iomem *base, u32 offset)
 {
 	u32 value;
 
@@ -32,12 +36,12 @@ static inline u32 dwc3_readl(void __iomem *base, 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;
 }
 
-static inline void dwc3_writel(void __iomem *base, u32 offset, u32 value)
+static inline void __dwc3_writel(struct dwc3 *dwc, void __iomem *base, u32 offset, u32 value)
 {
 	/*
 	 * We requested the mem region starting from the Globals address
@@ -51,7 +55,7 @@ static inline void dwc3_writel(void __iomem *base, 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 bdeb1aaf65d8..649bc536ca20 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -20,46 +20,51 @@
 #include "debug.h"
 
 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(dev_name, dev_name(dwc->dev))
 		__field(void *, base)
 		__field(u32, offset)
 		__field(u32, value)
 	),
 	TP_fast_assign(
+		__assign_str(dev_name);
 		__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(dev_name),
 		__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(dev_name, dev_name(dwc->dev))
 		__field(u32, event)
 		__field(u32, ep0state)
 	),
 	TP_fast_assign(
+		__assign_str(dev_name);
 		__entry->event = event;
 		__entry->ep0state = dwc->ep0state;
 	),
-	TP_printk("event (%08x): %s", __entry->event,
+	TP_printk("%s: event (%08x): %s", __get_str(dev_name), __entry->event,
 			dwc3_decode_event(__get_buf(DWC3_MSG_MAX), DWC3_MSG_MAX,
 					__entry->event, __entry->ep0state))
 );
@@ -70,9 +75,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(dev_name, dev_name(dwc->dev))
 		__field(__u8, bRequestType)
 		__field(__u8, bRequest)
 		__field(__u16, wValue)
@@ -80,13 +86,15 @@ DECLARE_EVENT_CLASS(dwc3_log_ctrl,
 		__field(__u16, wLength)
 	),
 	TP_fast_assign(
+		__assign_str(dev_name);
 		__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(dev_name), usb_decode_ctrl(__get_buf(DWC3_MSG_MAX),
+					DWC3_MSG_MAX,
 					__entry->bRequestType,
 					__entry->bRequest, __entry->wValue,
 					__entry->wIndex, __entry->wLength)
@@ -94,14 +102,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(dev_name, dev_name(req->dep->dwc->dev))
 		__string(name, req->dep->name)
 		__field(struct dwc3_request *, req)
 		__field(unsigned int, actual)
@@ -112,7 +121,7 @@ DECLARE_EVENT_CLASS(dwc3_log_request,
 		__field(int, no_interrupt)
 	),
 	TP_fast_assign(
-		__assign_str(name);
+		__assign_str(dev_name);
 		__entry->req = req;
 		__entry->actual = req->request.actual;
 		__entry->length = req->request.length;
@@ -121,8 +130,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(dev_name),
+		__get_str(name), __entry->req,
+		__entry->actual, __entry->length,
 		__entry->zero ? "Z" : "z",
 		__entry->short_not_ok ? "S" : "s",
 		__entry->no_interrupt ? "i" : "I",
@@ -156,28 +167,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(dev_name, dev_name(dwc->dev))
 		__field(unsigned int, cmd)
 		__field(u32, param)
 		__field(int, status)
 	),
 	TP_fast_assign(
+		__assign_str(dev_name);
 		__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(dev_name), 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,
@@ -185,6 +198,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(dev_name, dev_name(dep->dwc->dev))
 		__string(name, dep->name)
 		__field(unsigned int, cmd)
 		__field(u32, param0)
@@ -193,6 +207,7 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd,
 		__field(int, cmd_status)
 	),
 	TP_fast_assign(
+		__assign_str(dev_name);
 		__assign_str(name);
 		__entry->cmd = cmd;
 		__entry->param0 = params->param0;
@@ -200,8 +215,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(dev_name), __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)
@@ -218,6 +234,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(dev_name, dev_name(dep->dwc->dev))
 		__string(name, dep->name)
 		__field(struct dwc3_trb *, trb)
 		__field(u32, bpl)
@@ -229,6 +246,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
 		__field(u32, dequeue)
 	),
 	TP_fast_assign(
+		__assign_str(dev_name);
 		__assign_str(name);
 		__entry->trb = trb;
 		__entry->bpl = trb->bpl;
@@ -239,8 +257,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(dev_name), __get_str(name), __entry->trb, __entry->enqueue,
 		__entry->dequeue, __entry->bph, __entry->bpl,
 		({char *s;
 		int pcm = ((__entry->size >> 24) & 3) + 1;
@@ -290,6 +308,7 @@ DECLARE_EVENT_CLASS(dwc3_log_ep,
 	TP_PROTO(struct dwc3_ep *dep),
 	TP_ARGS(dep),
 	TP_STRUCT__entry(
+		__string(dev_name, dev_name(dep->dwc->dev))
 		__string(name, dep->name)
 		__field(unsigned int, maxpacket)
 		__field(unsigned int, maxpacket_limit)
@@ -301,6 +320,7 @@ DECLARE_EVENT_CLASS(dwc3_log_ep,
 		__field(u8, trb_dequeue)
 	),
 	TP_fast_assign(
+		__assign_str(dev_name);
 		__assign_str(name);
 		__entry->maxpacket = dep->endpoint.maxpacket;
 		__entry->maxpacket_limit = dep->endpoint.maxpacket_limit;
@@ -311,8 +331,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(dev_name), __get_str(name), __entry->maxpacket,
 		__entry->maxpacket_limit, __entry->max_streams,
 		__entry->maxburst, __entry->trb_enqueue,
 		__entry->trb_dequeue,
-- 
2.25.1

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Thinh Nguyen]

Hi,

On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
> source instance.

Can we print the base address instead of the device name? This will be
consistent across different device names, and it will be easier to
create filter.

> 
> Example trace output,
> before ->  dwc3_event: event (00000101): Reset [U0]
> after  ->  dwc3_event: a600000.usb: event (00000101): Reset [U0]
> 
> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/ep0.c    |  2 +-
>  drivers/usb/dwc3/gadget.c |  2 +-
>  drivers/usb/dwc3/gadget.h |  1 +
>  drivers/usb/dwc3/io.h     | 12 ++++---
>  drivers/usb/dwc3/trace.h  | 76 ++++++++++++++++++++++++---------------
>  5 files changed, 59 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index 666ac432f52d..b814bbba18ac 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -830,7 +830,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 25db36c63951..e3621cc318ea 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -271,7 +271,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/gadget.h b/drivers/usb/dwc3/gadget.h
> index d73e735e4081..dc9985523ed8 100644
> --- a/drivers/usb/dwc3/gadget.h
> +++ b/drivers/usb/dwc3/gadget.h
> @@ -131,6 +131,7 @@ int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index);
>  static inline void dwc3_gadget_ep_get_transfer_index(struct dwc3_ep *dep)
>  {
>  	u32			res_id;
> +	struct dwc3		*dwc = dep->dwc;

This looks unused when it's not.

>  
>  	res_id = dwc3_readl(dep->regs, DWC3_DEPCMD);
>  	dep->resource_index = DWC3_DEPCMD_GET_RSC_IDX(res_id);
> diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
> index 1e96ea339d48..8e8eb3265676 100644
> --- a/drivers/usb/dwc3/io.h
> +++ b/drivers/usb/dwc3/io.h
> @@ -16,7 +16,11 @@
>  #include "debug.h"
>  #include "core.h"
>  
> -static inline u32 dwc3_readl(void __iomem *base, u32 offset)
> +/* Note: Caller must have a reference to dwc3 structure */
> +#define dwc3_readl(b, o) __dwc3_readl(dwc, b, o)
> +#define dwc3_writel(b, o, v) __dwc3_writel(dwc, b, o, v)

Can we not do this. This will be hard to read. If we just print the base
address, this will be simpler.

Thanks,
Thinh

> +
> +static inline u32 __dwc3_readl(struct dwc3 *dwc, void __iomem *base, u32 offset)
>  {
>  	u32 value;
>  
> @@ -32,12 +36,12 @@ static inline u32 dwc3_readl(void __iomem *base, 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;
>  }
>  
> -static inline void dwc3_writel(void __iomem *base, u32 offset, u32 value)
> +static inline void __dwc3_writel(struct dwc3 *dwc, void __iomem *base, u32 offset, u32 value)
>  {
>  	/*
>  	 * We requested the mem region starting from the Globals address
> @@ -51,7 +55,7 @@ static inline void dwc3_writel(void __iomem *base, 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 bdeb1aaf65d8..649bc536ca20 100644
> --- a/drivers/usb/dwc3/trace.h
> +++ b/drivers/usb/dwc3/trace.h
> @@ -20,46 +20,51 @@
>  #include "debug.h"
>  
>  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(dev_name, dev_name(dwc->dev))
>  		__field(void *, base)
>  		__field(u32, offset)
>  		__field(u32, value)
>  	),
>  	TP_fast_assign(
> +		__assign_str(dev_name);
>  		__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(dev_name),
>  		__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(dev_name, dev_name(dwc->dev))
>  		__field(u32, event)
>  		__field(u32, ep0state)
>  	),
>  	TP_fast_assign(
> +		__assign_str(dev_name);
>  		__entry->event = event;
>  		__entry->ep0state = dwc->ep0state;
>  	),
> -	TP_printk("event (%08x): %s", __entry->event,
> +	TP_printk("%s: event (%08x): %s", __get_str(dev_name), __entry->event,
>  			dwc3_decode_event(__get_buf(DWC3_MSG_MAX), DWC3_MSG_MAX,
>  					__entry->event, __entry->ep0state))
>  );
> @@ -70,9 +75,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(dev_name, dev_name(dwc->dev))
>  		__field(__u8, bRequestType)
>  		__field(__u8, bRequest)
>  		__field(__u16, wValue)
> @@ -80,13 +86,15 @@ DECLARE_EVENT_CLASS(dwc3_log_ctrl,
>  		__field(__u16, wLength)
>  	),
>  	TP_fast_assign(
> +		__assign_str(dev_name);
>  		__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(dev_name), usb_decode_ctrl(__get_buf(DWC3_MSG_MAX),
> +					DWC3_MSG_MAX,
>  					__entry->bRequestType,
>  					__entry->bRequest, __entry->wValue,
>  					__entry->wIndex, __entry->wLength)
> @@ -94,14 +102,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(dev_name, dev_name(req->dep->dwc->dev))
>  		__string(name, req->dep->name)
>  		__field(struct dwc3_request *, req)
>  		__field(unsigned int, actual)
> @@ -112,7 +121,7 @@ DECLARE_EVENT_CLASS(dwc3_log_request,
>  		__field(int, no_interrupt)
>  	),
>  	TP_fast_assign(
> -		__assign_str(name);
> +		__assign_str(dev_name);
>  		__entry->req = req;
>  		__entry->actual = req->request.actual;
>  		__entry->length = req->request.length;
> @@ -121,8 +130,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(dev_name),
> +		__get_str(name), __entry->req,
> +		__entry->actual, __entry->length,
>  		__entry->zero ? "Z" : "z",
>  		__entry->short_not_ok ? "S" : "s",
>  		__entry->no_interrupt ? "i" : "I",
> @@ -156,28 +167,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(dev_name, dev_name(dwc->dev))
>  		__field(unsigned int, cmd)
>  		__field(u32, param)
>  		__field(int, status)
>  	),
>  	TP_fast_assign(
> +		__assign_str(dev_name);
>  		__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(dev_name), 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,
> @@ -185,6 +198,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(dev_name, dev_name(dep->dwc->dev))
>  		__string(name, dep->name)
>  		__field(unsigned int, cmd)
>  		__field(u32, param0)
> @@ -193,6 +207,7 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd,
>  		__field(int, cmd_status)
>  	),
>  	TP_fast_assign(
> +		__assign_str(dev_name);
>  		__assign_str(name);
>  		__entry->cmd = cmd;
>  		__entry->param0 = params->param0;
> @@ -200,8 +215,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(dev_name), __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)
> @@ -218,6 +234,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(dev_name, dev_name(dep->dwc->dev))
>  		__string(name, dep->name)
>  		__field(struct dwc3_trb *, trb)
>  		__field(u32, bpl)
> @@ -229,6 +246,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
>  		__field(u32, dequeue)
>  	),
>  	TP_fast_assign(
> +		__assign_str(dev_name);
>  		__assign_str(name);
>  		__entry->trb = trb;
>  		__entry->bpl = trb->bpl;
> @@ -239,8 +257,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(dev_name), __get_str(name), __entry->trb, __entry->enqueue,
>  		__entry->dequeue, __entry->bph, __entry->bpl,
>  		({char *s;
>  		int pcm = ((__entry->size >> 24) & 3) + 1;
> @@ -290,6 +308,7 @@ DECLARE_EVENT_CLASS(dwc3_log_ep,
>  	TP_PROTO(struct dwc3_ep *dep),
>  	TP_ARGS(dep),
>  	TP_STRUCT__entry(
> +		__string(dev_name, dev_name(dep->dwc->dev))
>  		__string(name, dep->name)
>  		__field(unsigned int, maxpacket)
>  		__field(unsigned int, maxpacket_limit)
> @@ -301,6 +320,7 @@ DECLARE_EVENT_CLASS(dwc3_log_ep,
>  		__field(u8, trb_dequeue)
>  	),
>  	TP_fast_assign(
> +		__assign_str(dev_name);
>  		__assign_str(name);
>  		__entry->maxpacket = dep->endpoint.maxpacket;
>  		__entry->maxpacket_limit = dep->endpoint.maxpacket_limit;
> @@ -311,8 +331,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(dev_name), __get_str(name), __entry->maxpacket,
>  		__entry->maxpacket_limit, __entry->max_streams,
>  		__entry->maxburst, __entry->trb_enqueue,
>  		__entry->trb_dequeue,
> -- 
> 2.25.1
> 

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Prashanth K]

On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
> Hi,
> 
> On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
>> source instance.
> 
> Can we print the base address instead of the device name? This will be
> consistent across different device names, and it will be easier to
> create filter.
> 
Did you mean to print the iomem (base address) directly?
I think using device name is more readable, in most cases device name
would contain the base address also. Let me know if you are pointing to
something else.>>
>> Example trace output,
>> before ->  dwc3_event: event (00000101): Reset [U0]
>> after  ->  dwc3_event: a600000.usb: event (00000101): Reset [U0]
>>
>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
>> ---
>>  drivers/usb/dwc3/ep0.c    |  2 +-
>>  drivers/usb/dwc3/gadget.c |  2 +-
>>  drivers/usb/dwc3/gadget.h |  1 +
>>  drivers/usb/dwc3/io.h     | 12 ++++---
>>  drivers/usb/dwc3/trace.h  | 76 ++++++++++++++++++++++++---------------
>>  5 files changed, 59 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>> index 666ac432f52d..b814bbba18ac 100644
>> --- a/drivers/usb/dwc3/ep0.c
>> +++ b/drivers/usb/dwc3/ep0.c
>> @@ -830,7 +830,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 25db36c63951..e3621cc318ea 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -271,7 +271,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/gadget.h b/drivers/usb/dwc3/gadget.h
>> index d73e735e4081..dc9985523ed8 100644
>> --- a/drivers/usb/dwc3/gadget.h
>> +++ b/drivers/usb/dwc3/gadget.h
>> @@ -131,6 +131,7 @@ int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index);
>>  static inline void dwc3_gadget_ep_get_transfer_index(struct dwc3_ep *dep)
>>  {
>>  	u32			res_id;
>> +	struct dwc3		*dwc = dep->dwc;
> 
> This looks unused when it's not.
> 
>>  
>>  	res_id = dwc3_readl(dep->regs, DWC3_DEPCMD);
>>  	dep->resource_index = DWC3_DEPCMD_GET_RSC_IDX(res_id);
>> diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
>> index 1e96ea339d48..8e8eb3265676 100644
>> --- a/drivers/usb/dwc3/io.h
>> +++ b/drivers/usb/dwc3/io.h
>> @@ -16,7 +16,11 @@
>>  #include "debug.h"
>>  #include "core.h"
>>  
>> -static inline u32 dwc3_readl(void __iomem *base, u32 offset)
>> +/* Note: Caller must have a reference to dwc3 structure */
>> +#define dwc3_readl(b, o) __dwc3_readl(dwc, b, o)
>> +#define dwc3_writel(b, o, v) __dwc3_writel(dwc, b, o, v)
> 
> Can we not do this. This will be hard to read. If we just print the base
> address, this will be simpler.
> 
> Thanks,
> Thinh
> 

This is just a wrapper macro for readl/writel APIs. I tried using
container_of in dwc3_readl/writel() to get the dwc pointer,
container_of(base, struct dwc3, regs))
but this causes trouble since we use dep->regs in some cases,
thats why i used a wrapper macro instead.

Regards,
Prashanth K

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Thinh Nguyen]

On Mon, Sep 01, 2025, Prashanth K wrote:
> 
> 
> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
> > Hi,
> > 
> > On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
> >> source instance.
> > 
> > Can we print the base address instead of the device name? This will be
> > consistent across different device names, and it will be easier to
> > create filter.
> > 
> Did you mean to print the iomem (base address) directly?
> I think using device name is more readable, in most cases device name
> would contain the base address also. Let me know if you are pointing to
> something else.>>

Yes, I mean the device base address. PCI devices won't have the base
address as part of the device name.

> >> Example trace output,
> >> before ->  dwc3_event: event (00000101): Reset [U0]
> >> after  ->  dwc3_event: a600000.usb: event (00000101): Reset [U0]
> >>
> >> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> >> ---
> >>  drivers/usb/dwc3/ep0.c    |  2 +-
> >>  drivers/usb/dwc3/gadget.c |  2 +-
> >>  drivers/usb/dwc3/gadget.h |  1 +
> >>  drivers/usb/dwc3/io.h     | 12 ++++---
> >>  drivers/usb/dwc3/trace.h  | 76 ++++++++++++++++++++++++---------------
> >>  5 files changed, 59 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> >> index 666ac432f52d..b814bbba18ac 100644
> >> --- a/drivers/usb/dwc3/ep0.c
> >> +++ b/drivers/usb/dwc3/ep0.c
> >> @@ -830,7 +830,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 25db36c63951..e3621cc318ea 100644
> >> --- a/drivers/usb/dwc3/gadget.c
> >> +++ b/drivers/usb/dwc3/gadget.c
> >> @@ -271,7 +271,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/gadget.h b/drivers/usb/dwc3/gadget.h
> >> index d73e735e4081..dc9985523ed8 100644
> >> --- a/drivers/usb/dwc3/gadget.h
> >> +++ b/drivers/usb/dwc3/gadget.h
> >> @@ -131,6 +131,7 @@ int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index);
> >>  static inline void dwc3_gadget_ep_get_transfer_index(struct dwc3_ep *dep)
> >>  {
> >>  	u32			res_id;
> >> +	struct dwc3		*dwc = dep->dwc;
> > 
> > This looks unused when it's not.
> > 
> >>  
> >>  	res_id = dwc3_readl(dep->regs, DWC3_DEPCMD);
> >>  	dep->resource_index = DWC3_DEPCMD_GET_RSC_IDX(res_id);
> >> diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
> >> index 1e96ea339d48..8e8eb3265676 100644
> >> --- a/drivers/usb/dwc3/io.h
> >> +++ b/drivers/usb/dwc3/io.h
> >> @@ -16,7 +16,11 @@
> >>  #include "debug.h"
> >>  #include "core.h"
> >>  
> >> -static inline u32 dwc3_readl(void __iomem *base, u32 offset)
> >> +/* Note: Caller must have a reference to dwc3 structure */
> >> +#define dwc3_readl(b, o) __dwc3_readl(dwc, b, o)
> >> +#define dwc3_writel(b, o, v) __dwc3_writel(dwc, b, o, v)
> > 
> > Can we not do this. This will be hard to read. If we just print the base
> > address, this will be simpler.
> > 
> > Thanks,
> > Thinh
> > 
> 
> This is just a wrapper macro for readl/writel APIs. I tried using
> container_of in dwc3_readl/writel() to get the dwc pointer,
> container_of(base, struct dwc3, regs))
> but this causes trouble since we use dep->regs in some cases,
> thats why i used a wrapper macro instead.
> 

We already have the base address in the argument. Just print that.
There's no need to use container_of.

BR,
Thinh

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Prashanth K]

On 9/3/2025 5:14 AM, Thinh Nguyen wrote:
> On Mon, Sep 01, 2025, Prashanth K wrote:
>>
>>
>> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
>>> Hi,
>>>
>>> On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
>>>> source instance.
>>>
>>> Can we print the base address instead of the device name? This will be
>>> consistent across different device names, and it will be easier to
>>> create filter.
>>>
>> Did you mean to print the iomem (base address) directly?
>> I think using device name is more readable, in most cases device name
>> would contain the base address also. Let me know if you are pointing to
>> something else.>>
> 
> Yes, I mean the device base address. PCI devices won't have the base
> address as part of the device name.
> 
But the base address (void __iomem *base) wouldn't be helpful.
Using the base address, i guess we would be able to differentiate the
traces when there are multiple instances, but it wouldn't help us
identify which controller instance generated which trace.

And for PCI devices, i agree that it doesn't have  address in device
name, but i think we should be able to identify the correct instance
based on the bus/device numbers, right ?

>>>> Example trace output,
>>>> before ->  dwc3_event: event (00000101): Reset [U0]
>>>> after  ->  dwc3_event: a600000.usb: event (00000101): Reset [U0]
>>>>
>>>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>

Regards,
Prashanth K

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Thinh Nguyen]

On Wed, Sep 03, 2025, Prashanth K wrote:
> 
> 
> On 9/3/2025 5:14 AM, Thinh Nguyen wrote:
> > On Mon, Sep 01, 2025, Prashanth K wrote:
> >>
> >>
> >> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
> >>> Hi,
> >>>
> >>> On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
> >>>> source instance.
> >>>
> >>> Can we print the base address instead of the device name? This will be
> >>> consistent across different device names, and it will be easier to
> >>> create filter.
> >>>
> >> Did you mean to print the iomem (base address) directly?
> >> I think using device name is more readable, in most cases device name
> >> would contain the base address also. Let me know if you are pointing to
> >> something else.>>
> > 
> > Yes, I mean the device base address. PCI devices won't have the base
> > address as part of the device name.
> > 
> But the base address (void __iomem *base) wouldn't be helpful.
> Using the base address, i guess we would be able to differentiate the
> traces when there are multiple instances, but it wouldn't help us
> identify which controller instance generated which trace.
> 
> And for PCI devices, i agree that it doesn't have  address in device
> name, but i think we should be able to identify the correct instance
> based on the bus/device numbers, right ?
> 

We may not have the PCI domain numbers if it's a child device as in the
case of dwc3-pci or dwc3-haps.

The base address _does_ tell you exactly which device the tracepoints
correspond to. The device name is inconsistent between different device
types and only relevant if we have access to the system to know which
name belongs to which instance.

Also, we (software engineers) are not the only consumer of this. The
the hardware team may also analyze these tracepoints. I'd think more
people are familiar with the base address rather than the naming
convention of the Linux devices.

> >>>> Example trace output,
> >>>> before ->  dwc3_event: event (00000101): Reset [U0]
> >>>> after  ->  dwc3_event: a600000.usb: event (00000101): Reset [U0]
> >>>>
> >>>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> 

BR,
Thinh

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Prashanth K]

On 9/4/2025 5:30 AM, Thinh Nguyen wrote:
> On Wed, Sep 03, 2025, Prashanth K wrote:
>>
>>
>> On 9/3/2025 5:14 AM, Thinh Nguyen wrote:
>>> On Mon, Sep 01, 2025, Prashanth K wrote:
>>>>
>>>>
>>>> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
>>>>> Hi,
>>>>>
>>>>> On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
>>>>>> source instance.
>>>>>
>>>>> Can we print the base address instead of the device name? This will be
>>>>> consistent across different device names, and it will be easier to
>>>>> create filter.
>>>>>
>>>> Did you mean to print the iomem (base address) directly?
>>>> I think using device name is more readable, in most cases device name
>>>> would contain the base address also. Let me know if you are pointing to
>>>> something else.>>
>>>
>>> Yes, I mean the device base address. PCI devices won't have the base
>>> address as part of the device name.
>>>
>> But the base address (void __iomem *base) wouldn't be helpful.
>> Using the base address, i guess we would be able to differentiate the
>> traces when there are multiple instances, but it wouldn't help us
>> identify which controller instance generated which trace.
>>
>> And for PCI devices, i agree that it doesn't have  address in device
>> name, but i think we should be able to identify the correct instance
>> based on the bus/device numbers, right ?
>>
> 
> We may not have the PCI domain numbers if it's a child device as in the
> case of dwc3-pci or dwc3-haps.
> 
> The base address _does_ tell you exactly which device the tracepoints
> correspond to. The device name is inconsistent between different device
> types and only relevant if we have access to the system to know which
> name belongs to which instance.

Yes, I agree that device name would be inconsistent for different for
PCI (and HAPS) devices. But IMO using base address (virtual) would just
make it more harder to read and identify the instance.

Perhaps we can cache the register addr and use it, what do you think?
Here we can at least differentiate the instances based on HW addr.

snprintf(dwc->inst, sizeof(dwc->inst), "0x%08llx", (unsigned long
long)res->start);
dev_info(dwc->dev, "addr:%s\n", dwc->inst);

Output --> [    4.521746] dwc3 a600000.usb: addr:0x0a600000
Regards,
Prashanth K

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Thinh Nguyen]

On Thu, Sep 04, 2025, Prashanth K wrote:
> 
> 
> On 9/4/2025 5:30 AM, Thinh Nguyen wrote:
> > On Wed, Sep 03, 2025, Prashanth K wrote:
> >>
> >>
> >> On 9/3/2025 5:14 AM, Thinh Nguyen wrote:
> >>> On Mon, Sep 01, 2025, Prashanth K wrote:
> >>>>
> >>>>
> >>>> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
> >>>>> Hi,
> >>>>>
> >>>>> On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
> >>>>>> source instance.
> >>>>>
> >>>>> Can we print the base address instead of the device name? This will be
> >>>>> consistent across different device names, and it will be easier to
> >>>>> create filter.
> >>>>>
> >>>> Did you mean to print the iomem (base address) directly?
> >>>> I think using device name is more readable, in most cases device name
> >>>> would contain the base address also. Let me know if you are pointing to
> >>>> something else.>>
> >>>
> >>> Yes, I mean the device base address. PCI devices won't have the base
> >>> address as part of the device name.
> >>>
> >> But the base address (void __iomem *base) wouldn't be helpful.
> >> Using the base address, i guess we would be able to differentiate the
> >> traces when there are multiple instances, but it wouldn't help us
> >> identify which controller instance generated which trace.
> >>
> >> And for PCI devices, i agree that it doesn't have  address in device
> >> name, but i think we should be able to identify the correct instance
> >> based on the bus/device numbers, right ?
> >>
> > 
> > We may not have the PCI domain numbers if it's a child device as in the
> > case of dwc3-pci or dwc3-haps.
> > 
> > The base address _does_ tell you exactly which device the tracepoints
> > correspond to. The device name is inconsistent between different device
> > types and only relevant if we have access to the system to know which
> > name belongs to which instance.
> 
> Yes, I agree that device name would be inconsistent for different for
> PCI (and HAPS) devices. But IMO using base address (virtual) would just
> make it more harder to read and identify the instance.
> 
> Perhaps we can cache the register addr and use it, what do you think?
> Here we can at least differentiate the instances based on HW addr.
> 
> snprintf(dwc->inst, sizeof(dwc->inst), "0x%08llx", (unsigned long
> long)res->start);
> dev_info(dwc->dev, "addr:%s\n", dwc->inst);
> 
> Output --> [    4.521746] dwc3 a600000.usb: addr:0x0a600000

I think there's some misunderstanding here. I refer the base address as
the hardware address.

I prefer something like this:

    dwc3_event: 0a600000: event (00000101): Reset [U0]

instead of the device name like this:

   dwc3_event: a600000.usb: event (00000101): Reset [U0]

BR,
Thinh

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Prashanth K]

On 9/5/2025 5:14 AM, Thinh Nguyen wrote:
> On Thu, Sep 04, 2025, Prashanth K wrote:
>>
>>
>> On 9/4/2025 5:30 AM, Thinh Nguyen wrote:
>>> On Wed, Sep 03, 2025, Prashanth K wrote:
>>>>
>>>>
>>>> On 9/3/2025 5:14 AM, Thinh Nguyen wrote:
>>>>> On Mon, Sep 01, 2025, Prashanth K wrote:
>>>>>>
>>>>>>
>>>>>> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
>>>>>>>> source instance.
>>>>>>>
>>>>>>> Can we print the base address instead of the device name? This will be
>>>>>>> consistent across different device names, and it will be easier to
>>>>>>> create filter.
>>>>>>>
>>>>>> Did you mean to print the iomem (base address) directly?
>>>>>> I think using device name is more readable, in most cases device name
>>>>>> would contain the base address also. Let me know if you are pointing to
>>>>>> something else.>>
>>>>>
>>>>> Yes, I mean the device base address. PCI devices won't have the base
>>>>> address as part of the device name.
>>>>>
>>>> But the base address (void __iomem *base) wouldn't be helpful.
>>>> Using the base address, i guess we would be able to differentiate the
>>>> traces when there are multiple instances, but it wouldn't help us
>>>> identify which controller instance generated which trace.
>>>>
>>>> And for PCI devices, i agree that it doesn't have  address in device
>>>> name, but i think we should be able to identify the correct instance
>>>> based on the bus/device numbers, right ?
>>>>
>>>
>>> We may not have the PCI domain numbers if it's a child device as in the
>>> case of dwc3-pci or dwc3-haps.
>>>
>>> The base address _does_ tell you exactly which device the tracepoints
>>> correspond to. The device name is inconsistent between different device
>>> types and only relevant if we have access to the system to know which
>>> name belongs to which instance.
>>
>> Yes, I agree that device name would be inconsistent for different for
>> PCI (and HAPS) devices. But IMO using base address (virtual) would just
>> make it more harder to read and identify the instance.
>>
>> Perhaps we can cache the register addr and use it, what do you think?
>> Here we can at least differentiate the instances based on HW addr.
>>
>> snprintf(dwc->inst, sizeof(dwc->inst), "0x%08llx", (unsigned long
>> long)res->start);
>> dev_info(dwc->dev, "addr:%s\n", dwc->inst);
>>
>> Output --> [    4.521746] dwc3 a600000.usb: addr:0x0a600000
> 
> I think there's some misunderstanding here. I refer the base address as
> the hardware address.
> 
> I prefer something like this:
> 
>     dwc3_event: 0a600000: event (00000101): Reset [U0]
> 
> instead of the device name like this:
> 
>    dwc3_event: a600000.usb: event (00000101): Reset [U0]
> 
> BR,
> Thinh

Initially I was also talking about HW address, but since we were
discussing this under dwc3_readl/writel functions context, i also got
confused whether you are pointing out the HW address or virtual address.

Anyways, i guess the above method using snprintf on res->start is one
way to get base address, is there any way to do this?

Regards,
Prashanth K

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Thinh Nguyen]

On Tue, Sep 09, 2025, Prashanth K wrote:
> 
> 
> On 9/5/2025 5:14 AM, Thinh Nguyen wrote:
> > On Thu, Sep 04, 2025, Prashanth K wrote:
> >>
> >>
> >> On 9/4/2025 5:30 AM, Thinh Nguyen wrote:
> >>> On Wed, Sep 03, 2025, Prashanth K wrote:
> >>>>
> >>>>
> >>>> On 9/3/2025 5:14 AM, Thinh Nguyen wrote:
> >>>>> On Mon, Sep 01, 2025, Prashanth K wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
> >>>>>>>> source instance.
> >>>>>>>
> >>>>>>> Can we print the base address instead of the device name? This will be
> >>>>>>> consistent across different device names, and it will be easier to
> >>>>>>> create filter.
> >>>>>>>
> >>>>>> Did you mean to print the iomem (base address) directly?
> >>>>>> I think using device name is more readable, in most cases device name
> >>>>>> would contain the base address also. Let me know if you are pointing to
> >>>>>> something else.>>
> >>>>>
> >>>>> Yes, I mean the device base address. PCI devices won't have the base
> >>>>> address as part of the device name.
> >>>>>
> >>>> But the base address (void __iomem *base) wouldn't be helpful.
> >>>> Using the base address, i guess we would be able to differentiate the
> >>>> traces when there are multiple instances, but it wouldn't help us
> >>>> identify which controller instance generated which trace.
> >>>>
> >>>> And for PCI devices, i agree that it doesn't have  address in device
> >>>> name, but i think we should be able to identify the correct instance
> >>>> based on the bus/device numbers, right ?
> >>>>
> >>>
> >>> We may not have the PCI domain numbers if it's a child device as in the
> >>> case of dwc3-pci or dwc3-haps.
> >>>
> >>> The base address _does_ tell you exactly which device the tracepoints
> >>> correspond to. The device name is inconsistent between different device
> >>> types and only relevant if we have access to the system to know which
> >>> name belongs to which instance.
> >>
> >> Yes, I agree that device name would be inconsistent for different for
> >> PCI (and HAPS) devices. But IMO using base address (virtual) would just
> >> make it more harder to read and identify the instance.
> >>
> >> Perhaps we can cache the register addr and use it, what do you think?
> >> Here we can at least differentiate the instances based on HW addr.
> >>
> >> snprintf(dwc->inst, sizeof(dwc->inst), "0x%08llx", (unsigned long
> >> long)res->start);
> >> dev_info(dwc->dev, "addr:%s\n", dwc->inst);
> >>
> >> Output --> [    4.521746] dwc3 a600000.usb: addr:0x0a600000
> > 
> > I think there's some misunderstanding here. I refer the base address as
> > the hardware address.
> > 
> > I prefer something like this:
> > 
> >     dwc3_event: 0a600000: event (00000101): Reset [U0]
> > 
> > instead of the device name like this:
> > 
> >    dwc3_event: a600000.usb: event (00000101): Reset [U0]
> > 
> > BR,
> > Thinh
> 
> Initially I was also talking about HW address, but since we were
> discussing this under dwc3_readl/writel functions context, i also got
> confused whether you are pointing out the HW address or virtual address.
> 
> Anyways, i guess the above method using snprintf on res->start is one
> way to get base address, is there any way to do this?
> 

You're right. I forgot that we can't do virt_to_phys() for ioremapped
resource...

In that case, can we pass the dwc3 structure in the dwc3_readl/writel? I
know there are many places that this change may touch, but I feel that
it's easier to read than creating the new macro dwc3_readl/writel.

Thanks,
Thinh

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Prashanth K]

On 9/11/2025 7:06 AM, Thinh Nguyen wrote:
> On Tue, Sep 09, 2025, Prashanth K wrote:
>>
>>
>> On 9/5/2025 5:14 AM, Thinh Nguyen wrote:
>>> On Thu, Sep 04, 2025, Prashanth K wrote:
>>>>
>>>>
>>>> On 9/4/2025 5:30 AM, Thinh Nguyen wrote:
>>>>> On Wed, Sep 03, 2025, Prashanth K wrote:
>>>>>>
>>>>>>
>>>>>> On 9/3/2025 5:14 AM, Thinh Nguyen wrote:
>>>>>>> On Mon, Sep 01, 2025, Prashanth K wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
>>>>>>>>>> source instance.
>>>>>>>>>
>>>>>>>>> Can we print the base address instead of the device name? This will be
>>>>>>>>> consistent across different device names, and it will be easier to
>>>>>>>>> create filter.
>>>>>>>>>
>>>>>>>> Did you mean to print the iomem (base address) directly?
>>>>>>>> I think using device name is more readable, in most cases device name
>>>>>>>> would contain the base address also. Let me know if you are pointing to
>>>>>>>> something else.>>
>>>>>>>
>>>>>>> Yes, I mean the device base address. PCI devices won't have the base
>>>>>>> address as part of the device name.
>>>>>>>
>>>>>> But the base address (void __iomem *base) wouldn't be helpful.
>>>>>> Using the base address, i guess we would be able to differentiate the
>>>>>> traces when there are multiple instances, but it wouldn't help us
>>>>>> identify which controller instance generated which trace.
>>>>>>
>>>>>> And for PCI devices, i agree that it doesn't have  address in device
>>>>>> name, but i think we should be able to identify the correct instance
>>>>>> based on the bus/device numbers, right ?
>>>>>>
>>>>>
>>>>> We may not have the PCI domain numbers if it's a child device as in the
>>>>> case of dwc3-pci or dwc3-haps.
>>>>>
>>>>> The base address _does_ tell you exactly which device the tracepoints
>>>>> correspond to. The device name is inconsistent between different device
>>>>> types and only relevant if we have access to the system to know which
>>>>> name belongs to which instance.
>>>>
>>>> Yes, I agree that device name would be inconsistent for different for
>>>> PCI (and HAPS) devices. But IMO using base address (virtual) would just
>>>> make it more harder to read and identify the instance.
>>>>
>>>> Perhaps we can cache the register addr and use it, what do you think?
>>>> Here we can at least differentiate the instances based on HW addr.
>>>>
>>>> snprintf(dwc->inst, sizeof(dwc->inst), "0x%08llx", (unsigned long
>>>> long)res->start);
>>>> dev_info(dwc->dev, "addr:%s\n", dwc->inst);
>>>>
>>>> Output --> [    4.521746] dwc3 a600000.usb: addr:0x0a600000
>>>
>>> I think there's some misunderstanding here. I refer the base address as
>>> the hardware address.
>>>
>>> I prefer something like this:
>>>
>>>     dwc3_event: 0a600000: event (00000101): Reset [U0]
>>>
>>> instead of the device name like this:
>>>
>>>    dwc3_event: a600000.usb: event (00000101): Reset [U0]
>>>
>>> BR,
>>> Thinh
>>
>> Initially I was also talking about HW address, but since we were
>> discussing this under dwc3_readl/writel functions context, i also got
>> confused whether you are pointing out the HW address or virtual address.
>>
>> Anyways, i guess the above method using snprintf on res->start is one
>> way to get base address, is there any way to do this?
>>
> 
> You're right. I forgot that we can't do virt_to_phys() for ioremapped
> resource...
> 
> In that case, can we pass the dwc3 structure in the dwc3_readl/writel? I
> know there are many places that this change may touch, but I feel that
> it's easier to read than creating the new macro dwc3_readl/writel.
> 
> Thanks,
> Thinh

1) Passing dwc3 structure to dwc3_readl/writel will need changes in
around 250 places, we can do that using 'find and replace'.

2) OR we can use container_of(base, struct dwc3, regs)) to get dwc pointer,
this will not work in few places where we use dep->regs (~6 places). we
can just create a separate function dwc3_dep_readl/writel for dep->regs,
and get dwc3 from dep. This will have lesser number of changes, and less
impact on git history.

I'm more inclined towards approach2, but fine with both approaches, let
me know which one suits here.

We can use snprintf on res->start to get the HW addr and store that
string into dwc3. Is that fine?

Regards,
Prashanth K

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Thinh Nguyen]

On Thu, Sep 11, 2025, Prashanth K wrote:
> 
> 
> On 9/11/2025 7:06 AM, Thinh Nguyen wrote:
> > On Tue, Sep 09, 2025, Prashanth K wrote:
> >>
> >>
> >> On 9/5/2025 5:14 AM, Thinh Nguyen wrote:
> >>> On Thu, Sep 04, 2025, Prashanth K wrote:
> >>>>
> >>>>
> >>>> On 9/4/2025 5:30 AM, Thinh Nguyen wrote:
> >>>>> On Wed, Sep 03, 2025, Prashanth K wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 9/3/2025 5:14 AM, Thinh Nguyen wrote:
> >>>>>>> On Mon, Sep 01, 2025, Prashanth K wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 8/29/2025 4:18 AM, Thinh Nguyen wrote:
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> On Mon, Aug 25, 2025, 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 device name to trace events to clearly identify the
> >>>>>>>>>> source instance.
> >>>>>>>>>
> >>>>>>>>> Can we print the base address instead of the device name? This will be
> >>>>>>>>> consistent across different device names, and it will be easier to
> >>>>>>>>> create filter.
> >>>>>>>>>
> >>>>>>>> Did you mean to print the iomem (base address) directly?
> >>>>>>>> I think using device name is more readable, in most cases device name
> >>>>>>>> would contain the base address also. Let me know if you are pointing to
> >>>>>>>> something else.>>
> >>>>>>>
> >>>>>>> Yes, I mean the device base address. PCI devices won't have the base
> >>>>>>> address as part of the device name.
> >>>>>>>
> >>>>>> But the base address (void __iomem *base) wouldn't be helpful.
> >>>>>> Using the base address, i guess we would be able to differentiate the
> >>>>>> traces when there are multiple instances, but it wouldn't help us
> >>>>>> identify which controller instance generated which trace.
> >>>>>>
> >>>>>> And for PCI devices, i agree that it doesn't have  address in device
> >>>>>> name, but i think we should be able to identify the correct instance
> >>>>>> based on the bus/device numbers, right ?
> >>>>>>
> >>>>>
> >>>>> We may not have the PCI domain numbers if it's a child device as in the
> >>>>> case of dwc3-pci or dwc3-haps.
> >>>>>
> >>>>> The base address _does_ tell you exactly which device the tracepoints
> >>>>> correspond to. The device name is inconsistent between different device
> >>>>> types and only relevant if we have access to the system to know which
> >>>>> name belongs to which instance.
> >>>>
> >>>> Yes, I agree that device name would be inconsistent for different for
> >>>> PCI (and HAPS) devices. But IMO using base address (virtual) would just
> >>>> make it more harder to read and identify the instance.
> >>>>
> >>>> Perhaps we can cache the register addr and use it, what do you think?
> >>>> Here we can at least differentiate the instances based on HW addr.
> >>>>
> >>>> snprintf(dwc->inst, sizeof(dwc->inst), "0x%08llx", (unsigned long
> >>>> long)res->start);
> >>>> dev_info(dwc->dev, "addr:%s\n", dwc->inst);
> >>>>
> >>>> Output --> [    4.521746] dwc3 a600000.usb: addr:0x0a600000
> >>>
> >>> I think there's some misunderstanding here. I refer the base address as
> >>> the hardware address.
> >>>
> >>> I prefer something like this:
> >>>
> >>>     dwc3_event: 0a600000: event (00000101): Reset [U0]
> >>>
> >>> instead of the device name like this:
> >>>
> >>>    dwc3_event: a600000.usb: event (00000101): Reset [U0]
> >>>
> >>> BR,
> >>> Thinh
> >>
> >> Initially I was also talking about HW address, but since we were
> >> discussing this under dwc3_readl/writel functions context, i also got
> >> confused whether you are pointing out the HW address or virtual address.
> >>
> >> Anyways, i guess the above method using snprintf on res->start is one
> >> way to get base address, is there any way to do this?
> >>
> > 
> > You're right. I forgot that we can't do virt_to_phys() for ioremapped
> > resource...
> > 
> > In that case, can we pass the dwc3 structure in the dwc3_readl/writel? I
> > know there are many places that this change may touch, but I feel that
> > it's easier to read than creating the new macro dwc3_readl/writel.
> > 
> > Thanks,
> > Thinh
> 
> 1) Passing dwc3 structure to dwc3_readl/writel will need changes in
> around 250 places, we can do that using 'find and replace'.

Yikes..

> 
> 2) OR we can use container_of(base, struct dwc3, regs)) to get dwc pointer,
> this will not work in few places where we use dep->regs (~6 places). we
> can just create a separate function dwc3_dep_readl/writel for dep->regs,

We can just update the endpoint macros to something like this:
#define DWC3_DEPCMD(n)		(DWC3_DEP_BASE(n) + 0x0c)

so we can do this:
	dwc3_readl(dwc->regs, DWC3_DEPCMD(dep->number));

We will get the proper endpoint offset, and we can also remove the dep->regs.

> and get dwc3 from dep. This will have lesser number of changes, and less
> impact on git history.
> 
> I'm more inclined towards approach2, but fine with both approaches, let
> me know which one suits here.
> 
> We can use snprintf on res->start to get the HW addr and store that
> string into dwc3. Is that fine?
> 

Option 2 sounds good.

Thanks!
Thinh

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Thinh Nguyen]

On Fri, Sep 12, 2025, Thinh Nguyen wrote:
> 
> > 
> > 2) OR we can use container_of(base, struct dwc3, regs)) to get dwc pointer,
> > this will not work in few places where we use dep->regs (~6 places). we
> > can just create a separate function dwc3_dep_readl/writel for dep->regs,
> 
> We can just update the endpoint macros to something like this:
> #define DWC3_DEPCMD(n)		(DWC3_DEP_BASE(n) + 0x0c)

Or we can just remove the DWC3_DEP_BASE and do as follow:

#define DWC3_DEPCMDPAR2(n)	(0xc800 + ((n) * 0x10))
#define DWC3_DEPCMDPAR1(n)	(0xc804 + ((n) * 0x10))
#define DWC3_DEPCMDPAR0(n)	(0xc808 + ((n) * 0x10))
#define DWC3_DEPCMD(n)		(0xc80c + ((n) * 0x10))


> 
> so we can do this:
> 	dwc3_readl(dwc->regs, DWC3_DEPCMD(dep->number));
> 

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Prashanth K]

On 9/13/2025 3:24 AM, Thinh Nguyen wrote:
>>> You're right. I forgot that we can't do virt_to_phys() for ioremapped
>>> resource...
>>>
>>> In that case, can we pass the dwc3 structure in the dwc3_readl/writel? I
>>> know there are many places that this change may touch, but I feel that
>>> it's easier to read than creating the new macro dwc3_readl/writel.
>>>
>>> Thanks,
>>> Thinh
>>
>> 1) Passing dwc3 structure to dwc3_readl/writel will need changes in
>> around 250 places, we can do that using 'find and replace'.
> 
> Yikes..
> 
>>
>> 2) OR we can use container_of(base, struct dwc3, regs)) to get dwc pointer,
>> this will not work in few places where we use dep->regs (~6 places). we
>> can just create a separate function dwc3_dep_readl/writel for dep->regs,
> 
> We can just update the endpoint macros to something like this:
> #define DWC3_DEPCMD(n)		(DWC3_DEP_BASE(n) + 0x0c)
> 
> so we can do this:
> 	dwc3_readl(dwc->regs, DWC3_DEPCMD(dep->number));
> 
> We will get the proper endpoint offset, and we can also remove the dep->regs.
> 
>> and get dwc3 from dep. This will have lesser number of changes, and less
>> impact on git history.
>>
>> I'm more inclined towards approach2, but fine with both approaches, let
>> me know which one suits here.
>>
>> We can use snprintf on res->start to get the HW addr and store that
>> string into dwc3. Is that fine?
>>
> 
> Option 2 sounds good.
> 
> Thanks!
> Thinh

I think we need to go ahead with approach 1 (Passing dwc3 structure to
dwc3_readl/writel), because we were wrong about the usage of
container_of(), it wouldn't work since it's a __iomem pointer.

I'm planning to break this into a 3 patch series
1. Removal of dep->regs and use dwc->regs everywhere
2. Use dwc pointer in all dwc3_readl/writel()
3. Adding the base addr in traces.

Regards,
Prashanth K

Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces

[Thinh Nguyen]

++Steven

On Fri, Sep 19, 2025, Prashanth K wrote:
> 
> I think we need to go ahead with approach 1 (Passing dwc3 structure to
> dwc3_readl/writel), because we were wrong about the usage of
> container_of(), it wouldn't work since it's a __iomem pointer.
> 
> I'm planning to break this into a 3 patch series
> 1. Removal of dep->regs and use dwc->regs everywhere
> 2. Use dwc pointer in all dwc3_readl/writel()
> 3. Adding the base addr in traces.
> 

Ok.

Hi Steven,

I have a question about trace event configuration. Is there a mechanism
to associate tracepoints with specific device instances when defining
trace events?

This would be useful when we want to avoid extensive driver rewrites,
for both dwc3 and xHCI devices (and potentially others).

Thanks,
Thinh