Re: [PATCH v4 2/3] venus: Make debug infrastructure more flexible

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Jun 15, 2020 at 12:55:29PM +0300, Stanimir Varbanov wrote:
> Hi Greg,
> 
> On 6/14/20 9:37 AM, Greg KH wrote:
> > On Sun, Jun 14, 2020 at 01:39:18AM +0300, Stanimir Varbanov wrote:
> >>  	if (slot == -1) {
> >> -		dev_dbg(inst->core->dev, "%s: no free slot\n", __func__);
> >> +		VDBGH("no free slot for timestamp\n");
> > 
> > Again, no, you just lost a lot of valuable information by changing to a
> > different format (like driver, specific device, etc.).  Please don't do
> > this, it just makes the information less than before.
> 
> OK, one of the reasons to use pr_debug inside VDBGH macro is to avoid
> having struct device *dev variable in every function with dev_dbg even
> when the function doesn't use it.

But the function _is_ using it, as you are referring to the device that
is being controlled by the driver.  That's the point, you are stripping
off that very valuable information for no reason.

Which means to me that you never really actually _NEED_ these debugging
messages, as you have not used them to see if it provides you with
something that can tell you something about something.

So, let me push harder, why do you even want this message at all?  What
can it provide you now that the driver is up and working properly?

> Are you fine with s/pr_debug/dev_dbg in VDBGX macros?

I would be a bit happier yes, but the fact that you didn't use it means
you aren't even looking at these messages, which implies that it isn't
even needed.

So, how about just stripping all of these debugging messages out
entirely?  What do they provide that you don't already know?  Who would
use them?

thanks,

greg k-h