[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5b30f63a-5999-48f1-972f-93f02fcc0ec2@oss.qualcomm.com>
Date: Mon, 1 Sep 2025 15:26:58 +0530
From: Prashanth K <prashanth.k@....qualcomm.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: dwc3: Log dwc3 instance name in traces
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@....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
Powered by blists - more mailing lists