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

Kershner, David A David.Kershner at unisys.com
Wed Jun 17 02:12:05 UTC 2015


> -----Original Message-----
> From: Greg KH [mailto:gregkh at linuxfoundation.org]
> Sent: Tuesday, June 16, 2015 10:09 PM
> To: Sell, Timothy C
> Cc: driverdev-devel at linuxdriverproject.org; Jes.Sorensen at redhat.com; *S-
> Par-Maintainer; Romer, Benjamin M
> Subject: Re: [PATCH 5/5] staging: unisys: add error messages to visornic
> 
> On Wed, Jun 17, 2015 at 12:08:11AM +0000, Sell, Timothy C wrote:
> > > -----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.
> 
> "hold open the window"?  What do you mean by this?
> 

We want to regroup before we resend this patch to make sure we are
putting the correct messages out. So it will be a few days before the
patch is resent. 

David Kershner

> > 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?
> 
> I don't know, I didn't get further than this function, sorry.  If they
> are doing nothing more than "look at this function this device called!"
> then they should be removed.
> 
> thanks,
> 
> greg k-h


More information about the devel mailing list