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
--
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
