Re: [PATCH 05/19] wilc: add wilc_wlan_if.h

[Ajay Singh <ajay.kathat@microchip.com>]

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