RE: [PATCH 1/3] mwifiex: fix misleading firmware download messages

[Amitkumar Karwar]

Hi Kalle,

> From: Kalle Valo [mailto:kv...@codeaurora.org]
> Sent: Tuesday, January 05, 2016 3:41 PM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam; Jeff
> CF Chen
> Subject: Re: [PATCH 1/3] mwifiex: fix misleading firmware download
> messages
> 
> Amitkumar Karwar <akar...@marvell.com> writes:
> 
> > From: chunfan chen <je...@marvell.com>
> >
> > mwifiex_check_fw_status() checks firmware status and also check if
> > WLAN is the winner for firmware downloading.
> >
> > Once we detect that other interface is downloading the firmware, we
> > call this routine again with max poll count to wait until firmware is
> > ready.
> >
> > This patch splits the routine to avoid checking winner status multiple
> > times and ensures that correct warning messages are displayed.
> >
> > Firmware status poll count is also increased to 150.
> 
> The title is somewhat misleading, you are doing a lot more than just
> fixing messages. Also please clarify the concept here, especially about
> "winner", "loser" and the other interface.
> 

Thanks for review. I will submit updated version with below changes.
1) Change title to "mwifiex: firmware download enhancements"
2) Modified description to explain "winner", "loser" and the other interface 
concepts.

> > Signed-off-by: chunfan chen <je...@marvell.com>
> 
> It's good practise to capitalise the name.
> 

Ack.

Regards,
Amitkumar Karwar
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] mwifiex: fix misleading firmware download messages

[Kalle Valo]

Amitkumar Karwar  writes:

> From: chunfan chen 
>
> mwifiex_check_fw_status() checks firmware status and also
> check if WLAN is the winner for firmware downloading.
>
> Once we detect that other interface is downloading
> the firmware, we call this routine again with max
> poll count to wait until firmware is ready.
>
> This patch splits the routine to avoid checking
> winner status multiple times and ensures that
> correct warning messages are displayed.
>
> Firmware status poll count is also increased to 150.

The title is somewhat misleading, you are doing a lot more than just
fixing messages. Also please clarify the concept here, especially about
"winner", "loser" and the other interface.

> Signed-off-by: chunfan chen 

It's good practise to capitalise the name.

-- 
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] mwifiex: fix misleading firmware download messages

[Amitkumar Karwar]

From: chunfan chen 

mwifiex_check_fw_status() checks firmware status and also
check if WLAN is the winner for firmware downloading.

Once we detect that other interface is downloading
the firmware, we call this routine again with max
poll count to wait until firmware is ready.

This patch splits the routine to avoid checking
winner status multiple times and ensures that
correct warning messages are displayed.

Firmware status poll count is also increased to 150.

Signed-off-by: chunfan chen 
Signed-off-by: Amitkumar Karwar 
---
 drivers/net/wireless/marvell/mwifiex/fw.h   |  2 +-
 drivers/net/wireless/marvell/mwifiex/init.c | 16 +---
 drivers/net/wireless/marvell/mwifiex/main.h |  1 +
 drivers/net/wireless/marvell/mwifiex/pcie.c | 40 +
 drivers/net/wireless/marvell/mwifiex/sdio.c | 33 ++--
 5 files changed, 57 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h 
b/drivers/net/wireless/marvell/mwifiex/fw.h
index ced7af2..426e76a 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -96,7 +96,7 @@ enum KEY_TYPE_ID {
 #define WAPI_KEY_LEN   (WLAN_KEY_LEN_SMS4 + PN_LEN + 2)
 
 #define MAX_POLL_TRIES 100
-#define MAX_FIRMWARE_POLL_TRIES100
+#define MAX_FIRMWARE_POLL_TRIES150
 
 #define FIRMWARE_READY_SDIO0xfedc
 #define FIRMWARE_READY_PCIE0xfedcba00
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c 
b/drivers/net/wireless/marvell/mwifiex/init.c
index 6f7876e..517653b 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -741,8 +741,6 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter,
u32 poll_num = 1;
 
if (adapter->if_ops.check_fw_status) {
-   adapter->winner = 0;
-
/* check if firmware is already running */
ret = adapter->if_ops.check_fw_status(adapter, poll_num);
if (!ret) {
@@ -750,13 +748,23 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter,
"WLAN FW already running! Skip FW dnld\n");
return 0;
}
+   }
+
+   /* check if we are the winner for downloading FW */
+   if (adapter->if_ops.check_winner_status) {
+   adapter->winner = 0;
+   ret = adapter->if_ops.check_winner_status(adapter);
 
poll_num = MAX_FIRMWARE_POLL_TRIES;
+   if (ret) {
+   mwifiex_dbg(adapter, MSG,
+   "WLAN read winner status failed!\n");
+   return ret;
+   }
 
-   /* check if we are the winner for downloading FW */
if (!adapter->winner) {
mwifiex_dbg(adapter, MSG,
-   "FW already running! Skip FW dnld\n");
+   "WLAN is not the winner! Skip FW dnld\n");
goto poll_fw;
}
}
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h 
b/drivers/net/wireless/marvell/mwifiex/main.h
index 2f7f478..c08be79 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -791,6 +791,7 @@ struct mwifiex_if_ops {
int (*init_if) (struct mwifiex_adapter *);
void (*cleanup_if) (struct mwifiex_adapter *);
int (*check_fw_status) (struct mwifiex_adapter *, u32);
+   int (*check_winner_status)(struct mwifiex_adapter *);
int (*prog_fw) (struct mwifiex_adapter *, struct mwifiex_fw_image *);
int (*register_dev) (struct mwifiex_adapter *);
void (*unregister_dev) (struct mwifiex_adapter *);
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c 
b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 9703848..4000357 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2007,14 +2007,12 @@ done:
 
 /*
  * This function checks the firmware status in card.
- *
- * The winner interface is also determined by this function.
  */
 static int
 mwifiex_check_fw_status(struct mwifiex_adapter *adapter, u32 poll_num)
 {
int ret = 0;
-   u32 firmware_stat, winner_status;
+   u32 firmware_stat;
struct pcie_service_card *card = adapter->card;
const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
u32 tries;
@@ -2054,19 +2052,28 @@ mwifiex_check_fw_status(struct mwifiex_adapter 
*adapter, u32 poll_num)
}
}
 
-   if (ret) {
-   if (mwifiex_read_reg(adapter, reg->fw_status,
-_status))
-   ret = -1;
-   else if