| Message ID | 1420716572-23826-1-git-send-email-michal.kazior@tieto.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
Michal Kazior <michal.kazior@tieto.com> writes: > The wmi-tlv firmware contains additional > versioning info. It may help reporting/debugging. > > Signed-off-by: Michal Kazior <michal.kazior@tieto.com> [...] > 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.
On 12 January 2015 at 13:56, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> [...] >> 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?
Michal Kazior <michal.kazior@tieto.com> writes: > On 12 January 2015 at 13:56, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> > [...] >>> 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.
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);
The wmi-tlv firmware contains additional versioning info. It may help reporting/debugging. Signed-off-by: Michal Kazior <michal.kazior@tieto.com> --- 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(-)