On 10/8/2018 8:03 PM, Johannes Berg wrote: >> +#define WILC_TX_ERR_NO_BUF (-2) > Hmm? what's wrong with just e.g. -ENOBUFS? If it doesn't go to userspace > it doesn't matter, and if it does you can't use this anyway? This would > be -ENOENT which is a bad idea. > Actually this value doesn't return to userspace and its used locally for the error status. But anyway we can make use of (-ENOBUFS) instead of adding this new macro. >> + >> +/******************************************** >> + * >> + * Wlan Configuration ID >> + * >> + ********************************************/ >> +#define WILC_MULTICAST_TABLE_SIZE 8 >> +#define MAX_SSID_LEN 33 > Err, it's 32? > >> +#define MAX_RATES_SUPPORTED 12 >> + >> +enum bss_types { >> + INFRASTRUCTURE = 0, >> + INDEPENDENT, >> + AP, >> +}; >> + >> +enum { >> + B_ONLY_MODE = 0, /* 1, 2 M, otherwise 5, 11 M */ >> + G_ONLY_MODE, /* 6,12,24 otherwise 9,18,36,48,54 */ >> + G_MIXED_11B_1_MODE, /* 1,2,5.5,11 otherwise all on */ >> + G_MIXED_11B_2_MODE, /* 1,2,5,11,6,12,24 otherwise all on */ >> +}; >> + >> +enum { >> + G_SHORT_PREAMBLE = 0, /* Short Preamble */ >> + G_LONG_PREAMBLE = 1, /* Long Preamble */ >> + G_AUTO_PREAMBLE = 2, /* Auto Preamble Selection */ >> +}; > here we have a lot of those "constants should have some sort of prefix" > things ... it's not even clear if they're spec or not: > >> +enum authtype { >> + OPEN_SYSTEM = 1, >> + SHARED_KEY = 2, >> + ANY = 3, >> + IEEE8021 = 5 >> +}; > These look like they're spec but aren't ... not a good idea. > Yes, these are not part of any spec . We will add the prefix for better understanding. Regards, Ajay