| Message ID | 20231128082544.613179-1-yu-hao.lin@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: mwifiex: added extra delay for firmware ready. | expand |
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. Do you have any idea why? Is it specific to this chip and/or firmware? I'm hesitant to add magic sleeps to everything, just because you have one buggy chip/firmware. If it's a known issue with a single chip, it seems like you should add a flag to struct mwifiex_sdio_device / mwifiex_sdio_sd8978. Brian > Workaround by adding 100ms delay at checking FW status. > > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> From: Francesco Dolcini <francesco@dolcini.it> > Sent: Wednesday, December 6, 2023 4:01 AM > 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; Pete > Hsieh <tsung-hsien.hsieh@nxp.com> > 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 <yu-hao.lin@nxp.com> > > 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
> From: Brian Norris <briannorris@chromium.org> > Sent: Wednesday, December 6, 2023 4:27 AM > To: David Lin <yu-hao.lin@nxp.com> > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > kvalo@kernel.org; francesco@dolcini.it; Pete Hsieh > <tsung-hsien.hsieh@nxp.com> > 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 > > > 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. > > Do you have any idea why? Is it specific to this chip and/or firmware? > I'm hesitant to add magic sleeps to everything, just because you have one > buggy chip/firmware. > > If it's a known issue with a single chip, it seems like you should add a flag to > struct mwifiex_sdio_device / mwifiex_sdio_sd8978. > This issue is reported by customer We confirmed it's specific to this chip and 100ms is sufficient and relatively safe/easy then change FW. Will add flag to struct mwifiex_sdio_device / mwifiex_sdio_sd8978 > Brian > > > Workaround by adding 100ms delay at checking FW status. > > > > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
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; }
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 <yu-hao.lin@nxp.com> --- drivers/net/wireless/marvell/mwifiex/sdio.c | 3 +++ 1 file changed, 3 insertions(+) base-commit: 783004b6dbda2cfe9a552a4cc9c1d168a2068f6c