linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kalle Valo <kvalo@qca.qualcomm.com>
To: <rmani@qca.qualcomm.com>
Cc: <linux-wireless@vger.kernel.org>, <ath6kl-devel@qualcomm.com>
Subject: Re: [PATCH] ath6kl: Check wow state before sending control and data pkt
Date: Mon, 6 Feb 2012 16:30:37 +0200	[thread overview]
Message-ID: <4F2FE40D.9020307@qca.qualcomm.com> (raw)
In-Reply-To: <1328536613-17521-1-git-send-email-rmani@qca.qualcomm.com>

On 02/06/2012 03:56 PM, rmani@qca.qualcomm.com wrote:
> From: Raja Mani <rmani@qca.qualcomm.com>
> 
> * TX operation (ctrl tx and data tx) has to be controlled based on
>   WOW suspend state. i.e, control packets are allowed to send from
>   the host until the suspend state goes ATH6KL_STATE_WOW and
>   the data packets are allowed until WOW suspend operation starts.
> 
> * Similary, wow resume is NOT allowed if WOW suspend is in progress.
> 
> Both of the above scenarios are taken care in this patch.
> 
> Signed-off-by: Raja Mani <rmani@qca.qualcomm.com>

What user visible bug are you fixing here?

> --- a/drivers/net/wireless/ath/ath6kl/txrx.c
> +++ b/drivers/net/wireless/ath/ath6kl/txrx.c
> @@ -284,6 +284,10 @@ int ath6kl_control_tx(void *devt, struct sk_buff *skb,
>  	int status = 0;
>  	struct ath6kl_cookie *cookie = NULL;
>  
> +#ifdef CONFIG_PM
> +	if (ar->wow_state == ATH6KL_WOW_STATE_SUSPENDED)
> +		return -EACCES;
> +#endif
>  	spin_lock_bh(&ar->lock);
>  
>  	ath6kl_dbg(ATH6KL_DBG_WLAN_TX,
> @@ -352,7 +356,12 @@ int ath6kl_data_tx(struct sk_buff *skb, struct net_device *dev)
>  	ath6kl_dbg(ATH6KL_DBG_WLAN_TX,
>  		   "%s: skb=0x%p, data=0x%p, len=0x%x\n", __func__,
>  		   skb, skb->data, skb->len);
> -
> +#ifdef CONFIG_PM
> +	if (ar->wow_state != ATH6KL_WOW_STATE_NONE) {
> +		dev_kfree_skb(skb);
> +		return 0;
> +	}
> +#endif

I think we should be more proactive than reactive. For example, we
should stop netdev queues to make sure that there are no data packets
transmitted. And we should do the same for ath6kl_control_tx() callers,
I guess that means cfg80211 ops?

But we could add warnings here to make sure that we are not transmitting
any packets in a wrong state.

Kalle

      parent reply	other threads:[~2012-02-06 14:30 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-06 13:56 [PATCH] ath6kl: Check wow state before sending control and data pkt rmani
2012-02-06 14:14 ` Kalle Valo
2012-02-07 10:14   ` Raja Mani
2012-02-07 13:33     ` Kalle Valo
2012-02-08  9:40       ` Raja Mani
2012-02-06 14:30 ` Kalle Valo [this message]

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=4F2FE40D.9020307@qca.qualcomm.com \
    --to=kvalo@qca.qualcomm.com \
    --cc=ath6kl-devel@qualcomm.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=rmani@qca.qualcomm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).