Re: [PATCH V2 0/2] OMAPDSS: Enable dynamic debug printing

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

Hi,

On Wed, 2012-09-26 at 10:45 +0530, Chandrabhanu Mahapatra wrote:
> Hi everyone,
> this patch series aims at cleaning up of DSS of printk()'s enabled with
> dss_debug and replace them with generic dynamic debug printing.
> 
> The 1st patch 
>  * replaces printk() in DSSDBG definition with pr_debug()
>  * removes DSSDBGF definition and replaces its instances with DSSDBG() 
> The 2nd patch
>  * cleans up printk()'s in omap_dispc_unregister_isr() and 
>    _dsi_print_reset_status() with pr_debug()
>  * removes dss_debug variable
> 
> Changes with respect to V1:
>  * added debug messages to DSSDBG calls replacing DSSDBGF
>  * added patch "OMAPDSS: Remove dss_debug variable"
> 
> All your comments and suggestions are welcome.

This doesn't work quite correctly. The problem is in dss.h, where we
define DEBUG if CONFIG_OMAP2_DSS_DEBUG_SUPPORT is set. The thing is,
DEBUG should be defined before including the kernel headers where the
pr_debug etc are defined.

So if you try the patches without dynamic debugging enabled, you won't
get any debug outputs at all, even if CONFIG_OMAP2_DSS_DEBUG_SUPPORT is
set.

And for dynamic debug, the Kconfig help says:

If a source file is compiled with DEBUG flag set, any       
pr_debug() calls in it are enabled by default, but can be   
disabled at runtime as below.  Note that DEBUG flag is      
turned on by many CONFIG_*DEBUG* options.                   

So if we have CONFIG_OMAP2_DSS_DEBUG_SUPPORT set, all the pr_debugs
should be enabled by default, which is not the case, again because DEBUG
is defined too late.

I think setting DEBUG in dss.h should be removed, and instead DEBUG
should be set in the makefile if CONFIG_OMAP2_DSS_DEBUG_SUPPORT is set.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]