2015-01-08 11:29:51

by Michal Kazior

[permalink] [raw]
Subject: [PATCH] ath10k: add additional fw build version to info print

The wmi-tlv firmware contains additional
versioning info. It may help reporting/debugging.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 4 ++++
drivers/net/wireless/ath/ath10k/debug.c | 8 ++++++--
drivers/net/wireless/ath/ath10k/wmi.c | 4 ++++
3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 7b6d9e4..aa12c8a 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -458,6 +458,10 @@ struct ath10k {
u32 fw_version_minor;
u16 fw_version_release;
u16 fw_version_build;
+ u32 fw_build_major;
+ u32 fw_build_minor;
+ u32 fw_build_si;
+ u32 fw_build_crm;
u32 phy_capability;
u32 hw_min_tx_power;
u32 hw_max_tx_power;
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 6ca2442..9a6b358 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -124,7 +124,7 @@ EXPORT_SYMBOL(ath10k_info);

void ath10k_print_driver_info(struct ath10k *ar)
{
- ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d cal %s max_sta %d\n",
+ ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d cal %s max_sta %d build %u.%u.%u.%u\n",
ar->hw_params.name,
ar->target_version,
ar->chip_id,
@@ -134,7 +134,11 @@ void ath10k_print_driver_info(struct ath10k *ar)
ar->htt.target_version_minor,
ar->wmi.op_version,
ath10k_cal_mode_str(ar->cal_mode),
- ar->max_num_stations);
+ ar->max_num_stations,
+ ar->fw_build_major,
+ ar->fw_build_minor,
+ ar->fw_build_si,
+ ar->fw_build_crm);
ath10k_info(ar, "debug %d debugfs %d tracing %d dfs %d testmode %d\n",
config_enabled(CONFIG_ATH10K_DEBUG),
config_enabled(CONFIG_ATH10K_DEBUGFS),
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index ac74290..8c26c2a 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2907,6 +2907,10 @@ void ath10k_wmi_event_service_ready(struct ath10k *ar, struct sk_buff *skb)
ar->fw_version_release =
(__le32_to_cpu(arg.sw_ver1) & 0xffff0000) >> 16;
ar->fw_version_build = (__le32_to_cpu(arg.sw_ver1) & 0x0000ffff);
+ ar->fw_build_major = (__le32_to_cpu(arg.fw_build) & 0xf0000000) >> 28;
+ ar->fw_build_minor = (__le32_to_cpu(arg.fw_build) & 0x0f000000) >> 24;
+ ar->fw_build_si = (__le32_to_cpu(arg.fw_build) & 0x00f00000) >> 20;
+ ar->fw_build_crm = (__le32_to_cpu(arg.fw_build) & 0x00007fff) >> 0;
ar->phy_capability = __le32_to_cpu(arg.phy_capab);
ar->num_rf_chains = __le32_to_cpu(arg.num_rf_chains);
ar->ath_common.regulatory.current_rd = __le32_to_cpu(arg.eeprom_rd);
--
1.8.5.3



2015-01-12 13:10:43

by Michal Kazior

[permalink] [raw]
Subject: Re: [PATCH] ath10k: add additional fw build version to info print

On 12 January 2015 at 13:56, Kalle Valo <[email protected]> wrote:
>> Signed-off-by: Michal Kazior <[email protected]>
[...]
>> void ath10k_print_driver_info(struct ath10k *ar)
>> {
>> - ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d cal %s max_sta %d\n",
>> + ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d cal %s max_sta %d build %u.%u.%u.%u\n",
[...]
>
> So now we print two different firmware versions to the user? That's just
> confusing. It's ok to print this build id in debug level, but not in
> info level. From user's point of view we should have only one firmware
> version.

I guess we shouldn't be printing htt version then too. Nevertheless I
understand your concern.


> I would rather make sure that ATH10K_FW_IE_FW_VERSION always contains
> the correct version string and then developers map whatever they need
> from that string.

Right. Let's drop this for now then.


MichaƂ

2015-01-12 12:56:34

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: add additional fw build version to info print

Michal Kazior <[email protected]> writes:

> The wmi-tlv firmware contains additional
> versioning info. It may help reporting/debugging.
>
> Signed-off-by: Michal Kazior <[email protected]>

[...]

> void ath10k_print_driver_info(struct ath10k *ar)
> {
> - ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d cal %s max_sta %d\n",
> + ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d cal %s max_sta %d build %u.%u.%u.%u\n",
> ar->hw_params.name,
> ar->target_version,
> ar->chip_id,
> @@ -134,7 +134,11 @@ void ath10k_print_driver_info(struct ath10k *ar)
> ar->htt.target_version_minor,
> ar->wmi.op_version,
> ath10k_cal_mode_str(ar->cal_mode),
> - ar->max_num_stations);
> + ar->max_num_stations,
> + ar->fw_build_major,
> + ar->fw_build_minor,
> + ar->fw_build_si,
> + ar->fw_build_crm);

So now we print two different firmware versions to the user? That's just
confusing. It's ok to print this build id in debug level, but not in
info level. From user's point of view we should have only one firmware
version.

I would rather make sure that ATH10K_FW_IE_FW_VERSION always contains
the correct version string and then developers map whatever they need
from that string.

--
Kalle Valo

2015-01-12 13:20:53

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: add additional fw build version to info print

Michal Kazior <[email protected]> writes:

> On 12 January 2015 at 13:56, Kalle Valo <[email protected]> wrote:
>>> Signed-off-by: Michal Kazior <[email protected]>
> [...]
>>> void ath10k_print_driver_info(struct ath10k *ar)
>>> {
>>> - ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d cal %s max_sta %d\n",
>>> + ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d cal %s max_sta %d build %u.%u.%u.%u\n",
> [...]
>>
>> So now we print two different firmware versions to the user? That's just
>> confusing. It's ok to print this build id in debug level, but not in
>> info level. From user's point of view we should have only one firmware
>> version.
>
> I guess we shouldn't be printing htt version then too. Nevertheless I
> understand your concern.

Yeah, but htt version is an interface version and I'm having a hard time
to believe that someone will mistake that as the actual firmware
version.

But what people confuse is the FW API version. When I ask what firmware
version you have way too often I get "firmware-3.bin" as a reply :) But
I have no ideas how to solve that.

--
Kalle Valo