From: Rajkumar Manoharan <rmanohar@codeaurora.org>
To: Erik Stromdahl <erik.stromdahl@gmail.com>
Cc: linux-wireless-owner@vger.kernel.org, netdev@vger.kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
ath10k@lists.infradead.org, Kalle Valo <kvalo@qca.qualcomm.com>,
Niklas Cassel <niklas.cassel@linaro.org>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH v2] ath10k: transmit queued frames after waking queues
Date: Wed, 23 May 2018 11:05:32 -0700 [thread overview]
Message-ID: <ddcd48cc8b1a77c489214a4f63c54b1a@codeaurora.org> (raw)
In-Reply-To: <c131da6e-6479-3a40-fbd3-9c61d6690ba8@gmail.com>
On 2018-05-23 09:25, Erik Stromdahl wrote:
> On 05/22/2018 11:15 PM, Niklas Cassel wrote:
>
[...]
>>
>> Perhaps it would be possible to call ath10k_mac_tx_push_pending()
>> from the equivalent to ath10k_htt_txrx_compl_task(),
>> but from SDIO's point of view.
> An equivalent for SDIO would most likely be
> *ath10k_htt_htc_t2h_msg_handler*
> or any of the other functions called from this function.
>
> *ath10k_txrx_tx_unref* is actually called from
> *ath10k_htt_htc_t2h_msg_handler*,
> so that function could be viewed as an equivalent.
>
> If the call should be added in the bus driver (sdio.c) it should most
> likely be
> placed in *ath10k_sdio_mbox_rx_process_packets*
>
> if (!pkt->trailer_only) {
> ep->ep_ops.ep_rx_complete(ar_sdio->ar, pkt->skb);
> ath10k_mac_tx_push_pending(ar_sdio->ar);
> } else {
> kfree_skb(pkt->skb)
> }
>
> The above call would of course result in lot's of calls to
> *ath10k_mac_tx_push_pending*
> Adding a htt_num_pending check here wouldn't look nice.
>
> The HL RX path differs from the LL path in that the t2h_msg_handler
> returns
> false indicating that it has consumed the skb.
>
> This is because it is the HL RX indication handler that delivers the
> skb's
> to mac80211.
>
I also dont prefer to call *_push_pending for every HTC packets. Similar
to
LL approach, call ath10k_mac_tx_push_pending after processing all
pending
rx messages like calling from ath10k_sdio_mbox_rxmsg_pending_handler.
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -807,6 +807,8 @@ static int
ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar,
ath10k_warn(ar, "failed to get pending recv messages:
%d\n",
ret);
+ ath10k_mac_tx_push_pending(ar);
+
return ret;
}
> Another solution could be to add an *else-statement* as a part of the
> *if (release)*
> in *ath10k_htt_htc_t2h_msg_handler*, where
> *ath10k_mac_tx_push_pending* could be called.
>
> Something like this perhaps:
>
> /* Free the indication buffer */
> if (release)
> dev_kfree_skb_any(skb);
> else if (!ar->htt.num_pending_tx)
> ath10k_mac_tx_push_pending(ar);
>
> I think I prefer your original patch though.
>>
Better to do changes as HL specific path instead in common path.
The above change will impact QCA6174 based devices.
-Rajkumar
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
WARNING: multiple messages have this Message-ID (diff)
From: Rajkumar Manoharan <rmanohar@codeaurora.org>
To: Erik Stromdahl <erik.stromdahl@gmail.com>
Cc: Niklas Cassel <niklas.cassel@linaro.org>,
Kalle Valo <kvalo@qca.qualcomm.com>,
"David S. Miller" <davem@davemloft.net>,
ath10k@lists.infradead.org, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-wireless-owner@vger.kernel.org
Subject: Re: [PATCH v2] ath10k: transmit queued frames after waking queues
Date: Wed, 23 May 2018 11:05:32 -0700 [thread overview]
Message-ID: <ddcd48cc8b1a77c489214a4f63c54b1a@codeaurora.org> (raw)
In-Reply-To: <c131da6e-6479-3a40-fbd3-9c61d6690ba8@gmail.com>
On 2018-05-23 09:25, Erik Stromdahl wrote:
> On 05/22/2018 11:15 PM, Niklas Cassel wrote:
>
[...]
>>
>> Perhaps it would be possible to call ath10k_mac_tx_push_pending()
>> from the equivalent to ath10k_htt_txrx_compl_task(),
>> but from SDIO's point of view.
> An equivalent for SDIO would most likely be
> *ath10k_htt_htc_t2h_msg_handler*
> or any of the other functions called from this function.
>
> *ath10k_txrx_tx_unref* is actually called from
> *ath10k_htt_htc_t2h_msg_handler*,
> so that function could be viewed as an equivalent.
>
> If the call should be added in the bus driver (sdio.c) it should most
> likely be
> placed in *ath10k_sdio_mbox_rx_process_packets*
>
> if (!pkt->trailer_only) {
> ep->ep_ops.ep_rx_complete(ar_sdio->ar, pkt->skb);
> ath10k_mac_tx_push_pending(ar_sdio->ar);
> } else {
> kfree_skb(pkt->skb)
> }
>
> The above call would of course result in lot's of calls to
> *ath10k_mac_tx_push_pending*
> Adding a htt_num_pending check here wouldn't look nice.
>
> The HL RX path differs from the LL path in that the t2h_msg_handler
> returns
> false indicating that it has consumed the skb.
>
> This is because it is the HL RX indication handler that delivers the
> skb's
> to mac80211.
>
I also dont prefer to call *_push_pending for every HTC packets. Similar
to
LL approach, call ath10k_mac_tx_push_pending after processing all
pending
rx messages like calling from ath10k_sdio_mbox_rxmsg_pending_handler.
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -807,6 +807,8 @@ static int
ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar,
ath10k_warn(ar, "failed to get pending recv messages:
%d\n",
ret);
+ ath10k_mac_tx_push_pending(ar);
+
return ret;
}
> Another solution could be to add an *else-statement* as a part of the
> *if (release)*
> in *ath10k_htt_htc_t2h_msg_handler*, where
> *ath10k_mac_tx_push_pending* could be called.
>
> Something like this perhaps:
>
> /* Free the indication buffer */
> if (release)
> dev_kfree_skb_any(skb);
> else if (!ar->htt.num_pending_tx)
> ath10k_mac_tx_push_pending(ar);
>
> I think I prefer your original patch though.
>>
Better to do changes as HL specific path instead in common path.
The above change will impact QCA6174 based devices.
-Rajkumar
next prev parent reply other threads:[~2018-05-23 18:05 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-21 20:43 [PATCH v2] ath10k: transmit queued frames after waking queues Niklas Cassel
2018-05-21 20:43 ` Niklas Cassel
2018-05-21 23:11 ` Rajkumar Manoharan
2018-05-21 23:11 ` Rajkumar Manoharan
2018-05-22 21:15 ` Niklas Cassel
2018-05-22 21:15 ` Niklas Cassel
2018-05-22 22:16 ` Rajkumar Manoharan
2018-05-22 22:16 ` Rajkumar Manoharan
2018-05-23 16:25 ` Erik Stromdahl
2018-05-23 16:25 ` Erik Stromdahl
2018-05-23 16:25 ` Erik Stromdahl
2018-05-23 18:05 ` Rajkumar Manoharan [this message]
2018-05-23 18:05 ` Rajkumar Manoharan
2018-05-23 22:44 ` Niklas Cassel
2018-05-23 22:44 ` Niklas Cassel
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=ddcd48cc8b1a77c489214a4f63c54b1a@codeaurora.org \
--to=rmanohar@codeaurora.org \
--cc=ath10k@lists.infradead.org \
--cc=davem@davemloft.net \
--cc=erik.stromdahl@gmail.com \
--cc=kvalo@qca.qualcomm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless-owner@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=niklas.cassel@linaro.org \
/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.