diff mbox series

wifi: mwifiex: added delay after firmware downloaded.

Message ID PA4PR04MB96388864C8B07F2B3AA7964AD1DBA@PA4PR04MB9638.eurprd04.prod.outlook.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series wifi: mwifiex: added delay after firmware downloaded. | expand

Commit Message

David Lin Oct. 20, 2023, 3:35 a.m. UTC
Added 100 ms delayed after firmware is downloaded and active in order to
avoid command timeout for the first command sent to firmware.

Signed-off-by: David Lin <yu-hao.lin@nxp.com>
---
 drivers/net/wireless/marvell/mwifiex/main.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Francesco Dolcini Oct. 21, 2023, 12:56 p.m. UTC | #1
Hello David,
thanks for your patch.

On Fri, Oct 20, 2023 at 03:35:34AM +0000, David Lin wrote:
> Added 100 ms delayed after firmware is downloaded and active in order to
> avoid command timeout for the first command sent to firmware.

Please have a look at Documentation/process/submitting-patches.rst.
In very brief:
 - you should use imperative mood to describe your changes
 - this is fixing a real issue, I believe this is worth a more complete commit
   message, including a log of the failure this is solving.

I am also inclined to think that this change should be backported and it likely
would need a Fixes: tag.

> ---
>  drivers/net/wireless/marvell/mwifiex/main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
> index d99127dc466e..a25e8bb8daec 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> @@ -570,6 +570,7 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context)
>  		goto err_dnld_fw;
>  
>  	mwifiex_dbg(adapter, MSG, "WLAN FW is active\n");
> +	mdelay(100);

from my understanding there is already a function polling for the firmware to be
active, e.g. mwifiex_check_fw_status(). Maybe is that function that needs to be
fixed instead of adding an additional delay here? 

Francesco
David Lin Oct. 23, 2023, 2:12 a.m. UTC | #2
> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Saturday, October 21, 2023 8:56 PM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> briannorris@chromium.org; kvalo@kernel.org; francesco@dolcini.it; Sharvari
> Harisangam <sharvari.harisangam@nxp.com>; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>
> Subject: [EXT] Re: [PATCH] wifi: mwifiex: added delay after firmware
> downloaded.
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hello David,
> thanks for your patch.
> 
> On Fri, Oct 20, 2023 at 03:35:34AM +0000, David Lin wrote:
> > Added 100 ms delayed after firmware is downloaded and active in order
> > to avoid command timeout for the first command sent to firmware.
> 
> Please have a look at Documentation/process/submitting-patches.rst.
> In very brief:
>  - you should use imperative mood to describe your changes
>  - this is fixing a real issue, I believe this is worth a more complete commit
>    message, including a log of the failure this is solving.
> 
> I am also inclined to think that this change should be backported and it likely
> would need a Fixes: tag.
> 
> > ---
> >  drivers/net/wireless/marvell/mwifiex/main.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/main.c
> > b/drivers/net/wireless/marvell/mwifiex/main.c
> > index d99127dc466e..a25e8bb8daec 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/main.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> > @@ -570,6 +570,7 @@ static int _mwifiex_fw_dpc(const struct firmware
> *firmware, void *context)
> >               goto err_dnld_fw;
> >
> >       mwifiex_dbg(adapter, MSG, "WLAN FW is active\n");
> > +     mdelay(100);
> 
> from my understanding there is already a function polling for the firmware to
> be active, e.g. mwifiex_check_fw_status(). Maybe is that function that needs
> to be fixed instead of adding an additional delay here?
> 

O.K. I will check the function to see if I can add this modification to the function.

> Francesco
diff mbox series

Patch

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index d99127dc466e..a25e8bb8daec 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -570,6 +570,7 @@  static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context)
 		goto err_dnld_fw;
 
 	mwifiex_dbg(adapter, MSG, "WLAN FW is active\n");
+	mdelay(100);
 
 	if (cal_data_cfg) {
 		if ((request_firmware(&adapter->cal_data, cal_data_cfg,