From: Tim Gardner <tim.gardner@canonical.com>
To: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: "John W. Linville" <linville@tuxdriver.com>,
linux-wireless@vger.kernel.org,
Larry Finger <Larry.Finger@lwfinger.net>,
Chaoming Li <chaoming_li@realsil.com.cn>,
Mike McCormack <mikem@ring3k.org>
Subject: Re: [PATCH 2/2] rtlwifi: merge ips,lps spinlocks into one mutex
Date: Mon, 12 Dec 2011 10:32:38 -0700 [thread overview]
Message-ID: <4EE63AB6.2010209@canonical.com> (raw)
In-Reply-To: <1323690204-5124-2-git-send-email-sgruszka@redhat.com>
On 12/12/2011 04:43 AM, Stanislaw Gruszka wrote:
> With previous patch "rtlwifi: use work for lps" we can now use mutex for
> protecting ps mode changing critical sections. This fixes running system
> with interrupts disabled for long time.
>
> Merge ips_lock and lps_lock as they seems to protect the same data
> structures (accessed in rtl_ps_set_rf_state() function).
>
> Reported-by: Philipp Dreimann<philipp@dreimann.net>
> Tested-by: Larry Finger<Larry.Finger@lwfinger.net>
> Cc: Mike McCormack<mikem@ring3k.org>
> Cc: Chaoming Li<chaoming_li@realsil.com.cn>
> Signed-off-by: Stanislaw Gruszka<sgruszka@redhat.com>
> ---
> drivers/net/wireless/rtlwifi/base.c | 3 +--
> drivers/net/wireless/rtlwifi/ps.c | 21 ++++++++++-----------
> drivers/net/wireless/rtlwifi/wifi.h | 3 +--
> 3 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
> index a13ecfc..d81a602 100644
> --- a/drivers/net/wireless/rtlwifi/base.c
> +++ b/drivers/net/wireless/rtlwifi/base.c
> @@ -448,12 +448,11 @@ int rtl_init_core(struct ieee80211_hw *hw)
>
> /*<4> locks */
> mutex_init(&rtlpriv->locks.conf_mutex);
> - spin_lock_init(&rtlpriv->locks.ips_lock);
> + mutex_init(&rtlpriv->locks.ps_mutex);
> spin_lock_init(&rtlpriv->locks.irq_th_lock);
> spin_lock_init(&rtlpriv->locks.h2c_lock);
> spin_lock_init(&rtlpriv->locks.rf_ps_lock);
> spin_lock_init(&rtlpriv->locks.rf_lock);
> - spin_lock_init(&rtlpriv->locks.lps_lock);
> spin_lock_init(&rtlpriv->locks.waitq_lock);
> spin_lock_init(&rtlpriv->locks.cck_and_rw_pagea_lock);
>
> diff --git a/drivers/net/wireless/rtlwifi/ps.c b/drivers/net/wireless/rtlwifi/ps.c
> index 55c8e50..a14a68b 100644
> --- a/drivers/net/wireless/rtlwifi/ps.c
> +++ b/drivers/net/wireless/rtlwifi/ps.c
> @@ -241,7 +241,7 @@ void rtl_ips_nic_on(struct ieee80211_hw *hw)
> if (mac->opmode != NL80211_IFTYPE_STATION)
> return;
>
> - spin_lock(&rtlpriv->locks.ips_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
>
> if (ppsc->inactiveps) {
> rtstate = ppsc->rfpwr_state;
> @@ -257,7 +257,7 @@ void rtl_ips_nic_on(struct ieee80211_hw *hw)
> }
> }
>
> - spin_unlock(&rtlpriv->locks.ips_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> /*for FW LPS*/
> @@ -395,7 +395,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw)
> if (mac->link_state != MAC80211_LINKED)
> return;
>
> - spin_lock_irq(&rtlpriv->locks.lps_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
>
> /* Idle for a while if we connect to AP a while ago. */
> if (mac->cnt_after_linked>= 2) {
> @@ -407,7 +407,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw)
> }
> }
>
> - spin_unlock_irq(&rtlpriv->locks.lps_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> /*Leave the leisure power save mode.*/
> @@ -416,9 +416,8 @@ void rtl_lps_leave(struct ieee80211_hw *hw)
> struct rtl_priv *rtlpriv = rtl_priv(hw);
> struct rtl_ps_ctl *ppsc = rtl_psc(rtl_priv(hw));
> struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
> - unsigned long flags;
>
> - spin_lock_irqsave(&rtlpriv->locks.lps_lock, flags);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
>
> if (ppsc->fwctrl_lps) {
> if (ppsc->dot11_psmode != EACTIVE) {
> @@ -439,7 +438,7 @@ void rtl_lps_leave(struct ieee80211_hw *hw)
> rtl_lps_set_psmode(hw, EACTIVE);
> }
> }
> - spin_unlock_irqrestore(&rtlpriv->locks.lps_lock, flags);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> /* For sw LPS*/
> @@ -540,9 +539,9 @@ void rtl_swlps_rf_awake(struct ieee80211_hw *hw)
> RT_CLEAR_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM);
> }
>
> - spin_lock_irq(&rtlpriv->locks.lps_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
> rtl_ps_set_rf_state(hw, ERFON, RF_CHANGE_BY_PS);
> - spin_unlock_irq(&rtlpriv->locks.lps_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> void rtl_swlps_rfon_wq_callback(void *data)
> @@ -575,9 +574,9 @@ void rtl_swlps_rf_sleep(struct ieee80211_hw *hw)
> if (rtlpriv->link_info.busytraffic)
> return;
>
> - spin_lock_irq(&rtlpriv->locks.lps_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
> rtl_ps_set_rf_state(hw, ERFSLEEP, RF_CHANGE_BY_PS);
> - spin_unlock_irq(&rtlpriv->locks.lps_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
>
> if (ppsc->reg_rfps_level& RT_RF_OFF_LEVL_ASPM&&
> !RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM)) {
> diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h
> index 6e6353b..085dccd 100644
> --- a/drivers/net/wireless/rtlwifi/wifi.h
> +++ b/drivers/net/wireless/rtlwifi/wifi.h
> @@ -1544,14 +1544,13 @@ struct rtl_hal_cfg {
> struct rtl_locks {
> /* mutex */
> struct mutex conf_mutex;
> + struct mutex ps_mutex;
>
> /*spin lock */
> - spinlock_t ips_lock;
> spinlock_t irq_th_lock;
> spinlock_t h2c_lock;
> spinlock_t rf_ps_lock;
> spinlock_t rf_lock;
> - spinlock_t lps_lock;
> spinlock_t waitq_lock;
>
> /*Dual mac*/
On an SMP platform spin_lock_irqsave() is only effective if the
interrupt handler uses the same spinlock. Its benign in this case, but
still bogus.
Tested-by: Tim Gardner <tim.gardner@canonical.com>
--
Tim Gardner tim.gardner@canonical.com
next prev parent reply other threads:[~2011-12-12 17:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-12 11:43 [PATCH 1/2] rtlwifi: use work for lps Stanislaw Gruszka
2011-12-12 11:43 ` [PATCH 2/2] rtlwifi: merge ips,lps spinlocks into one mutex Stanislaw Gruszka
2011-12-12 15:54 ` Larry Finger
2011-12-12 17:32 ` Tim Gardner [this message]
2011-12-12 15:54 ` [PATCH 1/2] rtlwifi: use work for lps Larry Finger
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=4EE63AB6.2010209@canonical.com \
--to=tim.gardner@canonical.com \
--cc=Larry.Finger@lwfinger.net \
--cc=chaoming_li@realsil.com.cn \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=mikem@ring3k.org \
--cc=sgruszka@redhat.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).