From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-wm0-f67.google.com ([74.125.82.67]:33965 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753990AbcKORbS (ORCPT ); Tue, 15 Nov 2016 12:31:18 -0500 Received: by mail-wm0-f67.google.com with SMTP id g23so2220212wme.1 for ; Tue, 15 Nov 2016 09:31:18 -0800 (PST) Subject: Re: [RFC 02/12] ath10k: htc: rx trailer lookahead support To: Michal Kazior References: <1479141222-8493-1-git-send-email-erik.stromdahl@gmail.com> <1479141222-8493-3-git-send-email-erik.stromdahl@gmail.com> Cc: Kalle Valo , linux-wireless , "ath10k@lists.infradead.org" From: Erik Stromdahl Message-ID: (sfid-20161115_183123_062153_8F93B996) Date: Tue, 15 Nov 2016 18:31:15 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: On 11/15/2016 10:57 AM, Michal Kazior wrote: > On 14 November 2016 at 17:33, Erik Stromdahl wrote: >> The RX trailer parsing is now capable of parsing lookahead reports. >> This is needed by SDIO/mbox. > > It'd be useful to know what exactly lookahead will be used for. What > payload does it carry. > It carries the 4 first bytes of the next RX message, i.e. the first 4 bytes of an HTC header. It is used by the sdio interrupt handler to know if the next packet is a part of an RX bundle, which endpoint it belongs to and how long it is (so we know how many bytes to allocate). > >> Signed-off-by: Erik Stromdahl >> --- >> drivers/net/wireless/ath/ath10k/htc.c | 91 ++++++++++++++++++++++++++++++++- >> drivers/net/wireless/ath/ath10k/htc.h | 30 +++++++++-- >> 2 files changed, 116 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c >> index 26b1e15..e3f7bf4 100644 >> --- a/drivers/net/wireless/ath/ath10k/htc.c >> +++ b/drivers/net/wireless/ath/ath10k/htc.c >> @@ -228,10 +228,74 @@ void ath10k_htc_tx_completion_handler(struct ath10k *ar, struct sk_buff *skb) >> spin_unlock_bh(&htc->tx_lock); >> } >> >> +static int >> +ath10k_htc_process_lookahead(struct ath10k_htc *htc, >> + const struct ath10k_htc_lookahead_report *report, >> + int len, >> + enum ath10k_htc_ep_id eid, >> + u32 *next_lk_ahds, > > next_lk_ahds should be u8 or void. From what I understand by glancing > through the code it is an agnostic buffer that carries payload which > is later interpreted either as eid or htc header, right? void is > probably most suitable in this case for passing around and u8 for > inline-based storage. > Sounds reasonable, I'll change to void pointer. > I'd also avoid silly abbreviations. Probably "lookahead" alone is enough. > Ok, this abbreviation was actually taken from the ath6kl code. I think the intention was to reduce line lengths. >> + int *next_lk_ahds_len) >> +{ >> + struct ath10k *ar = htc->ar; >> + >> + if (report->pre_valid != ((~report->post_valid) & 0xFF)) >> + /* Invalid lookahead flags are actually transmitted by >> + * the target in the HTC control message. >> + * Since this will happen at every boot we silently ignore >> + * the lookahead in this case >> + */ > > I'd put this comment before the if(). Ok > > >> + return 0; >> + >> + if (next_lk_ahds && next_lk_ahds_len) { >> + ath10k_dbg(ar, ATH10K_DBG_HTC, >> + "htc rx lk_ahd found pre_valid 0x%x post_valid 0x%x\n", >> + report->pre_valid, report->post_valid); >> + >> + /* look ahead bytes are valid, copy them over */ >> + memcpy((u8 *)&next_lk_ahds[0], report->lk_ahd, 4); >> + >> + *next_lk_ahds_len = 1; >> + } >> + >> + return 0; >> +} >> + >> +static int >> +ath10k_htc_process_lookahead_bundle(struct ath10k_htc *htc, >> + const struct ath10k_htc_lookahead_report_bundle *report, >> + int len, >> + enum ath10k_htc_ep_id eid, >> + u32 *next_lk_ahds, > > Ditto. void. > > >> + int *next_lk_ahds_len) >> +{ >> + int bundle_cnt = len / sizeof(*report); >> + >> + if (!bundle_cnt || (bundle_cnt > HTC_HOST_MAX_MSG_PER_BUNDLE)) { >> + WARN_ON(1); > > This should be ath10k_warn() instead. This isn't internal driver flow > assertion. It is possible firmware bug or revision misalignment > instead. > Ok > >> + return -EINVAL; >> + } >> + >> + if (next_lk_ahds && next_lk_ahds_len) { >> + int i; >> + >> + for (i = 0; i < bundle_cnt; i++) { >> + memcpy((u8 *)&next_lk_ahds[i], report->lk_ahd, >> + sizeof(u32)); > > You'll need to re-adjust the &x[i] to maintain proper offset with void pointer. > > > Michał > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-wm0-x243.google.com ([2a00:1450:400c:c09::243]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1c6ha7-0001aR-OO for ath10k@lists.infradead.org; Tue, 15 Nov 2016 17:31:40 +0000 Received: by mail-wm0-x243.google.com with SMTP id a20so2208352wme.2 for ; Tue, 15 Nov 2016 09:31:18 -0800 (PST) Subject: Re: [RFC 02/12] ath10k: htc: rx trailer lookahead support References: <1479141222-8493-1-git-send-email-erik.stromdahl@gmail.com> <1479141222-8493-3-git-send-email-erik.stromdahl@gmail.com> From: Erik Stromdahl Message-ID: Date: Tue, 15 Nov 2016 18:31:15 +0100 MIME-Version: 1.0 In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: Michal Kazior Cc: Kalle Valo , linux-wireless , "ath10k@lists.infradead.org" CgpPbiAxMS8xNS8yMDE2IDEwOjU3IEFNLCBNaWNoYWwgS2F6aW9yIHdyb3RlOgo+IE9uIDE0IE5v dmVtYmVyIDIwMTYgYXQgMTc6MzMsIEVyaWsgU3Ryb21kYWhsIDxlcmlrLnN0cm9tZGFobEBnbWFp bC5jb20+IHdyb3RlOgo+PiBUaGUgUlggdHJhaWxlciBwYXJzaW5nIGlzIG5vdyBjYXBhYmxlIG9m IHBhcnNpbmcgbG9va2FoZWFkIHJlcG9ydHMuCj4+IFRoaXMgaXMgbmVlZGVkIGJ5IFNESU8vbWJv eC4KPiAKPiBJdCdkIGJlIHVzZWZ1bCB0byBrbm93IHdoYXQgZXhhY3RseSBsb29rYWhlYWQgd2ls bCBiZSB1c2VkIGZvci4gV2hhdAo+IHBheWxvYWQgZG9lcyBpdCBjYXJyeS4KPiAKSXQgY2Fycmll cyB0aGUgNCBmaXJzdCBieXRlcyBvZiB0aGUgbmV4dCBSWCBtZXNzYWdlLCBpLmUuIHRoZSBmaXJz dCA0CmJ5dGVzIG9mIGFuIEhUQyBoZWFkZXIuCgpJdCBpcyB1c2VkIGJ5IHRoZSBzZGlvIGludGVy cnVwdCBoYW5kbGVyIHRvIGtub3cgaWYgdGhlIG5leHQgcGFja2V0IGlzIGEKcGFydCBvZiBhbiBS WCBidW5kbGUsIHdoaWNoIGVuZHBvaW50IGl0IGJlbG9uZ3MgdG8gYW5kIGhvdyBsb25nIGl0IGlz CihzbyB3ZSBrbm93IGhvdyBtYW55IGJ5dGVzIHRvIGFsbG9jYXRlKS4KCj4gCj4+IFNpZ25lZC1v ZmYtYnk6IEVyaWsgU3Ryb21kYWhsIDxlcmlrLnN0cm9tZGFobEBnbWFpbC5jb20+Cj4+IC0tLQo+ PiAgZHJpdmVycy9uZXQvd2lyZWxlc3MvYXRoL2F0aDEway9odGMuYyB8ICAgOTEgKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrKystCj4+ICBkcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRo MTBrL2h0Yy5oIHwgICAzMCArKysrKysrKystLQo+PiAgMiBmaWxlcyBjaGFuZ2VkLCAxMTYgaW5z ZXJ0aW9ucygrKSwgNSBkZWxldGlvbnMoLSkKPj4KPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0 L3dpcmVsZXNzL2F0aC9hdGgxMGsvaHRjLmMgYi9kcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRo MTBrL2h0Yy5jCj4+IGluZGV4IDI2YjFlMTUuLmUzZjdiZjQgMTAwNjQ0Cj4+IC0tLSBhL2RyaXZl cnMvbmV0L3dpcmVsZXNzL2F0aC9hdGgxMGsvaHRjLmMKPj4gKysrIGIvZHJpdmVycy9uZXQvd2ly ZWxlc3MvYXRoL2F0aDEway9odGMuYwo+PiBAQCAtMjI4LDEwICsyMjgsNzQgQEAgdm9pZCBhdGgx MGtfaHRjX3R4X2NvbXBsZXRpb25faGFuZGxlcihzdHJ1Y3QgYXRoMTBrICphciwgc3RydWN0IHNr X2J1ZmYgKnNrYikKPj4gICAgICAgICBzcGluX3VubG9ja19iaCgmaHRjLT50eF9sb2NrKTsKPj4g IH0KPj4KPj4gK3N0YXRpYyBpbnQKPj4gK2F0aDEwa19odGNfcHJvY2Vzc19sb29rYWhlYWQoc3Ry dWN0IGF0aDEwa19odGMgKmh0YywKPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICBjb25z dCBzdHJ1Y3QgYXRoMTBrX2h0Y19sb29rYWhlYWRfcmVwb3J0ICpyZXBvcnQsCj4+ICsgICAgICAg ICAgICAgICAgICAgICAgICAgICAgaW50IGxlbiwKPj4gKyAgICAgICAgICAgICAgICAgICAgICAg ICAgICBlbnVtIGF0aDEwa19odGNfZXBfaWQgZWlkLAo+PiArICAgICAgICAgICAgICAgICAgICAg ICAgICAgIHUzMiAqbmV4dF9sa19haGRzLAo+IAo+IG5leHRfbGtfYWhkcyBzaG91bGQgYmUgdTgg b3Igdm9pZC4gRnJvbSB3aGF0IEkgdW5kZXJzdGFuZCBieSBnbGFuY2luZwo+IHRocm91Z2ggdGhl IGNvZGUgaXQgaXMgYW4gYWdub3N0aWMgYnVmZmVyIHRoYXQgY2FycmllcyBwYXlsb2FkIHdoaWNo Cj4gaXMgbGF0ZXIgaW50ZXJwcmV0ZWQgZWl0aGVyIGFzIGVpZCBvciBodGMgaGVhZGVyLCByaWdo dD8gdm9pZCBpcwo+IHByb2JhYmx5IG1vc3Qgc3VpdGFibGUgaW4gdGhpcyBjYXNlIGZvciBwYXNz aW5nIGFyb3VuZCBhbmQgdTggZm9yCj4gaW5saW5lLWJhc2VkIHN0b3JhZ2UuCj4gClNvdW5kcyBy ZWFzb25hYmxlLCBJJ2xsIGNoYW5nZSB0byB2b2lkIHBvaW50ZXIuCgo+IEknZCBhbHNvIGF2b2lk IHNpbGx5IGFiYnJldmlhdGlvbnMuIFByb2JhYmx5ICJsb29rYWhlYWQiIGFsb25lIGlzIGVub3Vn aC4KPiAKT2ssIHRoaXMgYWJicmV2aWF0aW9uIHdhcyBhY3R1YWxseSB0YWtlbiBmcm9tIHRoZSBh dGg2a2wgY29kZS4gSSB0aGluawp0aGUgaW50ZW50aW9uIHdhcyB0byByZWR1Y2UgbGluZSBsZW5n dGhzLgoKPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICBpbnQgKm5leHRfbGtfYWhkc19s ZW4pCj4+ICt7Cj4+ICsgICAgICAgc3RydWN0IGF0aDEwayAqYXIgPSBodGMtPmFyOwo+PiArCj4+ ICsgICAgICAgaWYgKHJlcG9ydC0+cHJlX3ZhbGlkICE9ICgofnJlcG9ydC0+cG9zdF92YWxpZCkg JiAweEZGKSkKPj4gKyAgICAgICAgICAgICAgIC8qIEludmFsaWQgbG9va2FoZWFkIGZsYWdzIGFy ZSBhY3R1YWxseSB0cmFuc21pdHRlZCBieQo+PiArICAgICAgICAgICAgICAgICogdGhlIHRhcmdl dCBpbiB0aGUgSFRDIGNvbnRyb2wgbWVzc2FnZS4KPj4gKyAgICAgICAgICAgICAgICAqIFNpbmNl IHRoaXMgd2lsbCBoYXBwZW4gYXQgZXZlcnkgYm9vdCB3ZSBzaWxlbnRseSBpZ25vcmUKPj4gKyAg ICAgICAgICAgICAgICAqIHRoZSBsb29rYWhlYWQgaW4gdGhpcyBjYXNlCj4+ICsgICAgICAgICAg ICAgICAgKi8KPiAKPiBJJ2QgcHV0IHRoaXMgY29tbWVudCBiZWZvcmUgdGhlIGlmKCkuCgpPawo+ IAo+IAo+PiArICAgICAgICAgICAgICAgcmV0dXJuIDA7Cj4+ICsKPj4gKyAgICAgICBpZiAobmV4 dF9sa19haGRzICYmIG5leHRfbGtfYWhkc19sZW4pIHsKPj4gKyAgICAgICAgICAgICAgIGF0aDEw a19kYmcoYXIsIEFUSDEwS19EQkdfSFRDLAo+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAi aHRjIHJ4IGxrX2FoZCBmb3VuZCBwcmVfdmFsaWQgMHgleCBwb3N0X3ZhbGlkIDB4JXhcbiIsCj4+ ICsgICAgICAgICAgICAgICAgICAgICAgICAgIHJlcG9ydC0+cHJlX3ZhbGlkLCByZXBvcnQtPnBv c3RfdmFsaWQpOwo+PiArCj4+ICsgICAgICAgICAgICAgICAvKiBsb29rIGFoZWFkIGJ5dGVzIGFy ZSB2YWxpZCwgY29weSB0aGVtIG92ZXIgKi8KPj4gKyAgICAgICAgICAgICAgIG1lbWNweSgodTgg KikmbmV4dF9sa19haGRzWzBdLCByZXBvcnQtPmxrX2FoZCwgNCk7Cj4+ICsKPj4gKyAgICAgICAg ICAgICAgICpuZXh0X2xrX2FoZHNfbGVuID0gMTsKPj4gKyAgICAgICB9Cj4+ICsKPj4gKyAgICAg ICByZXR1cm4gMDsKPj4gK30KPj4gKwo+PiArc3RhdGljIGludAo+PiArYXRoMTBrX2h0Y19wcm9j ZXNzX2xvb2thaGVhZF9idW5kbGUoc3RydWN0IGF0aDEwa19odGMgKmh0YywKPj4gKyAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgY29uc3Qgc3RydWN0IGF0aDEwa19odGNfbG9va2Fo ZWFkX3JlcG9ydF9idW5kbGUgKnJlcG9ydCwKPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgaW50IGxlbiwKPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ZW51bSBhdGgxMGtfaHRjX2VwX2lkIGVpZCwKPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgdTMyICpuZXh0X2xrX2FoZHMsCj4gCj4gRGl0dG8uIHZvaWQuCj4gCj4gCj4+ICsg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGludCAqbmV4dF9sa19haGRzX2xlbikK Pj4gK3sKPj4gKyAgICAgICBpbnQgYnVuZGxlX2NudCA9IGxlbiAvIHNpemVvZigqcmVwb3J0KTsK Pj4gKwo+PiArICAgICAgIGlmICghYnVuZGxlX2NudCB8fCAoYnVuZGxlX2NudCA+IEhUQ19IT1NU X01BWF9NU0dfUEVSX0JVTkRMRSkpIHsKPj4gKyAgICAgICAgICAgICAgIFdBUk5fT04oMSk7Cj4g Cj4gVGhpcyBzaG91bGQgYmUgYXRoMTBrX3dhcm4oKSBpbnN0ZWFkLiBUaGlzIGlzbid0IGludGVy bmFsIGRyaXZlciBmbG93Cj4gYXNzZXJ0aW9uLiBJdCBpcyBwb3NzaWJsZSBmaXJtd2FyZSBidWcg b3IgcmV2aXNpb24gbWlzYWxpZ25tZW50Cj4gaW5zdGVhZC4KPiAKT2sKCj4gCj4+ICsgICAgICAg ICAgICAgICByZXR1cm4gLUVJTlZBTDsKPj4gKyAgICAgICB9Cj4+ICsKPj4gKyAgICAgICBpZiAo bmV4dF9sa19haGRzICYmIG5leHRfbGtfYWhkc19sZW4pIHsKPj4gKyAgICAgICAgICAgICAgIGlu dCBpOwo+PiArCj4+ICsgICAgICAgICAgICAgICBmb3IgKGkgPSAwOyBpIDwgYnVuZGxlX2NudDsg aSsrKSB7Cj4+ICsgICAgICAgICAgICAgICAgICAgICAgIG1lbWNweSgodTggKikmbmV4dF9sa19h aGRzW2ldLCByZXBvcnQtPmxrX2FoZCwKPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg IHNpemVvZih1MzIpKTsKPiAKPiBZb3UnbGwgbmVlZCB0byByZS1hZGp1c3QgdGhlICZ4W2ldIHRv IG1haW50YWluIHByb3BlciBvZmZzZXQgd2l0aCB2b2lkIHBvaW50ZXIuCj4gCj4gCj4gTWljaGHF ggo+IAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYXRo MTBrIG1haWxpbmcgbGlzdAphdGgxMGtAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlzdHMu aW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2F0aDEwawo=