[PATCH 5/5] staging: unisys: add error messages to visornic

[Sell, Timothy C]

> -----Original Message-----
> From: Greg KH [mailto:gregkh at linuxfoundation.org]
> Sent: Tuesday, June 16, 2015 5:36 PM
> To: Romer, Benjamin M
> Cc: driverdev-devel at linuxdriverproject.org; Jes.Sorensen at redhat.com; *S-
> Par-Maintainer; Sell, Timothy C
> Subject: Re: [PATCH 5/5] staging: unisys: add error messages to visornic
> 
> On Mon, Jun 15, 2015 at 11:32:00PM -0400, Benjamin Romer wrote:
> > From: Tim Sell <Timothy.Sell at unisys.com>
> >
> > Add error message to genuine rare error paths, and debug messages
> > to enable relatively non-verbose tracing of code paths
> >
> > You can enable debug messages by including this on the kernel command
> line:
> >
> >     visornic.dyndbg=+p
> >
> > or by this from the command line:
> >
> >     echo "module visornic +p" > <debugfs>/dynamic_debug/control
> >
> > Refer to Documentation/dynamic-debug-howto.txt for more details.
> >
> > In addition to the new debug and error messages, a message like the
> > following will be logged every time a visornic device is probed, which
> > will enable you to map back-and-forth between visorbus device names
> > (e.g., "vbus2:dev0") and netdev names (e.g., "eth0"):
> >
> >     visornic vbus2:dev0: visornic_probe success netdev=eth0
> >
> > With this patch and visornic debugging enabled, you should expect to see
> > messages like the following in the most-common scenarios:
> >
> > * driver loaded:
> >
> >       visornic_init
> >
> > * device probed:
> >
> >       visornic vbus2:dev0: visornic_probe
> >       visor_thread_start
> >       visor_thread_start success
> >
> > * network interface configured (ifconfig):
> >
> >       net eth0: visornic_open
> >       net eth0: visornic_enable_with_timeout
> >       net eth0: visornic_enable_with_timeout success
> >       net eth0: visornic_open success
> >
> > staging: unisys: More comments from Jes
> >
> > Signed-off-by: Tim Sell <Timothy.Sell at unisys.com>
> > Signed-off-by: David Kershner <david.kershner at unisys.com>
> > Signed-off-by: Benjamin Romer <benjamin.romer at unisys.com>
> > ---
> >  drivers/staging/unisys/visornic/visornic_main.c | 127
> ++++++++++++++++++++++--
> >  1 file changed, 116 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/staging/unisys/visornic/visornic_main.c
> b/drivers/staging/unisys/visornic/visornic_main.c
> > index 7100744..7b9821c 100644
> > --- a/drivers/staging/unisys/visornic/visornic_main.c
> > +++ b/drivers/staging/unisys/visornic/visornic_main.c
> > @@ -294,14 +294,18 @@ static int visor_thread_start(struct
> visor_thread_info *thrinfo,
> >  			      int (*threadfn)(void *),
> >  			      void *thrcontext, char *name)
> >  {
> > +	pr_debug("%s\n", __func__);
> 
> These types of "entered/exited" debugging lines should never be needed,
> just use the ftrace infrastructure instead which provides this for you,
> for free, in a much nicer interface.
> 
> Please remove these from the patch and resend.
> 
> thanks,
> 
> greg k-h

Thanks; we will get rid of the offending pr_debug()s from the patch and
re-send, but please do NOT hold open the window just for this follow-on
patch.  BTW, I assume it's only the pr_debug()s that you would like
removed, and that the dev_dbg()s can stay intact (since they also provide
relevant device name info), right?

Thanks for the pointer about ftrace; never used it before today but I
already tried it and like it.

Tim Sell