All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Johnson <jjohnson@codeaurora.org>
To: Tom Psyborg <pozega.tomislav@gmail.com>
Cc: Kalle Valo <kvalo@codeaurora.org>,
	linux-wireless@vger.kernel.org, ath10k@lists.infradead.org,
	linux-wireless-owner@vger.kernel.org
Subject: Re: [PATCH 5/5] ath10k: pull_svc_rdy code-style fix
Date: Fri, 04 Oct 2019 11:56:51 -0700	[thread overview]
Message-ID: <998c7ce60b99865835f619dee86b301b@codeaurora.org> (raw)
In-Reply-To: <CAKR_QVK=XwLtaGgoLeU5-+XQP_-jVvAdWfkGvdyV9WNK_5QUng@mail.gmail.com>

On 2019-09-24 00:49, Tom Psyborg wrote:
> On 24/09/2019, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Tomislav Požega <pozega.tomislav@gmail.com> writes:
>> Actually I prefer the original style, so that we first check the data 
>> in
>> skb is valid and only then assign the data to ev.
>> 
>> --
>> Kalle Valo
>> 
> 
> It came to my mind that this might be the reason why the current
> driver did not give me warning about too short service ready event,
> but there was no warning about event length in either case.
> I even tested this with compat wireless from 2013. and there the
> situation was the opposite: in both cases there was warning about
> service ready length.

Hmmm, my understanding of the way the TLV WMI is supposed to work is 
that the individual data structures are extensible, and in the case 
where a data structure is received with a "short" length the recipient 
is supposed to zero-extend to the expected length, and then handle the 
"zeroed" field(s) appropriately. This is supposed to hold for both 
host=>firmware and firmware=>host. Since the wmi_service_ready_event has 
been extended over time this behavior is necessary in the case of a host 
built with the current format interfacing to a firmware built with an 
earlier version of the format. I'm not sure why ath10k isn't supporting 
this since the QTI "out of tree" driver (my area of focus) has that 
support.

WARNING: multiple messages have this Message-ID (diff)
From: Jeff Johnson <jjohnson@codeaurora.org>
To: Tom Psyborg <pozega.tomislav@gmail.com>
Cc: linux-wireless-owner@vger.kernel.org,
	linux-wireless@vger.kernel.org, ath10k@lists.infradead.org,
	Kalle Valo <kvalo@codeaurora.org>
Subject: Re: [PATCH 5/5] ath10k: pull_svc_rdy code-style fix
Date: Fri, 04 Oct 2019 11:56:51 -0700	[thread overview]
Message-ID: <998c7ce60b99865835f619dee86b301b@codeaurora.org> (raw)
In-Reply-To: <CAKR_QVK=XwLtaGgoLeU5-+XQP_-jVvAdWfkGvdyV9WNK_5QUng@mail.gmail.com>

On 2019-09-24 00:49, Tom Psyborg wrote:
> On 24/09/2019, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Tomislav Požega <pozega.tomislav@gmail.com> writes:
>> Actually I prefer the original style, so that we first check the data 
>> in
>> skb is valid and only then assign the data to ev.
>> 
>> --
>> Kalle Valo
>> 
> 
> It came to my mind that this might be the reason why the current
> driver did not give me warning about too short service ready event,
> but there was no warning about event length in either case.
> I even tested this with compat wireless from 2013. and there the
> situation was the opposite: in both cases there was warning about
> service ready length.

Hmmm, my understanding of the way the TLV WMI is supposed to work is 
that the individual data structures are extensible, and in the case 
where a data structure is received with a "short" length the recipient 
is supposed to zero-extend to the expected length, and then handle the 
"zeroed" field(s) appropriately. This is supposed to hold for both 
host=>firmware and firmware=>host. Since the wmi_service_ready_event has 
been extended over time this behavior is necessary in the case of a host 
built with the current format interfacing to a firmware built with an 
earlier version of the format. I'm not sure why ath10k isn't supporting 
this since the QTI "out of tree" driver (my area of focus) has that 
support.

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

  reply	other threads:[~2019-10-04 18:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-23 19:49 [PATCH 1/5] ath10k: add 2ghz channel arguments to service ready structure Tomislav Požega
2019-09-23 19:49 ` Tomislav Požega
2019-09-23 19:49 ` [PATCH 2/5] ath10k: print service ready returned channel range Tomislav Požega
2019-09-23 19:49   ` Tomislav Požega
2019-09-23 19:49 ` [PATCH 3/5] ath10k: print supported MCS rates within service ready event Tomislav Požega
2019-09-23 19:49   ` Tomislav Požega
2019-09-23 19:49 ` [PATCH 4/5] ath10k: change sw version print format to hex Tomislav Požega
2019-09-23 19:49   ` Tomislav Požega
2019-09-23 19:49 ` [PATCH 5/5] ath10k: pull_svc_rdy code-style fix Tomislav Požega
2019-09-23 19:49   ` Tomislav Požega
2019-09-24  5:30   ` Kalle Valo
2019-09-24  5:30     ` Kalle Valo
2019-09-24  7:49     ` Tom Psyborg
2019-09-24  7:49       ` Tom Psyborg
2019-10-04 18:56       ` Jeff Johnson [this message]
2019-10-04 18:56         ` Jeff Johnson
2019-10-01 11:15 ` [PATCH 1/5] ath10k: add 2ghz channel arguments to service ready structure Kalle Valo
2019-10-01 11:15 ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=998c7ce60b99865835f619dee86b301b@codeaurora.org \
    --to=jjohnson@codeaurora.org \
    --cc=ath10k@lists.infradead.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless-owner@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=pozega.tomislav@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.