Re: ath10k: mac80211 driver for Qualcomm Atheros 802.11ac CQA98xxdevices

[Geert Uytterhoeven]

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

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

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).

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.

> +	snprintf(version, sizeof(version), "%u.%u.%u.%u",
> +		 SUPPORTED_FW_MAJOR, SUPPORTED_FW_MINOR,
> +		 SUPPORTED_FW_RELEASE, SUPPORTED_FW_BUILD);
> +
> +	ath10k_warn("WARNING: Firmware version %s is not officially supported.\n",
> +		    ar->hw->wiphy->fw_version);
> +	ath10k_warn("Please upgrade to version %s (or newer)\n", version);
> +
> +	return 0;
> +}

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/