Re: [PATCH 13/16] wcn36xx: Add wcn36xx.h

[Joe Perches <joe@perches.com>]

On Tue, 2013-08-20 at 19:41 +0200, Eugene Krasnikov wrote:
> Adding wcn36xx.h
[]
> +#define DRIVER_PREFIX "wcn36xx: "
[]

I think you should use pr_fmt and/or netdev_<level>

> +#define wcn36xx_error(fmt, arg...) do {			\
> +	pr_err(DRIVER_PREFIX "ERROR " fmt "\n", ##arg);	\
> +	__WARN();					\
> +} while (0)

What value is there in this __WARN?

Please use _err rather than _error

> +#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.