All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Xinming Hu <huxinming820@gmail.com>
Cc: Linux Wireless <linux-wireless@vger.kernel.org>,
	Kalle Valo <kvalo@qca.qualcomm.com>,
	Dmitry Torokhov <dtor@google.com>,
	Amitkumar Karwar <akarwar@marvell.com>,
	Cathy Luo <cluo@marvell.com>, Shengzhen Li <szli@marvell.com>
Subject: Re: [PATCH 01/12] mwifiex: fix power save issue when suspend
Date: Mon, 31 Oct 2016 16:57:12 -0700	[thread overview]
Message-ID: <20161031235712.GA101079@google.com> (raw)
In-Reply-To: <1477900940-10549-1-git-send-email-huxinming820@marvell.com>

Hi,

On Mon, Oct 31, 2016 at 04:02:09PM +0800, Xinming Hu wrote:
> From: Shengzhen Li <szli@marvell.com>
> 
> This patch fixes a corner case for "FROMLIST: mwifiex: fix corner case

Upstream patches don't normally get a 'FROMLIST' tag while you're
sending them to the mailing list :) That designation is used for Chrome
OS trees, so we know where to find the patch source. But the upstream
mailing list *is* the source.

> power save issue", main process will check the power save condition in
> PS_PRE_SLEEP status so the sleep handshake could continue.
> 
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> Signed-off-by: Shengzhen Li <szli@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> 
> BUG=chrome-os-partner:58164
> TEST=stress Wifi w/ power_save enabled
> 
> Change-Id: I5a36d9eaeb7fe5faaccc533e0d1ba1f3253666dc

In the same vein: while I'm happy to have the BUG=, TEST=, and Gerrit
Change-ID for our Chrome OS tree, I don't think upstream reviewers
typically care about those. Please remove those from upstream
submissions like this.

(Same applies to the rest of this series. Also, consider running
scripts/checkpatch.pl. It will at least remind you about removing the
Gerrit Change-ID.)

> ---
>  drivers/net/wireless/marvell/mwifiex/cmdevt.c | 3 ++-
>  drivers/net/wireless/marvell/mwifiex/main.c   | 5 +++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> index 5347728..9075be5 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> @@ -1123,8 +1123,9 @@ mwifiex_check_ps_cond(struct mwifiex_adapter *adapter)
>  		mwifiex_dnld_sleep_confirm_cmd(adapter);
>  	else
>  		mwifiex_dbg(adapter, CMD,
> -			    "cmd: Delay Sleep Confirm (%s%s%s)\n",
> +			    "cmd: Delay Sleep Confirm (%s%s%s%s)\n",
>  			    (adapter->cmd_sent) ? "D" : "",
> +			    (adapter->data_sent) ? "T" : "",
>  			    (adapter->curr_cmd) ? "C" : "",
>  			    (IS_CARD_RX_RCVD(adapter)) ? "R" : "");
>  }
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
> index 2478ccd..f559ead 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> @@ -308,6 +308,11 @@ process_start:
>  			/* We have tried to wakeup the card already */
>  			if (adapter->pm_wakeup_fw_try)
>  				break;
> +			if (adapter->ps_state == PS_STATE_PRE_SLEEP) {
> +				if (!adapter->cmd_sent && !adapter->curr_cmd)

The entire 'if' condition is unnecessary. That's all checked already
within mwifiex_check_ps_cond().

Brian

> +					mwifiex_check_ps_cond(adapter);
> +			}
> +
>  			if (adapter->ps_state != PS_STATE_AWAKE)
>  				break;
>  			if (adapter->tx_lock_flag) {
> -- 
> 1.8.1.4
> 

  parent reply	other threads:[~2016-10-31 23:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-31  8:02 [PATCH 01/12] mwifiex: fix power save issue when suspend Xinming Hu
2016-10-31  8:02 ` [PATCH 02/12] mwifiex: check tx_hw_pending before downloading sleep confirm Xinming Hu
2016-11-01  0:14   ` Brian Norris
2016-11-01 10:24     ` Xinming Hu
2016-10-31  8:02 ` [PATCH 03/12] mwifiex: don't do unbalanced free()'ing in cleanup_if() Xinming Hu
2016-10-31  8:02 ` [PATCH 04/12] mwifiex: resolve races between async FW init (failure) and device removal Xinming Hu
2016-10-31  8:02 ` [PATCH 05/12] mwifiex: remove redundant pdev check in suspend/resume handlers Xinming Hu
2016-10-31  8:02 ` [PATCH 06/12] mwifiex: don't pretend to resume while remove()'ing Xinming Hu
2016-10-31  8:02 ` [PATCH 07/12] mwifiex: resolve suspend() race with async FW init failure Xinming Hu
2016-10-31  8:02 ` [PATCH 08/12] mwifiex: reset card->adapter during device unregister Xinming Hu
2016-10-31  8:02 ` [PATCH 09/12] mwifiex: usb: handle HS failures Xinming Hu
2016-10-31  8:02 ` [PATCH 10/12] mwifiex: sdio: don't check for NULL sdio_func Xinming Hu
2016-10-31  8:02 ` [PATCH 11/12] mwifiex: stop checking for NULL drvata/intfdata Xinming Hu
2016-10-31  8:02 ` [PATCH 12/12] mwifiex: pcie: stop checking for NULL adapter->card Xinming Hu
2016-10-31 23:57 ` Brian Norris [this message]
2016-11-01 10:26   ` [PATCH 01/12] mwifiex: fix power save issue when suspend Xinming Hu

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=20161031235712.GA101079@google.com \
    --to=briannorris@chromium.org \
    --cc=akarwar@marvell.com \
    --cc=cluo@marvell.com \
    --cc=dtor@google.com \
    --cc=huxinming820@gmail.com \
    --cc=kvalo@qca.qualcomm.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=szli@marvell.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.