Messages in this thread | | | From | Kalle Valo <> | Subject | Re: ath10k: mac80211 driver for Qualcomm Atheros 802.11ac CQA98xx devices | Date | Thu, 11 Jul 2013 11:16:34 +0300 |
| |
Geert Uytterhoeven <geert@linux-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 linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |