Hi Joe, Thank you very much for your comments. Please find my comments bellow: >> Adding wcn36xx.h > [] >> +#define DRIVER_PREFIX "wcn36xx: " > [] > > I think you should use pr_fmt and/or netdev_<level> Thanx, will add pr_fmt in the next round. >> +#define wcn36xx_error(fmt, arg...) do { \ >> + pr_err(DRIVER_PREFIX "ERROR " fmt "\n", ##arg); \ >> + __WARN(); \ >> +} while (0) > > What value is there in this __WARN? This warn is there just to scream louder every time when error happens:) > Please use _err rather than _error Thanx, will fix in the next patch set. >> +#define wcn36xx_warn(fmt, arg...) \ >> + pr_warn(DRIVER_PREFIX "WARNING " fmt "\n", ##arg) >> + >> +#define wcn36xx_info(fmt, arg...) \ >> + pr_info(DRIVER_PREFIX fmt "\n", ##arg) >> + >> +#define wcn36xx_dbg(mask, fmt, arg...) do { \ >> + if (debug_mask & mask) \ >> + pr_debug(DRIVER_PREFIX fmt "\n", ##arg); \ >> +} while (0) >> + >> +#define wcn36xx_dbg_dump(mask, prefix_str, buf, len) do { \ >> + if (debug_mask & mask) \ >> + print_hex_dump(KERN_DEBUG, prefix_str, \ >> + DUMP_PREFIX_OFFSET, 32, 1, \ >> + buf, len, false); \ >> +} while (0) >> + > > Please move the "\n" to the uses instead of the macro. > This would be consistent with all the other ath macros. Thanx, will fix in the next patch set. -- Best regards, Eugene -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html