On Wednesday, June 1, 2016 10:52:06 AM CEST Binoy Jayan wrote:
> 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.
>
> Signed-off-by: Binoy Jayan <binoy.jayan@xxxxxxxxxx>
> ---
> This patch depends on the following patch:
> rtl8712: intf_priv: Replace semaphore lock with completion
>
> drivers/staging/rtl8712/rtl8712_cmd.c | 10 +++++-----
> drivers/staging/rtl8712/rtl871x_pwrctrl.c | 22 +++++++++++-----------
> drivers/staging/rtl8712/rtl871x_pwrctrl.h | 2 +-
> 3 files changed, 17 insertions(+), 17 deletions(-)
Looks good, but you are slightly changing the behavior, and that should
be mentioned in the changelog:
> diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c b/drivers/staging/rtl8712/rtl8712_cmd.c
> index d07abc7..e893d65 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);
> }
_enter_pwrlock was using down_interruptible(), so the lock could be broken by
sending a signal. This was almost certainly a bug, because nothing checks
the return code here, and the function just continues as if it had taken
the lock (and worse, it ends up releasing it even if it doesn't have it).
Your patch fixes this bug.
As a minor detail, I think you should remove the now unused _enter_pwrlock()
in the same patch.
Arnd
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
