From mboxrd@z Thu Jan  1 00:00:00 1970
Return-path: <linux-wireless-owner@vger.kernel.org>
Received: from mail-wm0-f65.google.com ([74.125.82.65]:35322 "EHLO
        mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750998AbcKORHM (ORCPT
        <rfc822;linux-wireless@vger.kernel.org>);
        Tue, 15 Nov 2016 12:07:12 -0500
Received: by mail-wm0-f65.google.com with SMTP id a20so1970858wme.2
        for <linux-wireless@vger.kernel.org>; Tue, 15 Nov 2016 09:07:12 -0800 (PST)
Subject: Re: [RFC 03/12] ath10k: htc: Changed order of wait target and ep
 connect
To: Michal Kazior <michal.kazior@tieto.com>
References: <1479141222-8493-1-git-send-email-erik.stromdahl@gmail.com>
 <1479141222-8493-4-git-send-email-erik.stromdahl@gmail.com>
 <CA+BoTQmA0E+N46E3XkpqVPXSVkQNyZj2Qzrizk8iLdYzVthNQg@mail.gmail.com>
Cc: Kalle Valo <kvalo@qca.qualcomm.com>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        "ath10k@lists.infradead.org" <ath10k@lists.infradead.org>
From: Erik Stromdahl <erik.stromdahl@gmail.com>
Message-ID: <33589cf2-a7e6-fe9a-ea2b-a2252ac00355@gmail.com> (sfid-20161115_180716_935445_3D251661)
Date: Tue, 15 Nov 2016 18:07:09 +0100
MIME-Version: 1.0
In-Reply-To: <CA+BoTQmA0E+N46E3XkpqVPXSVkQNyZj2Qzrizk8iLdYzVthNQg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Sender: linux-wireless-owner@vger.kernel.org
List-ID: <linux-wireless.vger.kernel.org>



On 11/15/2016 11:13 AM, Michal Kazior wrote:
> On 14 November 2016 at 17:33, Erik Stromdahl <erik.stromdahl@gmail.com> wrote:
>> This patch changes the order in which the driver waits for the
>> target to become ready and the service connect of the HTC
>> control service.
>>
>> The HTC control service is connected before the driver starts
>> waiting for the HTC ready control message.
>>
>> The reason for this is that the HTC ready control message is
>> transmitted on EP 0 and that sdio/mbox based systems will ignore
>> messages received on unconnected endpoints.
>>
>> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/htc.c |   32 ++++++++++++++++----------------
>>  1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
>> index e3f7bf4..7257366 100644
>> --- a/drivers/net/wireless/ath/ath10k/htc.c
>> +++ b/drivers/net/wireless/ath/ath10k/htc.c
>> @@ -606,6 +606,22 @@ int ath10k_htc_wait_target(struct ath10k_htc *htc)
>>         u16 credit_count;
>>         u16 credit_size;
>>
>> +       /* setup our pseudo HTC control endpoint connection */
>> +       memset(&conn_req, 0, sizeof(conn_req));
>> +       memset(&conn_resp, 0, sizeof(conn_resp));
>> +       conn_req.ep_ops.ep_tx_complete = ath10k_htc_control_tx_complete;
>> +       conn_req.ep_ops.ep_rx_complete = ath10k_htc_control_rx_complete;
>> +       conn_req.max_send_queue_depth = ATH10K_NUM_CONTROL_TX_BUFFERS;
>> +       conn_req.service_id = ATH10K_HTC_SVC_ID_RSVD_CTRL;
>> +
>> +       /* connect fake service */
>> +       status = ath10k_htc_connect_service(htc, &conn_req, &conn_resp);
>> +       if (status) {
>> +               ath10k_err(ar, "could not connect to htc service (%d)\n",
>> +                          status);
>> +               return status;
>> +       }
>> +
> 
> How is this supposed to work? ath10k_htc_connect_service() requires
> htc->target_credit_size to compute tx_credits_per_max_message. Or am I
> missing something? Applying this patch alone results in:
> 
> [    6.680101] divide error: 0000 [#1] SMP
> [    6.681342] Modules linked in: ath10k_pci(O) ath10k_core(O) ath
> mac80211 cfg80211
> [    6.684876] CPU: 3 PID: 823 Comm: kworker/u8:2 Tainted: G        W
> O    4.9.0-rc4-wt-ath+ #79
> [    6.688051] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
> [    6.691644] Workqueue: ath10k_wq ath10k_core_register_work [ath10k_core]
> [    6.694309] task: ffff88000a190000 task.stack: ffffc900006d4000
> [    6.695458] RIP: 0010:[<ffffffffa01ae46b>]  [<ffffffffa01ae46b>]
> ath10k_htc_connect_service+0x21b/0x420 [ath10k_core]
> 
> 
> Michał
> 

You're right. I have totally missed this. What is strange is that my
compiler (ARM linaro) seems to optimize the code in a way that removes
the tx_credits_per_max_message value.

If I add a printk in ath10k_htc_connect_service (printing the value) I
get a similar oops.

I think it has to do with the fact the this value isn't really used at
all. grepping the code reveals that tx_credits_per_max_message is only
used inside ath10k_htc_connect_service (only written, never read).

Removing it doesn't seem to break anything, so perhaps it should be removed?

Or is there something I have missed?

/Erik

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-path: <ath10k-bounces+kvalo=adurom.com@lists.infradead.org>
Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244])
 by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux))
 id 1c6hCn-0008Dv-BW
 for ath10k@lists.infradead.org; Tue, 15 Nov 2016 17:07:34 +0000
Received: by mail-wm0-x244.google.com with SMTP id g23so1983477wme.1
 for <ath10k@lists.infradead.org>; Tue, 15 Nov 2016 09:07:12 -0800 (PST)
Subject: Re: [RFC 03/12] ath10k: htc: Changed order of wait target and ep
 connect
References: <1479141222-8493-1-git-send-email-erik.stromdahl@gmail.com>
 <1479141222-8493-4-git-send-email-erik.stromdahl@gmail.com>
 <CA+BoTQmA0E+N46E3XkpqVPXSVkQNyZj2Qzrizk8iLdYzVthNQg@mail.gmail.com>
From: Erik Stromdahl <erik.stromdahl@gmail.com>
Message-ID: <33589cf2-a7e6-fe9a-ea2b-a2252ac00355@gmail.com>
Date: Tue, 15 Nov 2016 18:07:09 +0100
MIME-Version: 1.0
In-Reply-To: <CA+BoTQmA0E+N46E3XkpqVPXSVkQNyZj2Qzrizk8iLdYzVthNQg@mail.gmail.com>
List-Id: <ath10k.lists.infradead.org>
List-Unsubscribe: <http://lists.infradead.org/mailman/options/ath10k>,
 <mailto:ath10k-request@lists.infradead.org?subject=unsubscribe>
List-Archive: <http://lists.infradead.org/pipermail/ath10k/>
List-Post: <mailto:ath10k@lists.infradead.org>
List-Help: <mailto:ath10k-request@lists.infradead.org?subject=help>
List-Subscribe: <http://lists.infradead.org/mailman/listinfo/ath10k>,
 <mailto:ath10k-request@lists.infradead.org?subject=subscribe>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Sender: "ath10k" <ath10k-bounces@lists.infradead.org>
Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org
To: Michal Kazior <michal.kazior@tieto.com>
Cc: Kalle Valo <kvalo@qca.qualcomm.com>, linux-wireless <linux-wireless@vger.kernel.org>, "ath10k@lists.infradead.org" <ath10k@lists.infradead.org>

CgpPbiAxMS8xNS8yMDE2IDExOjEzIEFNLCBNaWNoYWwgS2F6aW9yIHdyb3RlOgo+IE9uIDE0IE5v
dmVtYmVyIDIwMTYgYXQgMTc6MzMsIEVyaWsgU3Ryb21kYWhsIDxlcmlrLnN0cm9tZGFobEBnbWFp
bC5jb20+IHdyb3RlOgo+PiBUaGlzIHBhdGNoIGNoYW5nZXMgdGhlIG9yZGVyIGluIHdoaWNoIHRo
ZSBkcml2ZXIgd2FpdHMgZm9yIHRoZQo+PiB0YXJnZXQgdG8gYmVjb21lIHJlYWR5IGFuZCB0aGUg
c2VydmljZSBjb25uZWN0IG9mIHRoZSBIVEMKPj4gY29udHJvbCBzZXJ2aWNlLgo+Pgo+PiBUaGUg
SFRDIGNvbnRyb2wgc2VydmljZSBpcyBjb25uZWN0ZWQgYmVmb3JlIHRoZSBkcml2ZXIgc3RhcnRz
Cj4+IHdhaXRpbmcgZm9yIHRoZSBIVEMgcmVhZHkgY29udHJvbCBtZXNzYWdlLgo+Pgo+PiBUaGUg
cmVhc29uIGZvciB0aGlzIGlzIHRoYXQgdGhlIEhUQyByZWFkeSBjb250cm9sIG1lc3NhZ2UgaXMK
Pj4gdHJhbnNtaXR0ZWQgb24gRVAgMCBhbmQgdGhhdCBzZGlvL21ib3ggYmFzZWQgc3lzdGVtcyB3
aWxsIGlnbm9yZQo+PiBtZXNzYWdlcyByZWNlaXZlZCBvbiB1bmNvbm5lY3RlZCBlbmRwb2ludHMu
Cj4+Cj4+IFNpZ25lZC1vZmYtYnk6IEVyaWsgU3Ryb21kYWhsIDxlcmlrLnN0cm9tZGFobEBnbWFp
bC5jb20+Cj4+IC0tLQo+PiAgZHJpdmVycy9uZXQvd2lyZWxlc3MvYXRoL2F0aDEway9odGMuYyB8
ICAgMzIgKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0tLS0KPj4gIDEgZmlsZSBjaGFuZ2Vk
LCAxNiBpbnNlcnRpb25zKCspLCAxNiBkZWxldGlvbnMoLSkKPj4KPj4gZGlmZiAtLWdpdCBhL2Ry
aXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGgxMGsvaHRjLmMgYi9kcml2ZXJzL25ldC93aXJlbGVz
cy9hdGgvYXRoMTBrL2h0Yy5jCj4+IGluZGV4IGUzZjdiZjQuLjcyNTczNjYgMTAwNjQ0Cj4+IC0t
LSBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGgxMGsvaHRjLmMKPj4gKysrIGIvZHJpdmVy
cy9uZXQvd2lyZWxlc3MvYXRoL2F0aDEway9odGMuYwo+PiBAQCAtNjA2LDYgKzYwNiwyMiBAQCBp
bnQgYXRoMTBrX2h0Y193YWl0X3RhcmdldChzdHJ1Y3QgYXRoMTBrX2h0YyAqaHRjKQo+PiAgICAg
ICAgIHUxNiBjcmVkaXRfY291bnQ7Cj4+ICAgICAgICAgdTE2IGNyZWRpdF9zaXplOwo+Pgo+PiAr
ICAgICAgIC8qIHNldHVwIG91ciBwc2V1ZG8gSFRDIGNvbnRyb2wgZW5kcG9pbnQgY29ubmVjdGlv
biAqLwo+PiArICAgICAgIG1lbXNldCgmY29ubl9yZXEsIDAsIHNpemVvZihjb25uX3JlcSkpOwo+
PiArICAgICAgIG1lbXNldCgmY29ubl9yZXNwLCAwLCBzaXplb2YoY29ubl9yZXNwKSk7Cj4+ICsg
ICAgICAgY29ubl9yZXEuZXBfb3BzLmVwX3R4X2NvbXBsZXRlID0gYXRoMTBrX2h0Y19jb250cm9s
X3R4X2NvbXBsZXRlOwo+PiArICAgICAgIGNvbm5fcmVxLmVwX29wcy5lcF9yeF9jb21wbGV0ZSA9
IGF0aDEwa19odGNfY29udHJvbF9yeF9jb21wbGV0ZTsKPj4gKyAgICAgICBjb25uX3JlcS5tYXhf
c2VuZF9xdWV1ZV9kZXB0aCA9IEFUSDEwS19OVU1fQ09OVFJPTF9UWF9CVUZGRVJTOwo+PiArICAg
ICAgIGNvbm5fcmVxLnNlcnZpY2VfaWQgPSBBVEgxMEtfSFRDX1NWQ19JRF9SU1ZEX0NUUkw7Cj4+
ICsKPj4gKyAgICAgICAvKiBjb25uZWN0IGZha2Ugc2VydmljZSAqLwo+PiArICAgICAgIHN0YXR1
cyA9IGF0aDEwa19odGNfY29ubmVjdF9zZXJ2aWNlKGh0YywgJmNvbm5fcmVxLCAmY29ubl9yZXNw
KTsKPj4gKyAgICAgICBpZiAoc3RhdHVzKSB7Cj4+ICsgICAgICAgICAgICAgICBhdGgxMGtfZXJy
KGFyLCAiY291bGQgbm90IGNvbm5lY3QgdG8gaHRjIHNlcnZpY2UgKCVkKVxuIiwKPj4gKyAgICAg
ICAgICAgICAgICAgICAgICAgICAgc3RhdHVzKTsKPj4gKyAgICAgICAgICAgICAgIHJldHVybiBz
dGF0dXM7Cj4+ICsgICAgICAgfQo+PiArCj4gCj4gSG93IGlzIHRoaXMgc3VwcG9zZWQgdG8gd29y
az8gYXRoMTBrX2h0Y19jb25uZWN0X3NlcnZpY2UoKSByZXF1aXJlcwo+IGh0Yy0+dGFyZ2V0X2Ny
ZWRpdF9zaXplIHRvIGNvbXB1dGUgdHhfY3JlZGl0c19wZXJfbWF4X21lc3NhZ2UuIE9yIGFtIEkK
PiBtaXNzaW5nIHNvbWV0aGluZz8gQXBwbHlpbmcgdGhpcyBwYXRjaCBhbG9uZSByZXN1bHRzIGlu
Ogo+IAo+IFsgICAgNi42ODAxMDFdIGRpdmlkZSBlcnJvcjogMDAwMCBbIzFdIFNNUAo+IFsgICAg
Ni42ODEzNDJdIE1vZHVsZXMgbGlua2VkIGluOiBhdGgxMGtfcGNpKE8pIGF0aDEwa19jb3JlKE8p
IGF0aAo+IG1hYzgwMjExIGNmZzgwMjExCj4gWyAgICA2LjY4NDg3Nl0gQ1BVOiAzIFBJRDogODIz
IENvbW06IGt3b3JrZXIvdTg6MiBUYWludGVkOiBHICAgICAgICBXCj4gTyAgICA0LjkuMC1yYzQt
d3QtYXRoKyAjNzkKPiBbICAgIDYuNjg4MDUxXSBIYXJkd2FyZSBuYW1lOiBRRU1VIFN0YW5kYXJk
IFBDIChpNDQwRlggKyBQSUlYLCAxOTk2KSwKPiBCSU9TIDEuNy41LTIwMTQwNTMxXzA4MzAzMC1n
YW5kYWxmIDA0LzAxLzIwMTQKPiBbICAgIDYuNjkxNjQ0XSBXb3JrcXVldWU6IGF0aDEwa193cSBh
dGgxMGtfY29yZV9yZWdpc3Rlcl93b3JrIFthdGgxMGtfY29yZV0KPiBbICAgIDYuNjk0MzA5XSB0
YXNrOiBmZmZmODgwMDBhMTkwMDAwIHRhc2suc3RhY2s6IGZmZmZjOTAwMDA2ZDQwMDAKPiBbICAg
IDYuNjk1NDU4XSBSSVA6IDAwMTA6WzxmZmZmZmZmZmEwMWFlNDZiPl0gIFs8ZmZmZmZmZmZhMDFh
ZTQ2Yj5dCj4gYXRoMTBrX2h0Y19jb25uZWN0X3NlcnZpY2UrMHgyMWIvMHg0MjAgW2F0aDEwa19j
b3JlXQo+IAo+IAo+IE1pY2hhxYIKPiAKCllvdSdyZSByaWdodC4gSSBoYXZlIHRvdGFsbHkgbWlz
c2VkIHRoaXMuIFdoYXQgaXMgc3RyYW5nZSBpcyB0aGF0IG15CmNvbXBpbGVyIChBUk0gbGluYXJv
KSBzZWVtcyB0byBvcHRpbWl6ZSB0aGUgY29kZSBpbiBhIHdheSB0aGF0IHJlbW92ZXMKdGhlIHR4
X2NyZWRpdHNfcGVyX21heF9tZXNzYWdlIHZhbHVlLgoKSWYgSSBhZGQgYSBwcmludGsgaW4gYXRo
MTBrX2h0Y19jb25uZWN0X3NlcnZpY2UgKHByaW50aW5nIHRoZSB2YWx1ZSkgSQpnZXQgYSBzaW1p
bGFyIG9vcHMuCgpJIHRoaW5rIGl0IGhhcyB0byBkbyB3aXRoIHRoZSBmYWN0IHRoZSB0aGlzIHZh
bHVlIGlzbid0IHJlYWxseSB1c2VkIGF0CmFsbC4gZ3JlcHBpbmcgdGhlIGNvZGUgcmV2ZWFscyB0
aGF0IHR4X2NyZWRpdHNfcGVyX21heF9tZXNzYWdlIGlzIG9ubHkKdXNlZCBpbnNpZGUgYXRoMTBr
X2h0Y19jb25uZWN0X3NlcnZpY2UgKG9ubHkgd3JpdHRlbiwgbmV2ZXIgcmVhZCkuCgpSZW1vdmlu
ZyBpdCBkb2Vzbid0IHNlZW0gdG8gYnJlYWsgYW55dGhpbmcsIHNvIHBlcmhhcHMgaXQgc2hvdWxk
IGJlIHJlbW92ZWQ/CgpPciBpcyB0aGVyZSBzb21ldGhpbmcgSSBoYXZlIG1pc3NlZD8KCi9Fcmlr
CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwphdGgxMGsg
bWFpbGluZyBsaXN0CmF0aDEwa0BsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZy
YWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vYXRoMTBrCg==