Re: [PATCH] staging: unisys: Convert direct LOGERR uses to pr_err

[Greg Kroah-Hartman]

On Sat, Jan 10, 2015 at 09:32:33PM -0800, Joe Perches wrote:
> On Sat, 2015-01-10 at 21:24 -0800, Greg Kroah-Hartman wrote:
> > On Sat, Jan 10, 2015 at 08:55:21PM -0800, Joe Perches wrote:
> > > On Sat, 2015-01-10 at 20:13 -0800, Greg Kroah-Hartman wrote:
> > > > On Sat, Jan 10, 2015 at 07:09:02PM -0800, Joe Perches wrote:
> > > > > Use the normal logging facility instead of a new macro.
> > > > > 
> > > > > Miscellanea:
> > > > > 
> > > > > o Realign arguments
> > > > > o Remove unnecessary alloc error messages as alloc failures
> > > > >   already emits a generic OOM message with a dump_stack
> > > > > o Add missing newlines to these messages to avoid interleaving
> > > > > o Spelling/typo fixes in those messages
> > > []
> > > > I rejected the other patch that did much this same thing, almost all of
> > > > these should be converted to use dev_* calls instead of pr_* calls, as
> > > > this is a driver, and drivers should always have access to a struct
> > > > device.  If not, they need to be fixed.
> > > 
> > > Baby steps.
> > > 
> > > This one prevents interleaving
> > > and fixes typos.
> > 
> > Yes, but converting it to dev_* at the same time saves a step, and gets
> > it done properly.  Leaving messages at pr_* levels takes away the
> > checkpatch warning, which makes the code "seem" to be acceptable.
> 
> I think you've not looked at the code very much.
> 
> Many of these things can not be converted to dev_<level>
> as there is not a single device there.

Then that needs to be fixed.

> Many of these are error messages referring to busses that
> support many devices.

The device that caused the error should emit the message, or the host
controller for that bus.  "general" messages shouldn't ever be happening
as they are useless.  If they are, something is wrong with the code.

> Converting them to dev_<level> piecemeal or wholesale
> isn't possible.

I beg to differ, no driver should be using pr_* except in init/exit
functions when there is no hardware yet found.  Everywhere else a struct
device is there to be used.  If the code isn't passing it around, that
is usually an indication that something is really wrong and should be
fixed.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/