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

Eugene Krasnikov k.eugene.e at gmail.com
Wed Aug 21 03:55:10 EDT 2013


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



More information about the wcn36xx mailing list