Hi Samuel.Ortiz, Thanks your review.
-#else > #define A_LOGGER(mask, mod, args...) printk(KERN_ALERT args) >I would rather use KERN_DEBUG or KERN_INFO here. This is for Android and I would remove it from meego.kernel. Because We won't use it. > +#define A_PRINTF(args...) printk(KERN_ERR args) > So all A_PRINTF will be errors ? Here as well, KERN_DEBUG or KERN_INFO would > make more sense to me. All A_PRINTF(), I will change it to AR_DEBUG_PRINTF(). So, we can use debug level to control it. -----Original Message----- From: Samuel Ortiz [mailto:sa...@linux.intel.com] Sent: Tuesday, June 14, 2011 9:21 PM To: Chang, Samuel Cc: Kok, Auke-jan H; Wu, Ricky; Huang, Maggie; meego-kernel@lists.meego.com; Yang, Jack Subject: Re: [Meego-kernel] [PATCH] ar6003 : reduce unnecessary log during driver loading V2 Hi Samuel, On Thu, Jun 09, 2011 at 05:31:31PM +0000, Chang, Samuel wrote: > Subject: Unnecessary log during driver loading, set the debug level to INFO > and modify printk level. > 1. Remove unnecessary log functions > 2. Drive WARN log to printk(KERN_WARNING) to instead of > printk(KER_ALERT) > 3. Driver ERROR log to printk(KERN_ERR) to instead of > printk(KER_ALERT) That looks better, but the first version of your patch already got merged into the MeeGo mrst adaptation kernel, so you should build a diff against it. Your patch should then only contain the osapi_linux.h macro fixes. Some additional comments: > diff -ruN > kernel-2.6.37.6-11.5/drivers/staging/ar6003/os/linux/include/osapi_linux.h > kernel-2.6.37.6-11.5_log_reduced_V2_patch/drivers/staging/ar6003/os/linux/include/osapi_linux.h > --- > kernel-2.6.37.6-11.5/drivers/staging/ar6003/os/linux/include/osapi_linux.h > 2011-06-10 00:28:40.483583244 +0800 > +++ > kernel-2.6.37.6-11.5_log_reduced_V2_patch/drivers/staging/ar6003/os/linux/include/osapi_linux.h > 2011-06-10 00:55:10.783469114 +0800 > @@ -103,56 +103,10 @@ > #define A_NETIF_RX_NI(skb) netif_rx_ni(skb) > #endif > > -#if defined(ANDROID_ENV) && defined(CONFIG_ANDROID_LOGGER) -extern > unsigned int enablelogcat; -extern int android_logger_lv(void* module, > int mask); -enum logidx { LOG_MAIN_IDX = 0 }; -extern int > logger_write(const enum logidx idx, > - const unsigned char prio, > - const char __kernel * const tag, > - const char __kernel * const fmt, > - ...); > -#define A_ANDROID_PRINTF(mask, module, tags, args...) do { \ > - if (enablelogcat) \ > - logger_write(LOG_MAIN_IDX, android_logger_lv(module, mask), tags, > args); \ > - else \ > - printk(KERN_ALERT args); \ > -} while (0) > -#ifdef DEBUG > -#define A_LOGGER_MODULE_NAME(x) #x > -#define A_LOGGER(mask, mod, args...) \ > - A_ANDROID_PRINTF(mask, &GET_ATH_MODULE_DEBUG_VAR_NAME(mod), "ar6k_" > A_LOGGER_MODULE_NAME(mod), args); > -#endif > -#define A_PRINTF(args...) A_ANDROID_PRINTF(ATH_DEBUG_INFO, NULL, > "ar6k_driver", args) -#else > #define A_LOGGER(mask, mod, args...) printk(KERN_ALERT args) I would rather use KERN_DEBUG or KERN_INFO here. > -#define A_PRINTF(args...) printk(KERN_ALERT args) > -#endif /* ANDROID */ > -#if defined(MEEGO_ENV) && defined(CONFIG_MEEGO_LOGGER) -extern > unsigned int enablelogcat; -extern int meego_logger_lv(void* module, > int mask); -enum logidx { LOG_MAIN_IDX = 0 }; -extern int > logger_write(const enum logidx idx, > - const unsigned char prio, > - const char __kernel * const tag, > - const char __kernel * const fmt, > - ...); > -#define A_MEEGO_PRINTF(mask, module, tags, args...) do { \ > - if (enablelogcat) \ > - logger_write(LOG_MAIN_IDX, meego_logger_lv(module, mask), tags, > args); \ > - else \ > - printk(KERN_ALERT args); \ > -} while (0) > -#ifdef DEBUG > -#define A_LOGGER_MODULE_NAME(x) #x > -#define A_LOGGER(mask, mod, args...) \ > - A_MEEGO_PRINTF(mask, &GET_ATH_MODULE_DEBUG_VAR_NAME(mod), "ar6k_" > A_LOGGER_MODULE_NAME(mod), args); > -#endif > -#define A_PRINTF(args...) A_MEEGO_PRINTF(ATH_DEBUG_INFO, NULL, > "ar6k_driver", args) -#else > -#define A_LOGGER(mask, mod, args...) printk(KERN_ALERT args) > -#define A_PRINTF(args...) printk(KERN_ALERT args) > -#endif /* MEEGO */ > +#define A_PRINTF(args...) printk(KERN_ERR args) So all A_PRINTF will be errors ? Here as well, KERN_DEBUG or KERN_INFO would make more sense to me. Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ _______________________________________________ MeeGo-kernel mailing list MeeGo-kernel@lists.meego.com http://lists.meego.com/listinfo/meego-kernel
