On 8 August 2013 11:05, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
> Michal Kazior <michal.kazior@xxxxxxxxx> writes:
>
>> New firmware comes with new HTT protocol version.
>> In 3.0 the separate mgmt tx command has been
>> removed. All traffic is to be pushed through data
>> tx (tx_frm) command with a twist - FW seems to not
>> be able (yet?) to access tx fragment table so for
>> manamgement frames frame pointer is passed
>> directly.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@xxxxxxxxx>
>
> [...]
>
>> static int ath10k_htt_verify_version(struct ath10k_htt *htt)
>> {
>> - ath10k_dbg(ATH10K_DBG_HTT,
>> - "htt target version %d.%d; host version %d.%d\n",
>> - htt->target_version_major,
>> - htt->target_version_minor,
>> - HTT_CURRENT_VERSION_MAJOR,
>> - HTT_CURRENT_VERSION_MINOR);
>> -
>> - if (htt->target_version_major != HTT_CURRENT_VERSION_MAJOR) {
>> + ath10k_dbg(ATH10K_DBG_HTT, "htt target version %d.%d\n",
>> + htt->target_version_major, htt->target_version_minor);
>
> This debug print is good to have, but with the new htt version it would
> be good to print it always using the info level. For example, can we add
> it to the same line with "firmware %s booted" string?
HTT target version is not known when firmware boots up. It's not known
until everything other (HTC, WMI) is set up. We then send a version
request command and we get a response.
We need to print it in a separate line.
> (Please take into account that the info messages still need to be
> compact, by default they should not be longer than five lines or so.)
>
>> + if (htt->target_version_major != 2 &&
>> + htt->target_version_major != 3) {
>> ath10k_err("htt major versions are incompatible!\n");
>> return -ENOTSUPP;
>> }
>
> Print the htt version in the error message as well?
Target version is printed in ath10k_dbg() now. If we change that to
ath10k_info() I don't see any reason to print the version again on
error. We may want to print the list of supported HTT major version
numbers though?
>> @@ -401,10 +401,15 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
>> goto err;
>> }
>>
>> - txfrag = dev_alloc_skb(frag_len);
>> - if (!txfrag) {
>> - res = -ENOMEM;
>> - goto err;
>> + /* Since HTT 3.0 there is no separate mgmt tx command. However in case
>> + * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
>> + * fragment list host driver specifies directly frame pointer. */
>> + if (!(htt->target_version_major >= 3 && ieee80211_is_mgmt(hdr->frame_control))) {
>
> I think using "< 3" is more readable:
>
> if (htt->target_version_major < 3 &&
> !ieee80211_is_mgmt(hdr->frame_control)) {
> ...
> }
I don't have a problem with changing that.
> And is the single line too long? Didn't count it, though.
Ah, I didn't check that. Sorry. Good catch.
Pozdrawiam / Best regards,
Michał Kazior.
--
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
