Hi Benoit,
On Thu, Jun 18, 2020 at 01:28:55PM -0500, Benoit Parrot wrote:
> Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote on Mon [2020-Jun-15 02:58:27 +0300]:
> > Use the dev_* print macros instead of the v4l2_* print macros. This
> > prepares for a common print infrastructure that will also support the
> > cal_camerarx instances.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/platform/ti-vpe/cal.c | 17 ++++++++++-------
> > 1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> > index e40967751aa4..e62089832713 100644
> > --- a/drivers/media/platform/ti-vpe/cal.c
> > +++ b/drivers/media/platform/ti-vpe/cal.c
> > @@ -53,19 +53,22 @@ static unsigned debug;
> > module_param(debug, uint, 0644);
> > MODULE_PARM_DESC(debug, "activates debug info");
> >
> > -#define cal_dbg(level, cal, fmt, arg...) \
> > - v4l2_dbg(level, debug, &cal->v4l2_dev, fmt, ##arg)
> > +#define cal_dbg(level, cal, fmt, arg...) \
> > + do { \
> > + if (debug >= (level)) \
> > + dev_dbg(&(cal)->pdev->dev, fmt, ##arg); \
> > + } while (0)
>
> The problem here with using dev_dbg is that you loose the ability to enable
> or disable debug log at runtime. Which I find highly convenient.
>
> With dev_dbg you have to compile with DEBUG defined or at the very least
> DYNAMIC_DEBUG.
That's actually one of the things I like about dev_dbg(), it shrinks the
kernel when DEBUG (or DYNAMIC_DEBUG) isn't defined. In which case we
could possibly even remove the debug module parameter, as DYNAMIC_DEBUG
already gives us control over which messages are printed.
I however hear your concern, and I don't dispute that a module parameter
can also make debugging easier, at in some cases. It's partly a matter
of getting use to DYNAMIC_DEBUG, but not just that. I'll thus replace
dev_dbg() with dev_printk(KERN_DEBUG, ...) here.
> > #define cal_info(cal, fmt, arg...) \
> > - v4l2_info(&cal->v4l2_dev, fmt, ##arg)
> > + dev_info(&(cal)->pdev->dev, fmt, ##arg)
> > #define cal_err(cal, fmt, arg...) \
> > - v4l2_err(&cal->v4l2_dev, fmt, ##arg)
> > + dev_err(&(cal)->pdev->dev, fmt, ##arg)
> >
> > #define ctx_dbg(level, ctx, fmt, arg...) \
> > - v4l2_dbg(level, debug, &ctx->v4l2_dev, fmt, ##arg)
> > + cal_dbg(level, (ctx)->cal, "ctx%u: " fmt, (ctx)->csi2_port, ##arg)
> > #define ctx_info(ctx, fmt, arg...) \
> > - v4l2_info(&ctx->v4l2_dev, fmt, ##arg)
> > + cal_info((ctx)->cal, "ctx%u: " fmt, (ctx)->csi2_port, ##arg)
> > #define ctx_err(ctx, fmt, arg...) \
> > - v4l2_err(&ctx->v4l2_dev, fmt, ##arg)
> > + cal_err((ctx)->cal, "ctx%u: " fmt, (ctx)->csi2_port, ##arg)
> >
> > #define CAL_NUM_CONTEXT 2
> >
--
Regards,
Laurent Pinchart
