On 21 October 2015 at 09:22, Manikanta <manikanta.pubbisetty@xxxxxxxxx> wrote:
> On Wednesday 21 October 2015 12:36 PM, Michal Kazior wrote:
>> On 20 October 2015 at 09:01, Manikanta Pubbisetty
>> <manikanta.pubbisetty@xxxxxxxxx> wrote:
>>> On 10/20/2015 04:55 PM, Kalle Valo wrote:
>>>>
>>>> From: Alan Liu <alanliu@xxxxxxxxxxxxxxxx>
>>>>
>>>> Add WMI-TLV and FW API support in ath10k testmode.
>>>> Ath10k can get right wmi command format from UTF image
>>>> to communicate UTF firmware.
>>>>
>>>> Signed-off-by: Alan Liu <alanliu@xxxxxxxxxxxxxxxx>
>>>> Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxxxx>
>>>> ---
[...]
>>>> + /* loop elements */
>>>> + while (len > sizeof(struct ath10k_fw_ie)) {
>>>> + hdr = (struct ath10k_fw_ie *)data;
>>>> +
>>>> + ie_id = le32_to_cpu(hdr->id);
>>>> + ie_len = le32_to_cpu(hdr->len);
>>>> +
>>>> + len -= sizeof(*hdr);
>>>> + data += sizeof(*hdr);
>>>> +
>>>> + if (len < ie_len) {
>>>> + ath10k_err(ar, "invalid length for FW IE %d (%zu
>>>> <
>>>> %zu)\n",
>>>> + ie_id, len, ie_len);
>>>> + ret = -EINVAL;
>>>> + goto err;
>>>> + }
>>>> +
>>>> + switch (ie_id) {
>>>> + case ATH10K_FW_IE_FW_VERSION:
>>>
>>>
>>> Something like ATH10K_UTF_IE_FW_VERSION or ATH10K_TESTMODE_IE_FW_VERSION
>>> would be less confusing and better, isn't it?
>>
>> I don't really see a reason to. UTF is just another main program to
>> run on the device.
>>
>> If anything I would suggest to unify the FW API parsing logic to work
>> with a dedicated structure instead of `struct ath10k` directly, i.e.
>>
>> struct ath10k_fw {
>> void *data;
>> size_t data_len;
>> enum wmi_op_version wmi_op_version;
>> // ...
>> };
>>
>> int ath10k_core_fw_api_parse(struct ath10k *ar,
>> struct ath10k_fw *arfw,
>> struct firmware *fw)
>> {
>> // parse and fill in `arfw`
>> }
>>
>> struct ath10k {
>> // ...
>> struct ath10k_fw fw_normal;
>> struct ath10k_fw fw_utf;
>> // ...
>> };
>>
>>
>> Michał
>
> Hmmm, this way we will have a unified firmware parsing logic. Is this a task
> which can be taken up easily or any other hidden complexities are invloved
> ?.
Decoupling the parsing logic should be rather easy. I don't think
there are any gotchas.
> I mean can we do the changes for current parsing logic and then rework the
> test mode patch ? what is your suggestion ?
If you want to do the unified parsing logic approach you should first
decouple the logic (i.e. make it not fill `struct ath10k` directly)
and then rework the testmode patch on top of that.
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
