Re: staging: ks7010: Replace three printk() calls by pr_err()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 2016-08-13 at 13:10 +0200, SF Markus Elfring wrote:
> > > Prefer usage of the macro "pr_err" over the interface "printk".
> > Not correct
> A checkpatch warning like "PREFER_PR_LEVEL" can point additional possibilities out
> for this use case.
> Would you like to introduce any of the higher level logging functions instead?

I think pr_<level> is OK if reworking the code
to support dev_<level> is not easy.

> > > diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> > []
> > > 
> > > @@ -998,11 +998,11 @@ static int ks7010_sdio_probe(struct sdio_func *func,
> > >  	/* private memory allocate */
> > >  	netdev = alloc_etherdev(sizeof(*priv));
> > >  	if (netdev == NULL) {
> > > -		printk(KERN_ERR "ks7010 : Unable to alloc new net device\n");
> > > +		pr_err(pr_fmt("Unable to alloc new net device\n"));
> > All of these pr_fmt uses are redundant as pr_err already does pr_fmt
> Thanks for your reminder.
> 
> Would you accept that another update will be appended to the discussed patch series?

No.  Patches should not knowingly introduce defects
that are corrected in follow-on patches.

> > alloc_etherdev already does a dump_stack so the OOM isn't useful.
> Does this information indicate that this printk() (or pr_err()) call
> should be deleted?

Markus, I don't know if it's your lack of English
comprehension or not, but it's fairly obvious from
my reply that this line should be deleted, either
in this patch or a follow-on.

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux