Messages in this thread | | | Subject | Re: staging: ks7010: Replace three printk() calls by pr_err() | From | Joe Perches <> | Date | Sat, 13 Aug 2016 10:31:18 -0700 |
| |
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.
| |