Message-ID: <f9e9b906-f3da-421a-b8b1-928534ad5703@oss.qualcomm.com>
Date: Thu, 11 Sep 2025 10:16:36 +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 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

Powered by blists - more mailing lists