lkml.org 
[lkml]   [2012]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH 3/3 v2] Staging: crystalhd: Replace the BCMLOG_ERR macro with pr_err
From
On Sat, Feb 18, 2012 at 8:31 PM, Joe Perches <joe@perches.com> wrote:
> On Sat, 2012-02-18 at 20:10 -0200, Jorgyano Vieira wrote:
>> Replace the usage of BCMLOG_ERR with pr_err and remove the macro definition.
>> Also added pr_fmt to identify the driver error messages.
>
> Hi Jorgyano.
Hi Joe,
>
> Couple of generic comments.
>
> o Please look for and fix spelling errors at the same time.
People are blamed for doing more-then-one-thing-per-patch,
so I prefer send only one big patch fixing all spelling errors and
coding style on further work.

> o Coalesce formats, don't worry about 80 columns for that.
> o pr_fmt needs to be #defined before any #include that might
> directly or indirectly #include kernel.h/printk.h
> use 'strings <path>/built-in.o | grep "^<.>"' to verify.
yes, I take care of this:--- a/drivers/staging/crystalhd/crystalhd_misc.h
+++ b/drivers/staging/crystalhd/crystalhd_misc.h
@@ -28,6 +28,8 @@
#ifndef _CRYSTALHD_MISC_H_
#define _CRYSTALHD_MISC_H_

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/errno.h>
> o [kv][kzm]alloc failures don't need specific OOM messages
> as the generic function does a dump_stack
You are right, this code really have some unnecessary messages,
perhaps it could be done on further patch, all at once.
The main idea on this patch is just replace convert the old macro to pr_err.

> o argument alignment could be after the open parenthesis.
yes, it could be done together with the coding style issues.

there is a lot of work to do on this driver.

Many thanks for your comments.

regards,
Jorgyano Vieira


\
 
 \ /
  Last update: 2012-02-19 00:11    [W:0.059 / U:0.368 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site