Re: ehci-dbg prints random memory (# CONFIG_DYNAMIC_DEBUG is not set)

[Rafał Miłecki]

On 13 April 2016 at 17:25, Alan Stern  wrote:
> On Wed, 13 Apr 2016, Rafał Miłecki wrote:
>> I'm trying to debug some EHCI issue so I enabled debugging by adding
>> ccflags-y := -DDEBUG
>> to the drivers/usb/host/Makefile
>>
>> Some of debugging lines contain random memory, e.g.:
>> ehci-platform ehci-platform.0: .|��`|���5P5@�3�.��*�.|��o
>>
>> It's caused by dbg_status, dbg_cmd and dbg_port calling ehci_dbg even with:
>> # CONFIG_DYNAMIC_DEBUG
>>
>> The lack of above config implies using dummy dbg_status_buf,
>> dbg_command_buf and dbg_port_buf. They don't print anything to the
>> buffer and it stays uninitialized.
>>
>> Can you give me a hint how to solve this (apart from enabling
>> CONFIG_DYNAMIC_DEBUG for my debugging which I did)? Should dbg_*_buf
>> functions return some error instead of 0? We could check for that in
>> dbg_* functions then. Unfortunately I'm not sure how it's going to
>> affect other places, e.g. fill_registers_buffer.
>
> That's a real problem.  The whole ehci-dbg.c file needs to be cleaned
> up -- we should just remove all the debugging when CONFIG_DYNAMIC_DEBUG
> isn't enabled.
>
> See if the patch below fixes the problem.

Sorry, I was a bit busy recently debugging this initial USB issue.

Your patch fixes the issue for me!

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ehci-dbg prints random memory (# CONFIG_DYNAMIC_DEBUG is not set)

[Alan Stern]

On Wed, 13 Apr 2016, Rafał Miłecki wrote:

> Hi,
> 
> I'm trying to debug some EHCI issue so I enabled debugging by adding
> ccflags-y := -DDEBUG
> to the drivers/usb/host/Makefile
> 
> Some of debugging lines contain random memory, e.g.:
> ehci-platform ehci-platform.0: .|��`|���5P5@�3�.��*�.|��o
> 
> It's caused by dbg_status, dbg_cmd and dbg_port calling ehci_dbg even with:
> # CONFIG_DYNAMIC_DEBUG
> 
> The lack of above config implies using dummy dbg_status_buf,
> dbg_command_buf and dbg_port_buf. They don't print anything to the
> buffer and it stays uninitialized.
> 
> Can you give me a hint how to solve this (apart from enabling
> CONFIG_DYNAMIC_DEBUG for my debugging which I did)? Should dbg_*_buf
> functions return some error instead of 0? We could check for that in
> dbg_* functions then. Unfortunately I'm not sure how it's going to
> affect other places, e.g. fill_registers_buffer.

That's a real problem.  The whole ehci-dbg.c file needs to be cleaned 
up -- we should just remove all the debugging when CONFIG_DYNAMIC_DEBUG 
isn't enabled.

See if the patch below fixes the problem.

Alan Stern



Index: usb-4.4/drivers/usb/host/ehci-dbg.c
===
--- usb-4.4.orig/drivers/usb/host/ehci-dbg.c
+++ usb-4.4/drivers/usb/host/ehci-dbg.c
@@ -56,13 +56,6 @@ static void dbg_hcs_params (struct ehci_
label, buf);
}
 }
-#else
-
-static inline void dbg_hcs_params (struct ehci_hcd *ehci, char *label) {}
-
-#endif
-
-#ifdef CONFIG_DYNAMIC_DEBUG
 
 /* check the values in the HCCPARAMS register
  * (host controller _Capability_ parameters)
@@ -95,13 +88,6 @@ static void dbg_hcc_params (struct ehci_
" 32 periodic list" : "");
}
 }
-#else
-
-static inline void dbg_hcc_params (struct ehci_hcd *ehci, char *label) {}
-
-#endif
-
-#ifdef CONFIG_DYNAMIC_DEBUG
 
 static void __maybe_unused
 dbg_qtd (const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
@@ -280,29 +266,6 @@ dbg_port_buf (char *buf, unsigned len, c
(status & PORT_CONNECT) ? " CONNECT" : "");
 }
 
-#else
-static inline void __maybe_unused
-dbg_qh (char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
-{}
-
-static inline int __maybe_unused
-dbg_status_buf (char *buf, unsigned len, const char *label, u32 status)
-{ return 0; }
-
-static inline int __maybe_unused
-dbg_command_buf (char *buf, unsigned len, const char *label, u32 command)
-{ return 0; }
-
-static inline int __maybe_unused
-dbg_intr_buf (char *buf, unsigned len, const char *label, u32 enable)
-{ return 0; }
-
-static inline int __maybe_unused
-dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status)
-{ return 0; }
-
-#endif /* CONFIG_DYNAMIC_DEBUG */
-
 /* functions have the "wrong" filename when they're output... */
 #define dbg_status(ehci, label, status) { \
char _buf [80]; \
@@ -324,13 +287,6 @@ dbg_port_buf (char *buf, unsigned len, c
 
 /*-*/
 
-#ifdef STUB_DEBUG_FILES
-
-static inline void create_debug_files (struct ehci_hcd *bus) { }
-static inline void remove_debug_files (struct ehci_hcd *bus) { }
-
-#else
-
 /* troubleshooting help: expose state in debugfs */
 
 static int debug_async_open(struct inode *, struct file *);
@@ -1089,4 +1045,38 @@ static inline void remove_debug_files (s
debugfs_remove_recursive(ehci->debug_dir);
 }
 
-#endif /* STUB_DEBUG_FILES */
+#else /* CONFIG_DYNAMIC_DEBUG */
+
+static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) { }
+static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) { }
+
+static inline void __maybe_unused dbg_qh(const char *label,
+   struct ehci_hcd *ehci, struct ehci_qh *qh) { }
+
+static inline int __maybe_unused dbg_status_buf(const char *buf,
+   unsigned len, const char *label, u32 status)
+{ return 0; }
+
+static inline int __maybe_unused dbg_command_buf(const char *buf,
+   unsigned len, const char *label, u32 command)
+{ return 0; }
+
+static inline int __maybe_unused dbg_intr_buf(const char *buf,
+   unsigned len, const char *label, u32 enable)
+{ return 0; }
+
+static inline int __maybe_unused dbg_port_buf(char *buf,
+   unsigned len, const char *label, int port, u32 status)
+{ return 0; }
+
+static inline void dbg_status(struct ehci_hcd *ehci, const char *label,
+   u32 status) { }
+static inline void dbg_cmd(struct ehci_hcd *ehci, const char *label,
+   u32 command) { }
+static inline void dbg_port(struct ehci_hcd *ehci, const char *label,
+   int port, u32 status) { }
+
+static inline void create_debug_files(struct ehci_hcd *bus) { }
+static inline void remove_debug_files(struct ehci_hcd *bus) { }
+
+#endif /* CONFIG_DYNAMIC_DEBUG */
Index: usb-4.4/drivers/usb/host/ehci.h

Re: ehci-dbg prints random memory (# CONFIG_DYNAMIC_DEBUG is not set)

[Greg Kroah-Hartman]

On Wed, Apr 13, 2016 at 08:44:25AM +0200, Rafał Miłecki wrote:
> Hi,
> 
> I'm trying to debug some EHCI issue so I enabled debugging by adding
> ccflags-y := -DDEBUG
> to the drivers/usb/host/Makefile
> 
> Some of debugging lines contain random memory, e.g.:
> ehci-platform ehci-platform.0: .|��`|���5P5@�3�.��*�.|��o
> 
> It's caused by dbg_status, dbg_cmd and dbg_port calling ehci_dbg even with:
> # CONFIG_DYNAMIC_DEBUG
> 
> The lack of above config implies using dummy dbg_status_buf,
> dbg_command_buf and dbg_port_buf. They don't print anything to the
> buffer and it stays uninitialized.
> 
> Can you give me a hint how to solve this (apart from enabling
> CONFIG_DYNAMIC_DEBUG for my debugging which I did)? Should dbg_*_buf
> functions return some error instead of 0? We could check for that in
> dbg_* functions then. Unfortunately I'm not sure how it's going to
> affect other places, e.g. fill_registers_buffer.

Please just enable CONFIG_DYNAMIC_DEBUG, as you did, that's the correct
thing to do here, don't try to hack up Makefiles by hand.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

ehci-dbg prints random memory (# CONFIG_DYNAMIC_DEBUG is not set)

[Rafał Miłecki]

Hi,

I'm trying to debug some EHCI issue so I enabled debugging by adding
ccflags-y := -DDEBUG
to the drivers/usb/host/Makefile

Some of debugging lines contain random memory, e.g.:
ehci-platform ehci-platform.0: .|��`|���5P5@�3�.��*�.|��o

It's caused by dbg_status, dbg_cmd and dbg_port calling ehci_dbg even with:
# CONFIG_DYNAMIC_DEBUG

The lack of above config implies using dummy dbg_status_buf,
dbg_command_buf and dbg_port_buf. They don't print anything to the
buffer and it stays uninitialized.

Can you give me a hint how to solve this (apart from enabling
CONFIG_DYNAMIC_DEBUG for my debugging which I did)? Should dbg_*_buf
functions return some error instead of 0? We could check for that in
dbg_* functions then. Unfortunately I'm not sure how it's going to
affect other places, e.g. fill_registers_buffer.

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html