WMI version handling

[Michal Kazior]

On 3 September 2014 17:00, Ben Greear <greearb at candelatech.com> wrote:
> On 09/02/2014 11:14 PM, Kalle Valo wrote:
>> Hi,
[...]
>> Also I have been thinking that using firmware feature bits (for example
>> ATH10K_FW_FEATURE_WMI_10_2 and ATH10K_FW_FEATURE_WMI_10X) for WMI
>> version is not the best way. I think it's easier to manage all this if
>> we have a u32 FW IE to provide WMI version. IIRC Ben was suggesting this
>> at some point.
>>
>> Example:
>>
>> enum ath10k_fw_wmi_version {
>>       ATH10K_FW_WMI_VERSION_MAIN = 0,
>>       ATH10K_FW_WMI_VERSION_10_1 = 1,
>>       ATH10K_FW_WMI_VERSION_10_2 = 2,
>> }
>>
>> And then wmi.c would set correct interface based on this version.
>
> I am happy with the current flags, it seems to work well enough.

I'm actually okay with this but I'd actually throw out the "wmi"
string from it and leave out just "ath10k_fw_version". I've recently
discovered some minor HTT discrepancies between fw branches so we
might want to use the version tag to pick HTT "backend" as well..


> I'd leave policy out of the IEs as much as possible, as normal users
> cannot modify it, and certainly it would be a problem if you wanted
> different policy with different NICs in same system.
> IEs should describe the minimum needed for the
> ath10k to enforce policy as best as it can.  We have to assume that
> users tweaking station and peer count can deal with fw crashes until
> they get the settings right.
>
> My CT firmware will print out available RAM once booted, so it is
> fairly easy to utilize maximum amount of resources with a bit of
> trial and error.  It would be trivial to make upstream firmware do
> the same, of course, but upstream firmware may crash for other
> reasons as you change resource config...

IOW You're okay if ath10k applies default/hardcoded configuration
values of 10.1 for your firmware (as your firmware is going to
advertise as a 10.1) - is that correct? You just want to be able to
tune these values per-device in runtime. I think debugfs is the best
candidate here then.

ath10k would need to have a separate debugfs entry for that then, i.e.
one that is independent from wiphy. You could have, e.g.
/sys/kernel/debug/ath10k/pci/0000:04:00.0/num_peers which re-registers
to mac80211 upon modification.


>> We would still use feature bits to enable and disable smaller changes
>> like ATH10K_FW_FEATURE_HAS_WMI_MGMT_TX does. But for bigger WMI changes
>> we would change the WMI version, for example if we have a new firmware
>> branch with significant changes or similar.
>>
>> And for backwards comptability we need to do so that
>> ATH10K_FW_FEATURE_WMI_10X sets ATH10K_FW_WMI_VERSION_10_1 and
>> ATH10K_FW_FEATURE_WMI_10_2 sets ATH10K_FW_WMI_VERSION_10_2.
>
> I was able to do all of my changes w/out needing new a new WMI version,
> and I think that is the right approach.  I do not like large amounts of
> duplicated code and table lookups, which is what new WMI versions appear
> to imply.

It's actually really simple to update WMI in an ABI-safe way.
Unfortunately official firmware updates break it fairly often and we
have to deal with that. Aaand there's going to be another WMI backend
soon most likely.


[...]
> If QCA goes to more of a rolling release for firmware, like I do for CT
> firmware, then we may want to add IEs a bit more often, to notify things like
> "can-support-feature-foo".  10.2 v/s 10.1 can still determine the
> over-all WMI api, perhaps, but at specific points we can take advantage of
> specific firmware features based on IE flag.

This is what WMI bitmap service is actually for. ath10k is ignoring
this now and exposes it via debugfs only. It's possible to, e.g. pick
"max num of stations" configuration values based on the "iram tids"
being set or not.


Michał