usb: dwc3: debug: Print register name
Hi,
Quoted 37 lines · Tejas Joglekar
From: Thinh Nguyen <[email protected]>
This commit adds a new debugging option
CONFIG_USB_DWC3_DEBUG_REG_PRINTto enable printing of register names to tracepoints for register read/write.Sample trace:
----------283.675504: dwc3_writel: DEPCMDPAR0 addr ffffc9000ba1c838 value 00000000 283.675505: dwc3_writel: DEPCMDPAR1 addr ffffc9000ba1c834 value 00000000 283.675505: dwc3_writel: DEPCMDPAR2 addr ffffc9000ba1c830 value 00000000 283.675506: dwc3_writel: DEPCMD addr ffffc9000ba1c83c value 00030d08 283.675506: dwc3_readl: DEPCMD addr ffffc9000ba1c83c value 00030d08 283.675509: dwc3_readl: DEPCMD addr ffffc9000ba1c83c value 00030908 283.675512: dwc3_gadget_ep_cmd: ep1in: cmd 'End Transfer' [199944] params 00000000 00000000 00000000 --> status: Successful 283.675524: dwc3_readl: GEVNTCOUNT addr ffffc9000ba1c40c value 00000004 283.675526: dwc3_readl: GEVNTSIZ addr ffffc9000ba1c408 value 00001000 283.675528: dwc3_writel: GEVNTSIZ addr ffffc9000ba1c408 value 80001000 283.675529: dwc3_writel: GEVNTCOUNT addr ffffc9000ba1c40c value 00000004 283.675614: dwc3_readl: DALEPENA addr ffffc9000ba1c720 value 00000003 283.675615: dwc3_writel: DALEPENA addr ffffc9000ba1c720 value 00000003----------
Signed-off-by Thinh Nguyen <[email protected]>Signed-off-by Tejas Joglekar <[email protected]>--- drivers/usb/dwc3/Kconfig | 8 +++ drivers/usb/dwc3/debug.h | 10 ++++ drivers/usb/dwc3/debugfs.c | 139 +++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/dwc3/trace.h | 7 ++- 4 files changed, 162 insertions(+), 2 deletions(-)drivers/usb/dwc3/Kconfig +0
-0
drivers/usb/dwc3/Kconfig config USB_DWC3_DUAL_ROLE · line 57
Quoted 10 lines
drivers/usb/dwc3/Kconfig +7
-0@@ -48,6 +48,14 @@ config USB_DWC3_DUAL_ROLE endchoice +config USB_DWC3_DEBUG_REG_PRINT + bool "Enable register name printing to the trace" + depends on (USB_DWC3 && DEBUG_FS) + default n + help + Select this option if you want to enable trace logging with + register name prints on register read and write.
Why is this a compile-time option?
Quoted 7 lines
drivers/usb/dwc3/debug.h +0
-0@@ -75,6 +75,16 @@ dwc3_gadget_generic_cmd_string(u8 cmd) } }
drivers/usb/dwc3/debug.h dwc3_gadget_generic_cmd_string(u8 cmd) · line 86
Quoted 10 lines
drivers/usb/dwc3/debug.h +9
-0+#ifdef CONFIG_USB_DWC3_DEBUG_REG_PRINT +const char *dwc3_gadget_register_string (u16 offset); +#else +static inline const char * +dwc3_gadget_register_string (u16 offset) +{ +return "" ; +} +#endif
no ifdefs, please.
Quoted 13 lines
drivers/usb/dwc3/debugfs.c +5
-0@@ -734,6 +734,145 @@ static void dwc3_debugfs_create_endpoint_dirs(struct dwc3 *dwc, } } +#ifdef CONFIG_USB_DWC3_DEBUG_REG_PRINT +/** +* dwc3_gadget_register_string - returns register name +* @offset: register offset +*/
drivers/usb/dwc3/debugfs.c static void dwc3_debugfs_create_endpoint_dirs(struct dwc3 *dwc, · line 751
Quoted 10 lines
drivers/usb/dwc3/debugfs.c +10
-0+const char *dwc3_gadget_register_string (u16 offset) +{ +if (offset >=DWC3_GTXFIFOSIZ (0 ) && offset <DWC3_GTXFIFOSIZ (32 )) +return "GTXFIFOSIZ " ; +if (offset >=DWC3_GRXFIFOSIZ (0 ) && offset <DWC3_GRXFIFOSIZ (32 )) +return "GRXFIFOSIZ " ; + +switch (offset) { +case DWC3_GUSB2PHYCFG (0 ): +return "GUSB2PHYCFG(0) " ;
I really wonder if we really need this in the kernel. I mean, look at the amount of code just to convert an address into a string. Why don't we do this as a post-processing step? Should be fairly simple to write a parser for this, no?
I mean, I don't mind having the kernel do this, but it just seems like a bit overkill. One can argue that the same can be said about control requests which I've added support for decoding, but the idea was that the decoding logic would be turned into a generic library which UDCs and HCDs can use. This, on the other hand, is dwc3 specific.
Would you be open to a tool that does post-processing on dwc3 tracepoints? I can help write it out, no issues, but we need to decide what needs to go in there. I'm also open to being persuaded to accepting $subject, but I need someone to convince me that this is the best way forward.
cheers