Re: [PATCH v3 2/7] ath10k: add ATH10K_FW_IE_WMI_OP_VERSION
On 1 December 2014 at 15:45, Kalle Valo kv...@qca.qualcomm.com wrote:
[...]
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 514c219263a5..92b04fe73151 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -120,6 +120,7 @@ struct ath10k_mem_chunk {
};
struct ath10k_wmi {
+ unsigned int op_version;
I wonder - can't we have this as `enum ath10k_fw_wmi_op_version op_version;` ?
@@ -378,8 +379,9 @@ enum ath10k_fw_features {
/* Firmware does not support P2P */
ATH10K_FW_FEATURE_NO_P2P = 3,
- /* Firmware 10.2 feature bit. The ATH10K_FW_FEATURE_WMI_10X feature
bit
-* is required to be set as well.
+ /* Firmware 10.2 feature bit. The ATH10K_FW_FEATURE_WMI_10X feature
+* bit is required to be set as well. Deprecated, don't use in new
+* code.
Just out of curiosity - any plans how long this is going to be
depracated until removed/replaced?
*/
ATH10K_FW_FEATURE_WMI_10_2 = 4,
diff --git a/drivers/net/wireless/ath/ath10k/hw.h
b/drivers/net/wireless/ath/ath10k/hw.h
index dfedfd0e0f34..04aaf9af3ca0 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -58,6 +58,16 @@ enum ath10k_fw_ie_type {
ATH10K_FW_IE_FEATURES = 2,
ATH10K_FW_IE_FW_IMAGE = 3,
ATH10K_FW_IE_OTP_IMAGE = 4,
+
+ /* WMI operations interface version, 32 bit value. Supported from
+* FW API 4 and above. */
+ ATH10K_FW_IE_WMI_OP_VERSION = 5,
Hmm.. shouldn't we bump up the firmware filename from -3 to -4 and try
loading 4..3..2..1?
Michał
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/7] ath10k: add ATH10K_FW_IE_WMI_OP_VERSION
Michal Kazior michal.kaz...@tieto.com writes:
On 1 December 2014 at 15:45, Kalle Valo kv...@qca.qualcomm.com wrote:
[...]
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 514c219263a5..92b04fe73151 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -120,6 +120,7 @@ struct ath10k_mem_chunk {
};
struct ath10k_wmi {
+ unsigned int op_version;
I wonder - can't we have this as `enum ath10k_fw_wmi_op_version op_version;` ?
Oh yes, I was even planning to change it but apparently forgot. Will fix
in v4.
@@ -378,8 +379,9 @@ enum ath10k_fw_features {
/* Firmware does not support P2P */
ATH10K_FW_FEATURE_NO_P2P = 3,
- /* Firmware 10.2 feature bit. The ATH10K_FW_FEATURE_WMI_10X feature
bit
-* is required to be set as well.
+ /* Firmware 10.2 feature bit. The ATH10K_FW_FEATURE_WMI_10X feature
+* bit is required to be set as well. Deprecated, don't use in new
+* code.
Just out of curiosity - any plans how long this is going to be
depracated until removed/replaced?
So there are two parts:
1) In the driver we should not use these deprated flags with new code,
and preferably remove existing uses bit by bit. But for backwards
compatibility we will keep the flags in
ath10k_core_init_firmware_features() so that we can set correct
wmi.op_version when using firmware images which don't have
ATH10K_FW_IE_WMI_OP_VERSION.
2) In firmware images we will continue use the deprecated flags with
main, 10.1 and 10.2 firmware branches, at least for the time being.
This makes it possible to have new firmware releases working on older
drivers.
diff --git a/drivers/net/wireless/ath/ath10k/hw.h
b/drivers/net/wireless/ath/ath10k/hw.h
index dfedfd0e0f34..04aaf9af3ca0 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -58,6 +58,16 @@ enum ath10k_fw_ie_type {
ATH10K_FW_IE_FEATURES = 2,
ATH10K_FW_IE_FW_IMAGE = 3,
ATH10K_FW_IE_OTP_IMAGE = 4,
+
+ /* WMI operations interface version, 32 bit value. Supported from
+* FW API 4 and above. */
+ ATH10K_FW_IE_WMI_OP_VERSION = 5,
Hmm.. shouldn't we bump up the firmware filename from -3 to -4 and try
loading 4..3..2..1?
I was thinking that we add FW API 4 only once we have all necessary
changes for the new hw. For example, in case we need HTT changes as
well.
--
Kalle Valo
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/7] ath10k: add ATH10K_FW_IE_WMI_OP_VERSION
On 1 December 2014 at 15:45, Kalle Valo kv...@qca.qualcomm.com wrote: [...] @@ -562,6 +562,17 @@ static int ath10k_core_fetch_firmware_api_n(struct ath10k *ar, const char *name) ar-otp_len = ie_len; break; + case ATH10K_FW_IE_WMI_OP_VERSION: + if (ie_len != sizeof(u32)) + break; + + version = (__le32 *)data; + + ar-wmi.op_version = le32_to_cpup(version); + + ath10k_dbg(ar, ATH10K_DBG_BOOT, found fw ie wmi op version %d\n, + ar-wmi.op_version); Hmm.. wouldn't it make sense to have a ATH10K_FW_WMI_OP_VERSION_MAX (or _LAST) and verify if the IE value doesn't exceed the last known value? This way switch()es won't need default handler by design and thus we'll get the benefit of compiler telling us about unhandled cases when we add new enums. Michał -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/7] ath10k: add ATH10K_FW_IE_WMI_OP_VERSION
On 1 December 2014 at 15:45, Kalle Valo kv...@qca.qualcomm.com wrote:
[...]
@@ -801,11 +812,24 @@ static int ath10k_core_init_firmware_features(struct
ath10k *ar)
}
if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar-fw_features)) {
- ar-max_num_peers = TARGET_10X_NUM_PEERS;
- ar-max_num_stations = TARGET_10X_NUM_STATIONS;
+ if (test_bit(ATH10K_FW_FEATURE_WMI_10_2, ar-fw_features))
+ ar-wmi.op_version = ATH10K_FW_WMI_OP_VERSION_10_2;
+ else
+ ar-wmi.op_version = ATH10K_FW_WMI_OP_VERSION_10_1;
} else {
+ ar-wmi.op_version = ATH10K_FW_WMI_OP_VERSION_MAIN;
+ }
You always overwrite ar-wmi.op_version with MAIN if it's not 10.x
which means TLV is/wiil be effectively overwritten.
Perhaps the op_version enum values should start with 1 so that 0 can
be used as unset and only in that case should the above fallback be
attempted.
Also, I think it might be a good idea to reset ar-wmi.op_version in
ath10k_core_fetch_firmware_api_n (and api_1 as well) so that if any
attempt fails (e.g. due to IE binary corruption) ar-wmi.op_version
isn't propagated/left unchanged to another fw API attempt.
Michał
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/7] ath10k: add ATH10K_FW_IE_WMI_OP_VERSION
Michal Kazior michal.kaz...@tieto.com writes:
On 1 December 2014 at 15:45, Kalle Valo kv...@qca.qualcomm.com wrote:
[...]
@@ -801,11 +812,24 @@ static int ath10k_core_init_firmware_features(struct
ath10k *ar)
}
if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar-fw_features)) {
- ar-max_num_peers = TARGET_10X_NUM_PEERS;
- ar-max_num_stations = TARGET_10X_NUM_STATIONS;
+ if (test_bit(ATH10K_FW_FEATURE_WMI_10_2, ar-fw_features))
+ ar-wmi.op_version = ATH10K_FW_WMI_OP_VERSION_10_2;
+ else
+ ar-wmi.op_version = ATH10K_FW_WMI_OP_VERSION_10_1;
} else {
+ ar-wmi.op_version = ATH10K_FW_WMI_OP_VERSION_MAIN;
+ }
You always overwrite ar-wmi.op_version with MAIN if it's not 10.x
which means TLV is/wiil be effectively overwritten.
Perhaps the op_version enum values should start with 1 so that 0 can
be used as unset and only in that case should the above fallback be
attempted.
That's a good idea, I'll add that.
Also, I think it might be a good idea to reset ar-wmi.op_version in
ath10k_core_fetch_firmware_api_n (and api_1 as well) so that if any
attempt fails (e.g. due to IE binary corruption) ar-wmi.op_version
isn't propagated/left unchanged to another fw API attempt.
Yeah, we should do that. But that's an existing problem, not something
introduced by this patchset (we have the same problem with
ar-fw_features) . So I would like to fix that later and added it to the
TODO list:
http://wireless.kernel.org/en/users/Drivers/ath10k/todo
--
Kalle Valo
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/7] ath10k: add ATH10K_FW_IE_WMI_OP_VERSION
Michal Kazior michal.kaz...@tieto.com writes: On 1 December 2014 at 15:45, Kalle Valo kv...@qca.qualcomm.com wrote: [...] @@ -562,6 +562,17 @@ static int ath10k_core_fetch_firmware_api_n(struct ath10k *ar, const char *name) ar-otp_len = ie_len; break; + case ATH10K_FW_IE_WMI_OP_VERSION: + if (ie_len != sizeof(u32)) + break; + + version = (__le32 *)data; + + ar-wmi.op_version = le32_to_cpup(version); + + ath10k_dbg(ar, ATH10K_DBG_BOOT, found fw ie wmi op version %d\n, + ar-wmi.op_version); Hmm.. wouldn't it make sense to have a ATH10K_FW_WMI_OP_VERSION_MAX (or _LAST) and verify if the IE value doesn't exceed the last known value? This way switch()es won't need default handler by design and thus we'll get the benefit of compiler telling us about unhandled cases when we add new enums. A good idea, I'll add that. -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
