On Thu, 2013-11-14 at 13:04 -0500, Alan Stern wrote:
> On Thu, 14 Nov 2013 oliver@xxxxxxxxxx wrote:
>
> > From: Oliver Neukum <oneukum@xxxxxxx>
> >
> > To allow a full switch to dynamic debugging make the
> > debug parameter conditional on defined(DEBUF) || defined(DYNAMIC_DEBUG)
> >
> > Signed-off-by: Oliver Neukum <oneukum@xxxxxxx>
> > ---
> > drivers/usb/host/uhci-hcd.c | 48 +++++++++++++++++++++++++++------------------
> > 1 file changed, 29 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
> > index 4a86b63..e3b1a4c 100644
> > --- a/drivers/usb/host/uhci-hcd.c
> > +++ b/drivers/usb/host/uhci-hcd.c
> > @@ -69,18 +69,21 @@ MODULE_PARM_DESC(ignore_oc, "ignore hardware overcurrent indications");
> > * show all queues in /sys/kernel/debug/uhci/[pci_addr]
> > * debug = 3, show all TDs in URBs when dumping
> > */
> > -#ifdef DEBUG
> > -#define DEBUG_CONFIGURED 1
> > +#if defined(DEBUG) || defined(DYNAMIC_DEBUG)
> > +
> > static int debug = 1;
> > module_param(debug, int, S_IRUGO | S_IWUSR);
> > MODULE_PARM_DESC(debug, "Debug level");
> > +static char *errbuf;
> >
> > #else
> > -#define DEBUG_CONFIGURED 0
> > -#define debug 0
> > +
> > +#define debug 0
> > +#define errbuf NULL
> > +
> > #endif
>
> The advantage of keeping the DEBUG_CONFIGURED symbol shows up later...
>
> > @@ -462,12 +465,14 @@ static irqreturn_t uhci_irq(struct usb_hcd *hcd)
> > if (uhci->rh_state >= UHCI_RH_RUNNING) {
> > dev_err(uhci_dev(uhci),
> > "host controller halted, very bad!\n");
> > +#if defined(DEBUG) || defined(DYNAMIC_DEBUG)
> > if (debug > 1 && errbuf) {
> > /* Print the schedule for debugging */
> > uhci_sprint_schedule(uhci, errbuf,
> > ERRBUF_LEN - EXTRA_SPACE);
> > lprintk(errbuf);
> > }
> > +#endif
>
> You're going to remove this #if, right?
Yes.
>
> > @@ -823,8 +827,10 @@ static int uhci_count_ports(struct usb_hcd *hcd)
> > if (!(portstatus & 0x0080) || portstatus == 0xffff)
> > break;
> > }
> > +#if defined(DEBUG) || defined(DYNAMIC_DEBUG)
> > if (debug)
> > dev_info(uhci_dev(uhci), "detected %d ports\n", port);
> > +#endif
>
> And this one?
>
> > @@ -868,14 +874,14 @@ static int __init uhci_hcd_init(void)
> > ignore_oc ? ", overcurrent ignored" : "");
> > set_bit(USB_UHCI_LOADED, &usb_hcds_loaded);
> >
> > - if (DEBUG_CONFIGURED) {
> > - errbuf = kmalloc(ERRBUF_LEN, GFP_KERNEL);
> > - if (!errbuf)
> > - goto errbuf_failed;
> > - uhci_debugfs_root = debugfs_create_dir("uhci", usb_debug_root);
> > - if (!uhci_debugfs_root)
> > - goto debug_failed;
> > - }
> > +#if defined(DEBUG) || defined(DYNAMIC_DEBUG)
> > + errbuf = kmalloc(ERRBUF_LEN, GFP_KERNEL);
> > + if (!errbuf)
> > + goto errbuf_failed;
> > +#endif
> > + uhci_debugfs_root = debugfs_create_dir("uhci", usb_debug_root);
> > + if (!uhci_debugfs_root)
> > + goto debug_failed;
>
> If you retain the DEBUG_CONFIGURED symbol then this change isn't
> needed.
That is not an advantage. If we effectively do conditional
compilation, we should make it explicit.
Regards
Oliver
--
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
