RE: [PATCH 2/3] [media] s5p-mfc: Replace printk with pr_* functions

[Kamil Debski <k.debski@samsung.com>]

Hi Sachin,

I am afraid that I have to NACK this patch.

pr_debug/pr_err/etc. is useful when you want to add some data in front of the
debug message.

So if you really insist then you could try to add something like this

+#define pr_fmt(fmt) ":%s:%d: " fmt, __func__, __LINE__

+#define mfc_err pr_err

I share the opinion on these patch with Sylwester's opinion on the similar
patch
for FIMC - I don't think it's worth the effort. 

Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center


> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
> Sent: 11 June 2012 12:14
> To: linux-media@vger.kernel.org
> Cc: t.stanislaws@samsung.com; k.debski@samsung.com;
> s.nawrocki@samsung.com; snjw23@gmail.com; kyungmin.park@samsung.com;
> mchehab@infradead.org; sachin.kamat@linaro.org; patches@linaro.org
> Subject: [PATCH 2/3] [media] s5p-mfc: Replace printk with pr_* functions
> 
> Replace printk with pr_* functions to silence checkpatch warnings.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/media/video/s5p-mfc/s5p_mfc_debug.h |    6 +++---
>  drivers/media/video/s5p-mfc/s5p_mfc_opr.c   |    5 +++--
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_debug.h
> b/drivers/media/video/s5p-mfc/s5p_mfc_debug.h
> index ecb8616..fea2c6e 100644
> --- a/drivers/media/video/s5p-mfc/s5p_mfc_debug.h
> +++ b/drivers/media/video/s5p-mfc/s5p_mfc_debug.h
> @@ -23,7 +23,7 @@ extern int debug;
>  #define mfc_debug(level, fmt, args...)				\
>  	do {							\
>  		if (debug >= level)				\
> -			printk(KERN_DEBUG "%s:%d: " fmt,	\
> +			pr_debug("%s:%d: " fmt,	\
>  				__func__, __LINE__, ##args);	\
>  	} while (0)
>  #else
> @@ -35,13 +35,13 @@ extern int debug;
> 
>  #define mfc_err(fmt, args...)				\
>  	do {						\
> -		printk(KERN_ERR "%s:%d: " fmt,		\
> +		pr_err("%s:%d: " fmt,		\
>  		       __func__, __LINE__, ##args);	\
>  	} while (0)
> 
>  #define mfc_info(fmt, args...)				\
>  	do {						\
> -		printk(KERN_INFO "%s:%d: " fmt,		\
> +		pr_info("%s:%d: " fmt,		\
>  		       __func__, __LINE__, ##args);	\
>  	} while (0)
> 
> diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_opr.c
> b/drivers/media/video/s5p-mfc/s5p_mfc_opr.c
> index e6217cb..6d3f398 100644
> --- a/drivers/media/video/s5p-mfc/s5p_mfc_opr.c
> +++ b/drivers/media/video/s5p-mfc/s5p_mfc_opr.c
> @@ -12,6 +12,8 @@
>   * published by the Free Software Foundation.
>   */
> 
> +#define pr_fmt(fmt) "s5p-mfc: " fmt
> +
>  #include "regs-mfc.h"
>  #include "s5p_mfc_cmd.h"
>  #include "s5p_mfc_common.h"
> @@ -187,8 +189,7 @@ int s5p_mfc_alloc_codec_buffers(struct s5p_mfc_ctx
> *ctx)
>  		dev->alloc_ctx[MFC_BANK1_ALLOC_CTX], ctx->bank1_size);
>  		if (IS_ERR(ctx->bank1_buf)) {
>  			ctx->bank1_buf = NULL;
> -			printk(KERN_ERR
> -			       "Buf alloc for decoding failed (port A)\n");
> +			pr_err("Buf alloc for decoding failed (port A)\n");
>  			return -ENOMEM;

This can be replaced with mfc_err to make it consistent with other error
messages in this file.
It's my mistake that I have use printk(KERN_ERR ...

I think it is beneficial to read the neighboring lines of the line which
checkpatch
returns a warning in.

>  		}
>  		ctx->bank1_phys = s5p_mfc_mem_cookie(
> --
> 1.7.4.1