Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200205070858.15386-1-yhchuang@realtek.com> <20200205070858.15386-5-yhchuang@realtek.com>
In-Reply-To: <20200205070858.15386-5-yhchuang@realtek.com>
From:   Chris Chiu <chiu@endlessm.com>
Date:   Wed, 12 Feb 2020 12:16:19 +0800
Message-ID: <CAB4CAweOc=BWhE3eUaVh3t4QZeAMF7nPxJWzv5j0tFd2nVphZA@mail.gmail.com>
Subject: Re: [PATCH 4/7] rtw88: move rtw_enter_ips() to the last when config
To:     Tony Chuang <yhchuang@realtek.com>
Cc:     Kalle Valo <kvalo@codeaurora.org>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        Brian Norris <briannorris@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Wed, Feb 5, 2020 at 3:09 PM <yhchuang@realtek.com> wrote:
>
> From: Ping-Ke Shih <pkshih@realtek.com>
>
> When driver is coming up, mac80211 will set changed as ~0 (0xffffffff),
> and driver could enter IDLE state (power off) before switching channel or
> other config event. So move rtw_enter_ips() to the last, to make sure the
> driver completed the config events before going to IDLE state.
>
> So, moves leave/enter IPS config to be first/last one according to flag
> IEEE80211_CONF_IDLE. If there're more configureations we want to add in
> the future, they must locate between leave/enter IPS.
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> ---
Reviewed-by: Chris Chiu <chiu@endlessm.com>
>  drivers/net/wireless/realtek/rtw88/mac80211.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
> index 6fc33e11d08c..8742b3f2b5c1 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac80211.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
> @@ -75,15 +75,12 @@ static int rtw_ops_config(struct ieee80211_hw *hw, u32 changed)
>
>         rtw_leave_lps_deep(rtwdev);
>
> -       if (changed & IEEE80211_CONF_CHANGE_IDLE) {
> -               if (hw->conf.flags & IEEE80211_CONF_IDLE) {
> -                       rtw_enter_ips(rtwdev);
> -               } else {
> -                       ret = rtw_leave_ips(rtwdev);
> -                       if (ret) {
> -                               rtw_err(rtwdev, "failed to leave idle state\n");
> -                               goto out;
> -                       }
> +       if ((changed & IEEE80211_CONF_CHANGE_IDLE) &&
> +           !(hw->conf.flags & IEEE80211_CONF_IDLE)) {
> +               ret = rtw_leave_ips(rtwdev);
> +               if (ret) {
> +                       rtw_err(rtwdev, "failed to leave idle state\n");
> +                       goto out;
>                 }
>         }
>
> @@ -99,6 +96,10 @@ static int rtw_ops_config(struct ieee80211_hw *hw, u32 changed)
>         if (changed & IEEE80211_CONF_CHANGE_CHANNEL)
>                 rtw_set_channel(rtwdev);
>
> +       if ((changed & IEEE80211_CONF_CHANGE_IDLE) &&
> +           (hw->conf.flags & IEEE80211_CONF_IDLE))
> +               rtw_enter_ips(rtwdev);
> +
>  out:
>         mutex_unlock(&rtwdev->mutex);
>         return ret;
> --
> 2.17.1
>