For SDIO IW416, in a corner case FW may return ready before complete full
initialization.
Command timeout may occur at driver load after reboot.
Workaround by adding 100ms delay at checking FW status.
Signed-off-by: David Lin <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/sdio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 6462a0ffe698..744e9403430a 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -783,6 +783,9 @@ static int mwifiex_check_fw_status(struct mwifiex_adapter *adapter,
ret = -1;
}
+ if (!ret)
+ msleep(100);
+
return ret;
}
base-commit: 783004b6dbda2cfe9a552a4cc9c1d168a2068f6c
--
2.25.1
Hello David,
thanks for your patch. Planning to run some test on this over the next
days and we'll provide some actual feedback if this really solves the
issue we are affected by.
Just a couple of nitpicky comments on the actual patch.
On the commit message you should use imperative mood, e.g.
`wifi: mwifiex: add extra delay for firmware ready`
with no period at the end of the line.
On Tue, Nov 28, 2023 at 04:25:44PM +0800, David Lin wrote:
> For SDIO IW416, in a corner case FW may return ready before complete full
> initialization.
> Command timeout may occur at driver load after reboot.
> Workaround by adding 100ms delay at checking FW status.
>
> Signed-off-by: David Lin <[email protected]>
Add
Cc: stable@...
> ---
> drivers/net/wireless/marvell/mwifiex/sdio.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index 6462a0ffe698..744e9403430a 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -783,6 +783,9 @@ static int mwifiex_check_fw_status(struct mwifiex_adapter *adapter,
> ret = -1;
> }
>
> + if (!ret)
> + msleep(100);
> +
you could just add the delay after
if (firmware_stat == FIRMWARE_READY_SDIO) {
this would be more read-able to me. Adding also a short comment like
/* Firmware might pretend to be ready, when it's not.
* Wait a little bit more as a workaround */
Francesco
> From: Francesco Dolcini <[email protected]>
> Sent: Wednesday, December 6, 2023 4:01 AM
> To: David Lin <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Pete
> Hsieh <[email protected]>
> Subject: [EXT] Re: [PATCH] wifi: mwifiex: added extra delay for firmware ready.
>
> 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. Planning to run some test on this over the next days and
> we'll provide some actual feedback if this really solves the issue we are
> affected by.
>
> Just a couple of nitpicky comments on the actual patch.
>
>
> On the commit message you should use imperative mood, e.g.
>
> `wifi: mwifiex: add extra delay for firmware ready`
>
> with no period at the end of the line.
>
Thanks. I will fix it in patch v2.
> On Tue, Nov 28, 2023 at 04:25:44PM +0800, David Lin wrote:
> > For SDIO IW416, in a corner case FW may return ready before complete
> > full initialization.
> > Command timeout may occur at driver load after reboot.
> > Workaround by adding 100ms delay at checking FW status.
> >
> > Signed-off-by: David Lin <[email protected]>
>
> Add
>
> Cc: stable@...
>
Thanks. I will add "cc: stable" in patch v2.
> > ---
> > drivers/net/wireless/marvell/mwifiex/sdio.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > index 6462a0ffe698..744e9403430a 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > @@ -783,6 +783,9 @@ static int mwifiex_check_fw_status(struct
> mwifiex_adapter *adapter,
> > ret = -1;
> > }
> >
> > + if (!ret)
> > + msleep(100);
> > +
>
> you could just add the delay after
>
> if (firmware_stat == FIRMWARE_READY_SDIO) {
>
> this would be more read-able to me. Adding also a short comment like
>
> /* Firmware might pretend to be ready, when it's not.
> * Wait a little bit more as a workaround */
>
O.K.
>
> Francesco