At Sun, 23 Nov 2014 13:40:51 +0530, Sudip Mukherjee wrote: > > From: Sudip Mukherjee <su...@vectorindia.org> > > replaced all references of the debug messages via printk > with dev_* macro (mostly dev_dbg). > one reference was changed to pr_err as there the card might have been > uninitialized. > > this patch will generate warning from checkpatch about broken quoted > strings. but that was not fixed intentionally to improve the > readability. > > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org> > --- > > change in v2: the build warnings of v1 are fixed. > > hi Takashi, > in your review of v1, you said about some lines which are not ending > with \n. but i was not able to find them. did i miss them somewhere?
The problem is the one with multiple "\n", for example: dev_dbg(korg1212->card->dev, "dspMemPhy = %08x U[%08x], " "PlayDataPhy = %08x L[%08x]\n" "korg1212: RecDataPhy = %08x L[%08x], " "VolumeTablePhy = %08x L[%08x]\n" "korg1212: RoutingTablePhy = %08x L[%08x], " "AdatTimeCodePhy = %08x L[%08x]\n", My biggest concern right now is, however, about the unnecessary code increase by this patch. Currently, most of debug prints were simply not built, because of: > // > ---------------------------------------------------------------------------- > -// Debug Stuff > -// > ---------------------------------------------------------------------------- > -#define K1212_DEBUG_LEVEL 0 > -#if K1212_DEBUG_LEVEL > 0 > -#define K1212_DEBUG_PRINTK(fmt,args...) printk(KERN_DEBUG fmt,##args) > -#else > -#define K1212_DEBUG_PRINTK(fmt,...) > -#endif > -#if K1212_DEBUG_LEVEL > 1 > -#define K1212_DEBUG_PRINTK_VERBOSE(fmt,args...) printk(KERN_DEBUG > fmt,##args) > -#else > -#define K1212_DEBUG_PRINTK_VERBOSE(fmt,...) > -#endif With your patch, now all these codes are compiled. I have no clear answer what would be the best in such a case. I'd say it really depends. If they are just silly messages that can be covered in a better way (like ftrace), just get rid of them. If they are intended for some good register dumps, then dev_dbg() might make sense. Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/