RE: [PATCH v2] mwifiex: fix race condition when downloading firmware

From: Amitkumar Karwar
Date: Thu Jun 09 2016 - 09:30:51 EST


> From: Wei-Ning Huang [mailto:wnhuang@xxxxxxxxxxxx]
> Sent: Friday, May 27, 2016 5:58 PM
> To: Linux-Wireless
> Cc: LKML; Amitkumar Karwar; djkurtz@xxxxxxxxxxxx; Wei-Ning Huang
> Subject: [PATCH v2] mwifiex: fix race condition when downloading
> firmware
>
> The action 'check for winner' and 'download firmware' should be an
> atomic action. This is true for btmrvl driver but not mwmfiex, which
> cause firmware download to fail when the following scenario happens:
>
> 1) mwifiex check winner status: true
> 2) btmrvl check winner status: true, and start downloading firmware
> 3) mwifiex tries to download firmware, but failed because btmrvl is
> already downloading.
>
> This won't happen if 1) and 3) is an atomic action. This patch adds
> sdio_claim/release_host call around those two actions to make sure it's
> atomic.
>
> Signed-off-by: Wei-Ning Huang <wnhuang@xxxxxxxxxxxx>
> ---
> drivers/net/wireless/marvell/mwifiex/init.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/init.c
> b/drivers/net/wireless/marvell/mwifiex/init.c
> index 78c532f..7703438 100644
> --- a/drivers/net/wireless/marvell/mwifiex/init.c
> +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> @@ -24,6 +24,7 @@
> #include "main.h"
> #include "wmm.h"
> #include "11n.h"
> +#include "sdio.h"
>
> /*
> * This function adds a BSS priority table to the table list.
> @@ -737,16 +738,20 @@ mwifiex_shutdown_drv(struct mwifiex_adapter
> *adapter) int mwifiex_dnld_fw(struct mwifiex_adapter *adapter,
> struct mwifiex_fw_image *pmfw)
> {
> + struct sdio_mmc_card *card = adapter->card;
> int ret;
> u32 poll_num = 1;
>
> + if (adapter->iface_type == MWIFIEX_SDIO)
> + sdio_claim_host(card->func);
> +
> if (adapter->if_ops.check_fw_status) {
> /* check if firmware is already running */
> ret = adapter->if_ops.check_fw_status(adapter, poll_num);
> if (!ret) {
> mwifiex_dbg(adapter, MSG,
> "WLAN FW already running! Skip FW dnld\n");
> - return 0;
> + goto release_host;
> }
> }
>
> @@ -759,7 +764,7 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter,
> if (ret) {
> mwifiex_dbg(adapter, MSG,
> "WLAN read winner status failed!\n");
> - return ret;
> + goto release_host;
> }
>
> if (!adapter->winner) {
> @@ -775,7 +780,7 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter,
> if (ret) {
> mwifiex_dbg(adapter, ERROR,
> "prog_fw failed ret=%#x\n", ret);
> - return ret;
> + goto release_host;
> }
> }
>
> @@ -785,6 +790,9 @@ poll_fw:
> if (ret)
> mwifiex_dbg(adapter, ERROR,
> "FW failed to be active in time\n");
> +release_host:
> + if (adapter->iface_type == MWIFIEX_SDIO)
> + sdio_release_host(card->func);
>
> return ret;
> }
> --
> 2.1.2

Patch looks fine to me.

Acked-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx>

Regards,
Amitkumar