lkml.org 
[lkml]   [2018]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/3] staging: rtl8723bs: change semaphores to completions
From
Date
Hi Arnd,

Thank you for the patches.

On 10-12-18 22:40, Arnd Bergmann wrote:
> This driver uses many semaphores, most of them are equivalent to
> completions. The other copies of this driver got moved over to
> completions a while ago, so do the same here.
>
> In this usage scenario, the two are equivalent, so the behavior
> should not change.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

The entire series looks good to me and I've also given it
a quick test run on a tablet with a rtl8723bs wifi chip:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans




> ---
> drivers/staging/rtl8723bs/core/rtw_cmd.c | 20 ++++++++---------
> drivers/staging/rtl8723bs/core/rtw_mlme.c | 2 --
> drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 4 ++--
> drivers/staging/rtl8723bs/core/rtw_xmit.c | 8 +++----
> .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 4 ++--
> .../staging/rtl8723bs/hal/rtl8723bs_xmit.c | 22 ++++++++-----------
> drivers/staging/rtl8723bs/hal/sdio_ops.c | 2 +-
> drivers/staging/rtl8723bs/include/rtw_cmd.h | 7 +++---
> drivers/staging/rtl8723bs/include/rtw_xmit.h | 9 ++++----
> drivers/staging/rtl8723bs/os_dep/os_intfs.c | 6 ++---
> drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 2 +-
> 11 files changed, 39 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> index 830be63391b7..3d206c33eafa 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> @@ -166,10 +166,8 @@ sint _rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
> {
> sint res = _SUCCESS;
>
> - sema_init(&(pcmdpriv->cmd_queue_sema), 0);
> - /* sema_init(&(pcmdpriv->cmd_done_sema), 0); */
> - sema_init(&(pcmdpriv->terminate_cmdthread_sema), 0);
> -
> + init_completion(&pcmdpriv->cmd_queue_comp);
> + init_completion(&pcmdpriv->terminate_cmdthread_comp);
>
> _rtw_init_queue(&(pcmdpriv->cmd_queue));
>
> @@ -369,7 +367,7 @@ u32 rtw_enqueue_cmd(struct cmd_priv *pcmdpriv, struct cmd_obj *cmd_obj)
> res = _rtw_enqueue_cmd(&pcmdpriv->cmd_queue, cmd_obj);
>
> if (res == _SUCCESS)
> - up(&pcmdpriv->cmd_queue_sema);
> + complete(&pcmdpriv->cmd_queue_comp);
>
> exit:
> return res;
> @@ -410,8 +408,8 @@ void rtw_stop_cmd_thread(struct adapter *adapter)
> atomic_read(&(adapter->cmdpriv.cmdthd_running)) == true &&
> adapter->cmdpriv.stop_req == 0) {
> adapter->cmdpriv.stop_req = 1;
> - up(&adapter->cmdpriv.cmd_queue_sema);
> - down(&adapter->cmdpriv.terminate_cmdthread_sema);
> + complete(&adapter->cmdpriv.cmd_queue_comp);
> + wait_for_completion(&adapter->cmdpriv.terminate_cmdthread_comp);
> }
> }
>
> @@ -435,13 +433,13 @@ int rtw_cmd_thread(void *context)
>
> pcmdpriv->stop_req = 0;
> atomic_set(&(pcmdpriv->cmdthd_running), true);
> - up(&pcmdpriv->terminate_cmdthread_sema);
> + complete(&pcmdpriv->terminate_cmdthread_comp);
>
> RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, ("start r871x rtw_cmd_thread !!!!\n"));
>
> while (1) {
> - if (down_interruptible(&pcmdpriv->cmd_queue_sema)) {
> - DBG_871X_LEVEL(_drv_always_, FUNC_ADPT_FMT" down_interruptible(&pcmdpriv->cmd_queue_sema) return != 0, break\n", FUNC_ADPT_ARG(padapter));
> + if (wait_for_completion_interruptible(&pcmdpriv->cmd_queue_comp)) {
> + DBG_871X_LEVEL(_drv_always_, FUNC_ADPT_FMT" wait_for_completion_interruptible(&pcmdpriv->cmd_queue_comp) return != 0, break\n", FUNC_ADPT_ARG(padapter));
> break;
> }
>
> @@ -581,7 +579,7 @@ int rtw_cmd_thread(void *context)
> rtw_free_cmd_obj(pcmd);
> } while (1);
>
> - up(&pcmdpriv->terminate_cmdthread_sema);
> + complete(&pcmdpriv->terminate_cmdthread_comp);
> atomic_set(&(pcmdpriv->cmdthd_running), false);
>
> thread_exit();
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> index 4c5d5cf9dfe0..81505f3bce8b 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> @@ -2373,8 +2373,6 @@ sint rtw_set_key(struct adapter *adapter, struct security_priv *psecuritypriv, s
>
> INIT_LIST_HEAD(&pcmd->list);
>
> - /* sema_init(&(pcmd->cmd_sem), 0); */
> -
> res = rtw_enqueue_cmd(pcmdpriv, pcmd);
> } else{
> setkey_hdl(adapter, (u8 *)psetkeyparm);
> diff --git a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
> index 59a667753266..a0d8fbeecf9b 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
> @@ -745,10 +745,10 @@ void cpwm_int_hdl(
>
> if (pwrpriv->cpwm >= PS_STATE_S2) {
> if (pwrpriv->alives & CMD_ALIVE)
> - up(&padapter->cmdpriv.cmd_queue_sema);
> + complete(&padapter->cmdpriv.cmd_queue_comp);
>
> if (pwrpriv->alives & XMIT_ALIVE)
> - up(&padapter->xmitpriv.xmit_sema);
> + complete(&padapter->xmitpriv.xmit_comp);
> }
>
> up(&pwrpriv->lock);
> diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> index 729fd37f440b..fdfa4b3c4c10 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> @@ -45,8 +45,8 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
>
> spin_lock_init(&pxmitpriv->lock);
> spin_lock_init(&pxmitpriv->lock_sctx);
> - sema_init(&pxmitpriv->xmit_sema, 0);
> - sema_init(&pxmitpriv->terminate_xmitthread_sema, 0);
> + init_completion(&pxmitpriv->xmit_comp);
> + init_completion(&pxmitpriv->terminate_xmitthread_comp);
>
> /*
> Please insert all the queue initializaiton using _rtw_init_queue below
> @@ -2879,7 +2879,7 @@ void enqueue_pending_xmitbuf(
> list_add_tail(&pxmitbuf->list, get_list_head(pqueue));
> spin_unlock_bh(&pqueue->lock);
>
> - up(&(pri_adapter->xmitpriv.xmit_sema));
> + complete(&(pri_adapter->xmitpriv.xmit_comp));
> }
>
> void enqueue_pending_xmitbuf_to_head(
> @@ -2998,7 +2998,7 @@ int rtw_xmit_thread(void *context)
> flush_signals_thread();
> } while (_SUCCESS == err);
>
> - up(&padapter->xmitpriv.terminate_xmitthread_sema);
> + complete(&padapter->xmitpriv.terminate_xmitthread_comp);
>
> thread_exit();
> }
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> index c7e55618b9a8..85fd12cca4ae 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> @@ -4502,8 +4502,8 @@ void rtl8723b_stop_thread(struct adapter *padapter)
>
> /* stop xmit_buf_thread */
> if (xmitpriv->SdioXmitThread) {
> - up(&xmitpriv->SdioXmitSema);
> - down(&xmitpriv->SdioXmitTerminateSema);
> + complete(&xmitpriv->SdioXmitStart);
> + wait_for_completion(&xmitpriv->SdioXmitTerminate);
> xmitpriv->SdioXmitThread = NULL;
> }
> #endif
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> index 10b3f9733bad..69c4db5b5b0c 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> @@ -148,7 +148,7 @@ s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
>
> pxmitpriv = &padapter->xmitpriv;
>
> - if (down_interruptible(&pxmitpriv->xmit_sema)) {
> + if (wait_for_completion_interruptible(&pxmitpriv->xmit_comp)) {
> DBG_871X_LEVEL(_drv_emerg_, "%s: down SdioXmitBufSema fail!\n", __func__);
> return _FAIL;
> }
> @@ -312,7 +312,7 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
> DBG_871X_LEVEL(_drv_err_, "%s: xmit_buf is not enough!\n", __func__);
> #endif
> err = -2;
> - up(&(pxmitpriv->xmit_sema));
> + complete(&(pxmitpriv->xmit_comp));
> break;
> }
> k = 0;
> @@ -420,8 +420,8 @@ static s32 rtl8723bs_xmit_handler(struct adapter *padapter)
>
> pxmitpriv = &padapter->xmitpriv;
>
> - if (down_interruptible(&pxmitpriv->SdioXmitSema)) {
> - DBG_871X_LEVEL(_drv_emerg_, "%s: down sema fail!\n", __func__);
> + if (wait_for_completion_interruptible(&pxmitpriv->SdioXmitStart)) {
> + DBG_871X_LEVEL(_drv_emerg_, "%s: SdioXmitStart fail!\n", __func__);
> return _FAIL;
> }
>
> @@ -490,10 +490,6 @@ int rtl8723bs_xmit_thread(void *context)
>
> DBG_871X("start "FUNC_ADPT_FMT"\n", FUNC_ADPT_ARG(padapter));
>
> - /* For now, no one would down sema to check thread is running, */
> - /* so mark this temporary, Lucas@20130820 */
> -/* up(&pxmitpriv->SdioXmitTerminateSema); */
> -
> do {
> ret = rtl8723bs_xmit_handler(padapter);
> if (signal_pending(current)) {
> @@ -501,7 +497,7 @@ int rtl8723bs_xmit_thread(void *context)
> }
> } while (_SUCCESS == ret);
>
> - up(&pxmitpriv->SdioXmitTerminateSema);
> + complete(&pxmitpriv->SdioXmitTerminate);
>
> RT_TRACE(_module_hal_xmit_c_, _drv_notice_, ("-%s\n", __func__));
>
> @@ -590,7 +586,7 @@ s32 rtl8723bs_hal_xmit(
> return true;
> }
>
> - up(&pxmitpriv->SdioXmitSema);
> + complete(&pxmitpriv->SdioXmitStart);
>
> return false;
> }
> @@ -611,7 +607,7 @@ s32 rtl8723bs_hal_xmitframe_enqueue(
> #ifdef CONFIG_SDIO_TX_TASKLET
> tasklet_hi_schedule(&pxmitpriv->xmit_tasklet);
> #else
> - up(&pxmitpriv->SdioXmitSema);
> + complete(&pxmitpriv->SdioXmitStart);
> #endif
> }
>
> @@ -634,8 +630,8 @@ s32 rtl8723bs_init_xmit_priv(struct adapter *padapter)
> phal = GET_HAL_DATA(padapter);
>
> spin_lock_init(&phal->SdioTxFIFOFreePageLock);
> - sema_init(&xmitpriv->SdioXmitSema, 0);
> - sema_init(&xmitpriv->SdioXmitTerminateSema, 0);
> + init_completion(&xmitpriv->SdioXmitStart);
> + init_completion(&xmitpriv->SdioXmitTerminate);
>
> return _SUCCESS;
> }
> diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c b/drivers/staging/rtl8723bs/hal/sdio_ops.c
> index d6b93e1f78d8..3fee34484577 100644
> --- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
> +++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
> @@ -1023,7 +1023,7 @@ void sd_int_dpc(struct adapter *adapter)
> u8 freepage[4];
>
> _sdio_local_read(adapter, SDIO_REG_FREE_TXPG, 4, freepage);
> - up(&(adapter->xmitpriv.xmit_sema));
> + complete(&(adapter->xmitpriv.xmit_comp));
> }
>
> if (hal->sdio_hisr & SDIO_HISR_CPWM1) {
> diff --git a/drivers/staging/rtl8723bs/include/rtw_cmd.h b/drivers/staging/rtl8723bs/include/rtw_cmd.h
> index 7dc5bf050839..299b55538788 100644
> --- a/drivers/staging/rtl8723bs/include/rtw_cmd.h
> +++ b/drivers/staging/rtl8723bs/include/rtw_cmd.h
> @@ -7,6 +7,7 @@
> #ifndef __RTW_CMD_H_
> #define __RTW_CMD_H_
>
> +#include <linux/completion.h>
>
> #define C2H_MEM_SZ (16*1024)
>
> @@ -27,7 +28,6 @@
> u8 *rsp;
> u32 rspsz;
> struct submit_ctx *sctx;
> - /* _sema cmd_sem; */
> struct list_head list;
> };
>
> @@ -38,9 +38,8 @@
> };
>
> struct cmd_priv {
> - _sema cmd_queue_sema;
> - /* _sema cmd_done_sema; */
> - _sema terminate_cmdthread_sema;
> + struct completion cmd_queue_comp;
> + struct completion terminate_cmdthread_comp;
> struct __queue cmd_queue;
> u8 cmd_seq;
> u8 *cmd_buf; /* shall be non-paged, and 4 bytes aligned */
> diff --git a/drivers/staging/rtl8723bs/include/rtw_xmit.h b/drivers/staging/rtl8723bs/include/rtw_xmit.h
> index a75b668d09a6..1b38b9182b31 100644
> --- a/drivers/staging/rtl8723bs/include/rtw_xmit.h
> +++ b/drivers/staging/rtl8723bs/include/rtw_xmit.h
> @@ -7,6 +7,7 @@
> #ifndef _RTW_XMIT_H_
> #define _RTW_XMIT_H_
>
> +#include <linux/completion.h>
>
> #define MAX_XMITBUF_SZ (20480) /* 20k */
>
> @@ -364,8 +365,8 @@ struct xmit_priv {
>
> _lock lock;
>
> - _sema xmit_sema;
> - _sema terminate_xmitthread_sema;
> + struct completion xmit_comp;
> + struct completion terminate_xmitthread_comp;
>
> /* struct __queue blk_strms[MAX_NUMBLKS]; */
> struct __queue be_pending;
> @@ -419,8 +420,8 @@ struct xmit_priv {
> struct tasklet_struct xmit_tasklet;
> #else
> void *SdioXmitThread;
> - _sema SdioXmitSema;
> - _sema SdioXmitTerminateSema;
> + struct completion SdioXmitStart;
> + struct completion SdioXmitTerminate;
> #endif /* CONFIG_SDIO_TX_TASKLET */
>
> struct __queue free_xmitbuf_queue;
> diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> index 181642358e3f..143e3f9b31aa 100644
> --- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> +++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> @@ -585,7 +585,7 @@ u32 rtw_start_drv_threads(struct adapter *padapter)
> if (IS_ERR(padapter->cmdThread))
> _status = _FAIL;
> else
> - down(&padapter->cmdpriv.terminate_cmdthread_sema); /* wait for cmd_thread to run */
> + wait_for_completion(&padapter->cmdpriv.terminate_cmdthread_comp); /* wait for cmd_thread to run */
>
> rtw_hal_start_thread(padapter);
> return _status;
> @@ -598,8 +598,8 @@ void rtw_stop_drv_threads (struct adapter *padapter)
> rtw_stop_cmd_thread(padapter);
>
> /* Below is to termindate tx_thread... */
> - up(&padapter->xmitpriv.xmit_sema);
> - down(&padapter->xmitpriv.terminate_xmitthread_sema);
> + complete(&padapter->xmitpriv.xmit_comp);
> + wait_for_completion(&padapter->xmitpriv.terminate_xmitthread_comp);
> RT_TRACE(_module_os_intfs_c_, _drv_info_, ("\n drv_halt: rtw_xmit_thread can be terminated !\n"));
>
> rtw_hal_stop_thread(padapter);
> diff --git a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> index 2cf903c66854..4e4e565d991c 100644
> --- a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> +++ b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> @@ -101,7 +101,7 @@ void rtw_os_xmit_schedule(struct adapter *padapter)
> return;
>
> if (!list_empty(&padapter->xmitpriv.pending_xmitbuf_queue.queue))
> - up(&pri_adapter->xmitpriv.xmit_sema);
> + complete(&pri_adapter->xmitpriv.xmit_comp);
> }
>
> static void rtw_check_xmit_resource(struct adapter *padapter, _pkt *pkt)
>

\
 
 \ /
  Last update: 2018-12-11 13:32    [W:0.042 / U:0.508 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site