Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> 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 linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
