Re: [PATCH v3 2/7] ath10k: add ATH10K_FW_IE_WMI_OP_VERSION

[Kalle Valo <kvalo@qca.qualcomm.com>]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 1 December 2014 at 15:45, Kalle Valo <kvalo@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