Re: [RFC] xhci: remove USB_XHCI_HCD_DEBUGGING config option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/01/2013 04:24 AM, Sarah Sharp wrote:
On Sun, Jun 30, 2013 at 02:54:26PM -0700, Greg KH wrote:
On Mon, Jul 01, 2013 at 12:23:18AM +0300, Xenia Ragiadakou wrote:
CONFIG_USB_XHCI_HCD_DEBUGGING option is used to enable
verbose debugging output for the xHCI host controller
driver.

In the current version of the xhci-hcd driver, this
option must be turned on, in order for the debugging
log messages to be displayed, and users may need to
recompile the linux kernel to obtain debugging
information that will help them track down problems.

This patch removes the above debug option to enable
debugging log messages at all times.
The aim of this is to rely on the debugfs and the
dynamic debugging feature for fine-grained management
of debugging messages and to not force users to set
the debug config option and compile the linux kernel
in order to have access in that information.

This patch, also, removes the XHCI_DEBUG symbol and the
functions dma_to_stream_ring(), xhci_test_radix_tree()
and xhci_event_ring_work() that are not useful anymore.
Those functions really aren't useful anymore?  If so, great, but
wouldn't be nice to be able to enable them dynamically through debugfs
if someone wanted the information they provide?
They're really not useful.

The xhci_test_radix_tree() runs the same static test on the radix tree
whenever the xHCI module loads.  I mostly wrote it to test my code, but
it's not useful anymore.  dma_to_stream_ring() is only called from
xhci_test_radix_tree().

xhci_event_ring_work() spews the contents of the endpoint rings every
minute.  It's really not useful to leave turned on, since the rings are
printed in the various places where interesting ring events occur.  The
ring printing also doesn't scale as more and more devices are added.

I think it would be better if users could get the contents of the
rings through a debugfs file instead.  I think the EHCI driver does
something similar for the frame list.  But one thing at a time.  I'll
save that item for Xenia in my 'future-tasks' file.

  #define xhci_dbg(xhci, fmt, args...) \
-	do { if (XHCI_DEBUG) dev_dbg(xhci_to_hcd(xhci)->self.controller , fmt , ## args); } while (0)
+	dev_dbg(xhci_to_hcd(xhci)->self.controller , fmt , ## args)
That's good.

  #define xhci_info(xhci, fmt, args...) \
-	do { if (XHCI_DEBUG) dev_info(xhci_to_hcd(xhci)->self.controller , fmt , ## args); } while (0)
+	dev_info(xhci_to_hcd(xhci)->self.controller , fmt , ## args)
I think you might have just turned on a bunch more "default" information
here.  Hm, it's only used twice anyway, in the same "error", shouldn't
they be turned into xhci_dbg() calls instead?
Yeah, they should just be turned into xhci_dbg(), and xhci_info should
be removed.

Sarah Sharp

I will replace xhci_info() and printk() calls with xhci_dbg()
in another patch. Because if I replace xhci_info() in this
patch it will look out of topic, I think.

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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux