Hi Joe, Thanks for your comments. > -----Original Message----- > From: Joe Perches [mailto:joe@xxxxxxxxxxx] > Sent: Thursday, March 03, 2011 12:07 AM > To: Bing Zhao > Cc: linux-wireless@xxxxxxxxxxxxxxx; John W. Linville; Johannes Berg; Amitkumar Karwar; Kiran Divekar; > Yogesh Powar; Marc Yang; Frank Huang > Subject: Re: [PATCH v2] mwifiex: remove PRINTM/HEXDUMP and associated > > On Wed, 2011-03-02 at 14:31 -0800, Bing Zhao wrote: > > use pr_debug family APIs instead. > > MERROR/MFATAL/MMSG: pr_err, pr_warning, pr_notice > > Trivia: > > It looks to me as if all the MFATAL should > map to KERN_ERR. Yes. MERROR & MFATAL: mapped to pr_err using KERN_ERR. MMSG: mapped to pr_err, pr_warn, or pr_notice. I will make the description more clear. > > Please use pr_fmt > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > to prefix "mwifiex: " rather than put it each > format string. Make sense. Thanks for the tips. > > Please use pr_warn not pr_warning. OK. > > I really don't think you need to add another prefix > string like "err" and "info" and "cmd" to the various > levels. The output content is enough to figure it out. The purpose of these prefix levels (data/cmd/event/int etc.) is for selective printing using dynamic debug. This way I can enable any combinations of them. For example, I can do "format 'mwifiex cmd' +p" and "format 'mwifiex event' +p" to enable debugging for "cmd" and "event" only. > > Perhaps use use dev/netdev/wiphy_<level> where it makes > sense? I thought that too. But these APIs require a parameter of "struct device *". Some of mwifiex_ functions do not have this parameter. > > You've sometimes added __func__ and sometimes used or > kept some shortened function name equivalent. It'd > probably be better to consistently use __func__ where > appropriate. OK. Thanks, Bing ÿô.nlj·Ÿ®‰†+%ŠË±é¥Šwÿº{.nlj·¥Š{±ÿ«zW¬³ø¡Ü}©ž²ÆzÚj:+v‰¨þø®w¥þŠàÞ¨è&¢)ß«a¶Úÿûz¹ÞúŽŠÝjÿŠwèf
