Re: [PATCH 3/3 v2] Staging: crystalhd: Replace the BCMLOG_ERR macrowith pr_err

[Jorgyano vieira]

On Sat, Feb 18, 2012 at 8:31 PM, Joe Perches <joe@xxxxxxxxxxx> 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
--
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/