Re: [PATCH] media: staging: zoran: refactor printk debugging function

[Dan Carpenter <dan.carpenter@oracle.com>]

On Wed, Apr 20, 2022 at 08:23:16PM -0400, Ian Cowan wrote:
> This is a patch to refactor the zoran debugging function. This function
> existed in all of the changed files and they also all import the
> videocodec header file. This patch moves the dprintk function into the
> videocodec header file and out of each of the individual files.
> 
> Signed-off-by: Ian Cowan <ian@linux.cowan.aero>
> ---
>  drivers/staging/media/zoran/videocodec.c | 67 +++++++++++++--------
>  drivers/staging/media/zoran/videocodec.h |  7 +++
>  drivers/staging/media/zoran/zr36016.c    | 48 ++++++++-------
>  drivers/staging/media/zoran/zr36050.c    | 77 ++++++++++++------------
>  drivers/staging/media/zoran/zr36060.c    | 54 ++++++++---------
>  5 files changed, 141 insertions(+), 112 deletions(-)
> 
> diff --git a/drivers/staging/media/zoran/videocodec.c b/drivers/staging/media/zoran/videocodec.c
> index 3af7d02bd910..e35d1f54d9b8 100644
> --- a/drivers/staging/media/zoran/videocodec.c
> +++ b/drivers/staging/media/zoran/videocodec.c
> @@ -20,12 +20,6 @@ static int videocodec_debug;
>  module_param(videocodec_debug, int, 0);
>  MODULE_PARM_DESC(videocodec_debug, "Debug level (0-4)");
>  
> -#define dprintk(num, format, args...) \
> -	do { \
> -		if (videocodec_debug >= num) \
> -			printk(format, ##args); \
> -	} while (0)
> -
>  struct attached_list {
>  	struct videocodec *codec;
>  	struct attached_list *next;
> @@ -56,8 +50,8 @@ struct videocodec *videocodec_attach(struct videocodec_master *master)
>  		return NULL;
>  	}
>  
> -	dprintk(2, "%s: '%s', flags %lx, magic %lx\n", __func__,
> -		master->name, master->flags, master->magic);
> +	dprintk(videocodec_debug, 2, "%s: '%s', flags %lx, magic %lx\n",
> +		__func__, master->name, master->flags, master->magic);

[ snip ]

> +/* Print debug information based on the specified level of debugging */
> +#define dprintk(debug, num, format, args...) \
> +do { \
> +	if (debug > num) \
> +		printk(format, ##args); \
> +} while (0)
> +

I don't like the new format.  It needs to have a KERN_DEBUG at the
start.  Passing the param_module is awkward.  No one knows what the
magic "num" things are.

Using a module parameter is old school badness.  The standard dev_dbg()
macros are better and more flexible.  Just use them instead.

regards,
dan carpenter