DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CEB4820835
Subject: Re: [PATCH 05/19] wilc: add wilc_wlan_if.h
To:     Johannes Berg <johannes@sipsolutions.net>,
        <linux-wireless@vger.kernel.org>
CC:     <kvalo@codeaurora.org>, <gregkh@linuxfoundation.org>,
        <ganesh.krishna@microchip.com>, <aditya.shankar@microchip.com>,
        <venkateswara.kaja@microchip.com>, <claudiu.beznea@microchip.com>,
        <adham.abozaeid@microchip.com>
References: <1537957525-11467-1-git-send-email-ajay.kathat@microchip.com>
 <1537957525-11467-6-git-send-email-ajay.kathat@microchip.com>
 <1539009224.3687.68.camel@sipsolutions.net>
From:   Ajay Singh <ajay.kathat@microchip.com>
Message-ID: <c48810b4-11ed-4df4-2bb6-d9725406667f@microchip.com>
Date:   Thu, 11 Oct 2018 12:29:17 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <1539009224.3687.68.camel@sipsolutions.net>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Received-SPF: None (protection.outlook.com: microchip.com does not designate
 permitted sender hosts)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Oct 2018 06:59:38.1777 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: da2f03ad-9e2d-4d15-9d79-08d62f471e6c
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB2566
X-OriginatorOrg: microchip.com
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org


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