netdev - Re: ath10k: mac80211 driver for Qualcomm Atheros 802.11ac CQA98xx devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87sizl7bq5.fsf@kamboji.qca.qualcomm.com>
Date:	Thu, 11 Jul 2013 11:16:34 +0300
From:	Kalle Valo <kvalo@....qualcomm.com>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
CC:	"John W. Linville" <linville@...driver.com>,
	<linux-wireless@...r.kernel.org>, <netdev@...r.kernel.org>,
	Linux Kernel Development <linux-kernel@...r.kernel.org>,
	<ath10k@...ts.infradead.org>
Subject: Re: ath10k: mac80211 driver for Qualcomm Atheros 802.11ac CQA98xx devices

Geert Uytterhoeven <geert@...ux-m68k.org> writes:

> On Wed, 10 Jul 2013, Linux Kernel Mailing List wrote:
>> --- /dev/null
>> +++ b/drivers/net/wireless/ath/ath10k/hw.h
>
>> +#define SUPPORTED_FW_MAJOR	1
>> +#define SUPPORTED_FW_MINOR	0
>> +#define SUPPORTED_FW_RELEASE	0
>> +#define SUPPORTED_FW_BUILD	629
>
>> +static int ath10k_check_fw_version(struct ath10k *ar)
>> +{
>> +	char version[32];
>> +
>> +	if (ar->fw_version_major >= SUPPORTED_FW_MAJOR &&
>> +	    ar->fw_version_minor >= SUPPORTED_FW_MINOR &&
>> +	    ar->fw_version_release >= SUPPORTED_FW_RELEASE &&
>> +	    ar->fw_version_build >= SUPPORTED_FW_BUILD)
>> +		return 0;
>
> My attention got triggered by:
>
> drivers/net/wireless/ath/ath10k/core.c: In function ‘ath10k_check_fw_version’:
> drivers/net/wireless/ath/ath10k/core.c:79: warning: comparison is always true due to limited range of data type

I haven't seen this warning, I guess my compiler is too old.

> as an u16 is always larger or equal than zero. Not much you can do to
> silence that warning, though.

Too bad, I really would like to keep ath10k warning free. Easier to
maintain that way.

Does anyone have any nice trick in their sleeves to make this warning go
away? I guess one ugly option is to change u16 to int.

>
> However, I don't think the version check is correct.
> Shouldn't it stop checking later fields if an exact match is found in an
> earlier field?
>
> I.e.
>
> 	if (ar->fw_version_major > SUPPORTED_FW_MAJOR ||
> 	    (ar->fw_version_major == SUPPORTED_FW_MAJOR &&
> 	     ar->fw_version_minor > SUPPORTED_FW_MINOR) ||
> 	     ...) { ... }
>
> Currently e.g. (major, minor) = (3, 0) is considered older than (2, 1).

Doh, that is indeed wrong. I'll fix that, thanks for spotting this.

> Or perhaps minor is never reset to zero when major is increased? In that
> case, the check is correct, but IMHO it's a bit silly to split the version
> number in seperate fields.

No, the firmware engineers are supposed to reset minor version whenever
major changes.

-- 
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html