On Thu, Apr 21, 2022 at 05:21:54PM +0300, Dan Carpenter wrote:
> 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.
> >
> > -#define dprintk(num, format, args...) \
> > - do { \
> > - if (videocodec_debug >= num) \
> > - printk(format, ##args); \
> > - } while (0)
> > -
[ 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.
I was going to put KERN_DEBUG at the beginning, but it appears that some
of the calls already include KERN_ERR, KERN_WARNING, or KERN_INFO, but
most of them do not. Would it be better to split these into multiple
macros or use printk in those specific spots that difer?
> 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
>
For using the dev_dbg() macro, do you define this in the header file
(i.e. for this it would be videocodec.h), or where should this be
included from?
---
Ian Cowan
