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
