Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1411808 > unrolled thread
| Started by | Binoy Jayan <binoy.jayan@linaro.org> |
|---|---|
| First post | 2016-06-02 06:30 +0200 |
| Last post | 2016-06-02 17:20 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/4] *** rtl8712: Replace semaphores with mutex / completions *** Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-02 06:30 +0200
[PATCH v2 4/4] rtl8712: pwrctrl_priv: Replace semaphore lock with mutex Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-02 06:30 +0200
[PATCH v2 2/4] rtl8712: Replace semaphore terminate_cmdthread_sema with completion Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-02 06:30 +0200
Re: [PATCH v2 0/4] *** rtl8712: Replace semaphores with mutex / completions *** Arnd Bergmann <arnd@arndb.de> - 2016-06-02 09:50 +0200
Re: [PATCH v2 0/4] *** rtl8712: Replace semaphores with mutex / completions *** Larry Finger <Larry.Finger@lwfinger.net> - 2016-06-02 17:20 +0200
| From | Binoy Jayan <binoy.jayan@linaro.org> |
|---|---|
| Date | 2016-06-02 06:30 +0200 |
| Subject | [PATCH v2 0/4] *** rtl8712: Replace semaphores with mutex / completions *** |
| Message-ID | <rFsqB-1dm-5@gated-at.bofh.it> |
Hi, These are a set of patches [v2] which removes semaphores from: drivers/staging/rtl8712 They build correctly (individually and as a whole). NB: I have not tested this as I do not have the following hardware: "RealTek RTL8712U (RTL8192SU) Wireless LAN NIC driver" Rework on comments w.r.t. PATCH v1: - Removed wrapper functions _wait_completion, _down_sema and _enter_pwrlock - Updated changelog to explain use of mutex_lock instead of mutex_lock_interruptible in [PATCH v2 4/4] Binoy Binoy Jayan (4): rtl8712: Replace semaphore cmd_queue_sema with completion rtl8712: Replace semaphore terminate_cmdthread_sema with completion rtl8712: intf_priv: Replace semaphore lock with completion rtl8712: pwrctrl_priv: Replace semaphore lock with mutex drivers/staging/rtl8712/os_intfs.c | 4 ++-- drivers/staging/rtl8712/osdep_intf.h | 2 +- drivers/staging/rtl8712/osdep_service.h | 7 ------- drivers/staging/rtl8712/rtl8712_cmd.c | 14 +++++++------- drivers/staging/rtl8712/rtl871x_cmd.c | 8 ++++---- drivers/staging/rtl8712/rtl871x_cmd.h | 4 ++-- drivers/staging/rtl8712/rtl871x_pwrctrl.c | 24 ++++++++++++------------ drivers/staging/rtl8712/rtl871x_pwrctrl.h | 7 +------ drivers/staging/rtl8712/usb_ops_linux.c | 6 +++--- 9 files changed, 32 insertions(+), 44 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[toc] | [next] | [standalone]
| From | Binoy Jayan <binoy.jayan@linaro.org> |
|---|---|
| Date | 2016-06-02 06:30 +0200 |
| Subject | [PATCH v2 4/4] rtl8712: pwrctrl_priv: Replace semaphore lock with mutex |
| Message-ID | <rFsqC-1dm-21@gated-at.bofh.it> |
| In reply to | #1411808 |
The semaphore 'lock' in 'pwrctrl_priv' is used as a simple mutex, so it
should be written as one. Semaphores are going away in the future.
_enter_pwrlock was using down_interruptible(), so the lock could be broken
by sending a signal. This could be a bug, because nothing checks the return
code here. Hence, using mutex_lock instead of the interruptible version.
Removing the now unused _enter_pwrlock and _down_sema.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
This patch depends on the following patch:
rtl8712: intf_priv: Replace semaphore lock with completion
drivers/staging/rtl8712/osdep_service.h | 7 -------
drivers/staging/rtl8712/rtl8712_cmd.c | 10 +++++-----
drivers/staging/rtl8712/rtl871x_pwrctrl.c | 22 +++++++++++-----------
drivers/staging/rtl8712/rtl871x_pwrctrl.h | 7 +------
4 files changed, 17 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/rtl8712/osdep_service.h b/drivers/staging/rtl8712/osdep_service.h
index 076d508..d89ae09 100644
--- a/drivers/staging/rtl8712/osdep_service.h
+++ b/drivers/staging/rtl8712/osdep_service.h
@@ -60,13 +60,6 @@ struct __queue {
#define LIST_CONTAINOR(ptr, type, member) \
((type *)((char *)(ptr)-(SIZE_T)(&((type *)0)->member)))
-static inline u32 _down_sema(struct semaphore *sema)
-{
- if (down_interruptible(sema))
- return _FAIL;
- return _SUCCESS;
-}
-
static inline u32 end_of_queue_search(struct list_head *head,
struct list_head *plist)
{
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c b/drivers/staging/rtl8712/rtl8712_cmd.c
index 1badc6c..9934eab 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -264,9 +264,9 @@ static struct cmd_obj *cmd_hdl_filter(struct _adapter *padapter,
*/
if (padapter->pwrctrlpriv.pwr_mode > PS_MODE_ACTIVE) {
padapter->pwrctrlpriv.pwr_mode = PS_MODE_ACTIVE;
- _enter_pwrlock(&(padapter->pwrctrlpriv.lock));
+ mutex_lock(&padapter->pwrctrlpriv.mutex_lock);
r8712_set_rpwm(padapter, PS_STATE_S4);
- up(&(padapter->pwrctrlpriv.lock));
+ mutex_unlock(&padapter->pwrctrlpriv.mutex_lock);
}
pcmd_r = pcmd;
break;
@@ -395,10 +395,10 @@ _next:
}
if (pcmd->cmdcode == GEN_CMD_CODE(_SetPwrMode)) {
if (padapter->pwrctrlpriv.bSleep) {
- _enter_pwrlock(&(padapter->
- pwrctrlpriv.lock));
+ mutex_lock(&padapter->
+ pwrctrlpriv.mutex_lock);
r8712_set_rpwm(padapter, PS_STATE_S2);
- up(&padapter->pwrctrlpriv.lock);
+ mutex_unlock(&padapter->pwrctrlpriv.mutex_lock);
}
}
r8712_free_cmd_obj(pcmd);
diff --git a/drivers/staging/rtl8712/rtl871x_pwrctrl.c b/drivers/staging/rtl8712/rtl871x_pwrctrl.c
index 98a5e74..8d7ead6 100644
--- a/drivers/staging/rtl8712/rtl871x_pwrctrl.c
+++ b/drivers/staging/rtl8712/rtl871x_pwrctrl.c
@@ -103,14 +103,14 @@ void r8712_cpwm_int_hdl(struct _adapter *padapter,
if (pwrpriv->cpwm_tog == ((preportpwrstate->state) & 0x80))
return;
del_timer(&padapter->pwrctrlpriv.rpwm_check_timer);
- _enter_pwrlock(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->mutex_lock);
pwrpriv->cpwm = (preportpwrstate->state) & 0xf;
if (pwrpriv->cpwm >= PS_STATE_S2) {
if (pwrpriv->alives & CMD_ALIVE)
complete(&(pcmdpriv->cmd_queue_comp));
}
pwrpriv->cpwm_tog = (preportpwrstate->state) & 0x80;
- up(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->mutex_lock);
}
static inline void register_task_alive(struct pwrctrl_priv *pwrctrl, uint tag)
@@ -141,10 +141,10 @@ static void SetPSModeWorkItemCallback(struct work_struct *work)
struct _adapter *padapter = container_of(pwrpriv,
struct _adapter, pwrctrlpriv);
if (!pwrpriv->bSleep) {
- _enter_pwrlock(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->mutex_lock);
if (pwrpriv->pwr_mode == PS_MODE_ACTIVE)
r8712_set_rpwm(padapter, PS_STATE_S4);
- up(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->mutex_lock);
}
}
@@ -155,11 +155,11 @@ static void rpwm_workitem_callback(struct work_struct *work)
struct _adapter *padapter = container_of(pwrpriv,
struct _adapter, pwrctrlpriv);
if (pwrpriv->cpwm != pwrpriv->rpwm) {
- _enter_pwrlock(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->mutex_lock);
r8712_read8(padapter, SDIO_HCPWM);
pwrpriv->rpwm_retry = 1;
r8712_set_rpwm(padapter, pwrpriv->rpwm);
- up(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->mutex_lock);
}
}
@@ -175,7 +175,7 @@ void r8712_init_pwrctrl_priv(struct _adapter *padapter)
struct pwrctrl_priv *pwrctrlpriv = &padapter->pwrctrlpriv;
memset((unsigned char *)pwrctrlpriv, 0, sizeof(struct pwrctrl_priv));
- sema_init(&pwrctrlpriv->lock, 1);
+ mutex_init(&pwrctrlpriv->mutex_lock);
pwrctrlpriv->cpwm = PS_STATE_S4;
pwrctrlpriv->pwr_mode = PS_MODE_ACTIVE;
pwrctrlpriv->smart_ps = 0;
@@ -207,13 +207,13 @@ sint r8712_register_cmd_alive(struct _adapter *padapter)
uint res = _SUCCESS;
struct pwrctrl_priv *pwrctrl = &padapter->pwrctrlpriv;
- _enter_pwrlock(&pwrctrl->lock);
+ mutex_lock(&pwrctrl->mutex_lock);
register_task_alive(pwrctrl, CMD_ALIVE);
if (pwrctrl->cpwm < PS_STATE_S2) {
r8712_set_rpwm(padapter, PS_STATE_S3);
res = _FAIL;
}
- up(&pwrctrl->lock);
+ mutex_unlock(&pwrctrl->mutex_lock);
return res;
}
@@ -229,7 +229,7 @@ void r8712_unregister_cmd_alive(struct _adapter *padapter)
{
struct pwrctrl_priv *pwrctrl = &padapter->pwrctrlpriv;
- _enter_pwrlock(&pwrctrl->lock);
+ mutex_lock(&pwrctrl->mutex_lock);
unregister_task_alive(pwrctrl, CMD_ALIVE);
if ((pwrctrl->cpwm > PS_STATE_S2) &&
(pwrctrl->pwr_mode > PS_MODE_ACTIVE)) {
@@ -239,5 +239,5 @@ void r8712_unregister_cmd_alive(struct _adapter *padapter)
r8712_set_rpwm(padapter, PS_STATE_S0);
}
}
- up(&pwrctrl->lock);
+ mutex_unlock(&pwrctrl->mutex_lock);
}
diff --git a/drivers/staging/rtl8712/rtl871x_pwrctrl.h b/drivers/staging/rtl8712/rtl871x_pwrctrl.h
index dbfb555..231445e 100644
--- a/drivers/staging/rtl8712/rtl871x_pwrctrl.h
+++ b/drivers/staging/rtl8712/rtl871x_pwrctrl.h
@@ -87,13 +87,8 @@ struct reportpwrstate_parm {
unsigned short rsvd;
};
-static inline void _enter_pwrlock(struct semaphore *plock)
-{
- _down_sema(plock);
-}
-
struct pwrctrl_priv {
- struct semaphore lock;
+ struct mutex mutex_lock;
/*volatile*/ u8 rpwm; /* requested power state for fw */
/* fw current power state. updated when 1. read from HCPWM or
* 2. driver lowers power level */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[toc] | [prev] | [next] | [standalone]
| From | Binoy Jayan <binoy.jayan@linaro.org> |
|---|---|
| Date | 2016-06-02 06:30 +0200 |
| Subject | [PATCH v2 2/4] rtl8712: Replace semaphore terminate_cmdthread_sema with completion |
| Message-ID | <rFsqB-1dm-7@gated-at.bofh.it> |
| In reply to | #1411808 |
The semaphore 'terminate_cmdthread_sema' is used as completion,
so convert it to a struct completion type.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
This patch depends on the following patch:
rtl8712: Replace semaphore cmd_queue_sema with completion
drivers/staging/rtl8712/os_intfs.c | 2 +-
drivers/staging/rtl8712/rtl8712_cmd.c | 2 +-
drivers/staging/rtl8712/rtl871x_cmd.c | 2 +-
drivers/staging/rtl8712/rtl871x_cmd.h | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
index c07bcd0..2396bac 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -245,7 +245,7 @@ void r8712_stop_drv_threads(struct _adapter *padapter)
/*Below is to terminate r8712_cmd_thread & event_thread...*/
complete(&padapter->cmdpriv.cmd_queue_comp);
if (padapter->cmdThread)
- _down_sema(&padapter->cmdpriv.terminate_cmdthread_sema);
+ wait_for_completion_interruptible(&padapter->cmdpriv.terminate_cmdthread_comp);
padapter->cmdpriv.cmd_seq = 1;
}
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c b/drivers/staging/rtl8712/rtl8712_cmd.c
index 172f51f..1badc6c 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -420,7 +420,7 @@ _next:
break;
r8712_free_cmd_obj(pcmd);
} while (1);
- up(&pcmdpriv->terminate_cmdthread_sema);
+ complete(&pcmdpriv->terminate_cmdthread_comp);
thread_exit();
}
diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c b/drivers/staging/rtl8712/rtl871x_cmd.c
index 69e650b..74fd928 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.c
+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
@@ -58,7 +58,7 @@ No irqsave is necessary.
static sint _init_cmd_priv(struct cmd_priv *pcmdpriv)
{
init_completion(&pcmdpriv->cmd_queue_comp);
- sema_init(&(pcmdpriv->terminate_cmdthread_sema), 0);
+ init_completion(&pcmdpriv->terminate_cmdthread_comp);
_init_queue(&(pcmdpriv->cmd_queue));
diff --git a/drivers/staging/rtl8712/rtl871x_cmd.h b/drivers/staging/rtl8712/rtl871x_cmd.h
index 1907bc9..ebd2e1d 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.h
+++ b/drivers/staging/rtl8712/rtl871x_cmd.h
@@ -51,7 +51,7 @@ struct cmd_obj {
struct cmd_priv {
struct completion cmd_queue_comp;
- struct semaphore terminate_cmdthread_sema;
+ struct completion terminate_cmdthread_comp;
struct __queue cmd_queue;
u8 cmd_seq;
u8 *cmd_buf; /*shall be non-paged, and 4 bytes aligned*/
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-06-02 09:50 +0200 |
| Message-ID | <rFvya-32D-17@gated-at.bofh.it> |
| In reply to | #1411808 |
On Thursday, June 2, 2016 9:54:06 AM CEST Binoy Jayan wrote: > Hi, > > These are a set of patches [v2] which removes semaphores from: > > drivers/staging/rtl8712 > > They build correctly (individually and as a whole). > NB: I have not tested this as I do not have the following hardware: > > "RealTek RTL8712U (RTL8192SU) Wireless LAN NIC driver" > > Rework on comments w.r.t. PATCH v1: > > - Removed wrapper functions _wait_completion, _down_sema and _enter_pwrlock > - Updated changelog to explain use of mutex_lock instead of > mutex_lock_interruptible in [PATCH v2 4/4] > All four look great now, Reviewed-by: Arnd Bergmann <arnd@arndb.de>
[toc] | [prev] | [next] | [standalone]
| From | Larry Finger <Larry.Finger@lwfinger.net> |
|---|---|
| Date | 2016-06-02 17:20 +0200 |
| Subject | Re: [PATCH v2 0/4] *** rtl8712: Replace semaphores with mutex / completions *** |
| Message-ID | <rFCzD-7xa-21@gated-at.bofh.it> |
| In reply to | #1411952 |
On 06/02/2016 02:43 AM, Arnd Bergmann wrote: > On Thursday, June 2, 2016 9:54:06 AM CEST Binoy Jayan wrote: >> Hi, >> >> These are a set of patches [v2] which removes semaphores from: >> >> drivers/staging/rtl8712 >> >> They build correctly (individually and as a whole). >> NB: I have not tested this as I do not have the following hardware: >> >> "RealTek RTL8712U (RTL8192SU) Wireless LAN NIC driver" >> >> Rework on comments w.r.t. PATCH v1: >> >> - Removed wrapper functions _wait_completion, _down_sema and _enter_pwrlock >> - Updated changelog to explain use of mutex_lock instead of >> mutex_lock_interruptible in [PATCH v2 4/4] >> > > All four look great now, > > Reviewed-by: Arnd Bergmann <arnd@arndb.de> I agree. Tested-by: Larry Finger <Larry.Finger@lwfinger.net> Larry
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web