On Fri, Sep 28, 2012 at 4:52 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> On Fri, 2012-09-28 at 15:53 +0530, Chandrabhanu Mahapatra wrote:
>> The printk in DSSDBG function definition is replaced with dynamic debug enabled
>> pr_debug(). The use of dynamic debugging provides more flexibility as each debug
>> statement can be enabled or disabled dynamically on basis of source filename,
>> line number, module name etc. by writing to a control file in debugfs
>> filesystem. For better understanding please refer to
>> Documentation/dynamic-debug-howto.txt.
>>
>> The DSSDBGF() differs from DSSDBG() by providing function name. However,
>> function name, line number, module name and thread ID can be printed through
>> dynamic debug by setting appropriate flags 'f','l','m' and 't' in the debugfs
>> control file. So, DSSDBGF instances are replaced with DSSDBG.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@xxxxxx>
>> ---
>> drivers/video/omap2/dss/apply.c | 8 ++++----
>> drivers/video/omap2/dss/dsi.c | 12 ++++++------
>> drivers/video/omap2/dss/dss.h | 34 ++++++++--------------------------
>> 3 files changed, 18 insertions(+), 36 deletions(-)
>>
>
>> - DSSDBGF("%d", channel);
>> + DSSDBG("Initial Config of Virtual Channel %d", channel);
>
> A Bit Too Much Capital Letters here for my taste =). Use just one at the
> beginning of the print, or none at all.
>
>>
>> r = dsi_read_reg(dsidev, DSI_VC_CTRL(channel));
>>
>> @@ -2814,7 +2814,7 @@ static int dsi_vc_config_source(struct platform_device *dsidev, int channel,
>> if (dsi->vc[channel].source == source)
>> return 0;
>>
>> - DSSDBGF("%d", channel);
>> + DSSDBG("Source Config of Virtual Channel %d", channel);
>
> Here also.
>
Ok.
>>
>> dsi_sync_vc(dsidev, channel);
>>
>> @@ -3572,7 +3572,7 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
>> int r, i;
>> unsigned mask;
>>
>> - DSSDBGF();
>> + DSSDBG("Entering ULPS");
>>
>> WARN_ON(!dsi_bus_is_locked(dsidev));
>>
>> @@ -4276,7 +4276,7 @@ int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
>> unsigned long pck;
>> int r;
>>
>> - DSSDBGF("ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
>> + DSSDBG("Setting DSI clocks: ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
>>
>> mutex_lock(&dsi->lock);
>>
>> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
>> index ffbba7e..5ef4e17 100644
>> --- a/drivers/video/omap2/dss/dss.h
>> +++ b/drivers/video/omap2/dss/dss.h
>> @@ -25,38 +25,20 @@
>>
>> #ifdef DEBUG
>> extern bool dss_debug;
>> -#ifdef DSS_SUBSYS_NAME
>> -#define DSSDBG(format, ...) \
>> - if (dss_debug) \
>> - printk(KERN_DEBUG "omapdss " DSS_SUBSYS_NAME ": " format, \
>> - ## __VA_ARGS__)
>> -#else
>> -#define DSSDBG(format, ...) \
>> - if (dss_debug) \
>> - printk(KERN_DEBUG "omapdss: " format, ## __VA_ARGS__)
>> #endif
>>
>> -#ifdef DSS_SUBSYS_NAME
>> -#define DSSDBGF(format, ...) \
>> - if (dss_debug) \
>> - printk(KERN_DEBUG "omapdss " DSS_SUBSYS_NAME \
>> - ": %s(" format ")\n", \
>> - __func__, \
>> - ## __VA_ARGS__)
>> -#else
>> -#define DSSDBGF(format, ...) \
>> - if (dss_debug) \
>> - printk(KERN_DEBUG "omapdss: " \
>> - ": %s(" format ")\n", \
>> - __func__, \
>> - ## __VA_ARGS__)
>> +#ifdef pr_fmt
>> +#undef pr_fmt
>> #endif
>>
>> -#else /* DEBUG */
>> -#define DSSDBG(format, ...)
>> -#define DSSDBGF(format, ...)
>> +#ifdef DSS_SUBSYS_NAME
>> +#define pr_fmt(fmt) DSS_SUBSYS_NAME ": " fmt
>> +#else
>> +#define pr_fmt(fmt) fmt
>> #endif
>
> I think you could just do:
>
> #ifdef DSS_SUBSYS_NAME
> #ifdef pr_fmt
> #undef pr_fmt
> #endif
> #define pr_fmt(fmt) DSS_SUBSYS_NAME ": " fmt
> #endif
>
> For the case where there's no DSS_SUBSYS_NAME, there's no need to undef
> pr_fmt, only to redefine it again back to the original.
>
> Tomi
>
Ok. But I thought it could be more clearer if the definition of pr_fmt
is more clearer in both the cases, when DSS_SUBSYS_NAME is defined and
when not.
--
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
