| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <34f364a5ed0703b562b631efa1f3cd1fd8f3a93a.camel@ziswiler.com>
Date: Thu, 14 Dec 2023 11:44:29 +0100
From: Marcel Ziswiler <marcel@...wiler.com>
To: David Lin <yu-hao.lin@....com>, linux-wireless@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, briannorris@...omium.org,
kvalo@...nel.org, francesco@...cini.it, tsung-hsien.hsieh@....com,
stable@...r.kernel.org
Subject: Re: [PATCH v3] wifi: mwifiex: add extra delay for firmware ready
On Sat, 2023-12-09 at 07:40 +0800, David Lin wrote:
> For SDIO IW416, due to a bug, FW may return ready before complete
> full initialization.
BTW: What makes you think this issue is exclusive to the IW416?
We have also seen this in the past both on our Verdin iMX8M Mini (SDIO/SDIO) and Verdin iMX8M Plus (SDIO/UART)
with 88W8997.
good case:
[ 6.496541] mwifiex_sdio mmc0:0001:1: info: FW download over, size 594556 bytes
...
[ 7.272436] mwifiex_sdio mmc0:0001:1: WLAN FW is active
[ 7.314958] mwifiex_sdio mmc0:0001:1: Unknown api_id: 5
[ 7.347647] mwifiex_sdio mmc0:0001:1: info: MWIFIEX VERSION: mwifiex 1.0 (16.92.21.p55)
[ 7.355977] mwifiex_sdio mmc0:0001:1: driver_version = mwifiex 1.0 (16.92.21.p55)
bad case:
[ 8.720216] mwifiex_sdio mmc0:0001:1: info: FW download over, size 594556 bytes
...
[ 24.976699] mwifiex_sdio mmc0:0001:1: FW failed to be active in time
[ 24.983098] mwifiex_sdio mmc0:0001:1: info: _mwifiex_fw_dpc: unregister device
> 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@....com>
> Cc: stable@...r.kernel.org
Tested-by: Marcel Ziswiler <marcel.ziswiler@...adex.com> # Verdin AM62 (IW416)
> ---
>
> v3:
> - v2 was a not finished patch that was send to the LKML by mistake
> - changed check condition for extra delay with clear comments.
> - added flag to struct mwifiex_sdio_device / mwifiex_sdio_sd8978 to
> enable extra delay only for IW416.
> ---
> drivers/net/wireless/marvell/mwifiex/sdio.c | 19 +++++++++++++++++++
> drivers/net/wireless/marvell/mwifiex/sdio.h | 2 ++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index 6462a0ffe698..ef3e68d1059c 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -331,6 +331,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8786 = {
> .can_dump_fw = false,
> .can_auto_tdls = false,
> .can_ext_scan = false,
> + .fw_ready_extra_delay = false,
> };
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8787 = {
> @@ -346,6 +347,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8787 = {
> .can_dump_fw = false,
> .can_auto_tdls = false,
> .can_ext_scan = true,
> + .fw_ready_extra_delay = false,
> };
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8797 = {
> @@ -361,6 +363,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8797 = {
> .can_dump_fw = false,
> .can_auto_tdls = false,
> .can_ext_scan = true,
> + .fw_ready_extra_delay = false,
> };
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8897 = {
> @@ -376,6 +379,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8897 = {
> .can_dump_fw = true,
> .can_auto_tdls = false,
> .can_ext_scan = true,
> + .fw_ready_extra_delay = false,
> };
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8977 = {
> @@ -392,6 +396,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8977 = {
> .fw_dump_enh = true,
> .can_auto_tdls = false,
> .can_ext_scan = true,
> + .fw_ready_extra_delay = false,
> };
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8978 = {
> @@ -408,6 +413,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8978 = {
> .fw_dump_enh = true,
> .can_auto_tdls = false,
> .can_ext_scan = true,
> + .fw_ready_extra_delay = true,
> };
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8997 = {
> @@ -425,6 +431,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8997 = {
> .fw_dump_enh = true,
> .can_auto_tdls = false,
> .can_ext_scan = true,
> + .fw_ready_extra_delay = false,
> };
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8887 = {
> @@ -440,6 +447,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8887 = {
> .can_dump_fw = false,
> .can_auto_tdls = true,
> .can_ext_scan = true,
> + .fw_ready_extra_delay = false,
> };
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8987 = {
> @@ -456,6 +464,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8987 = {
> .fw_dump_enh = true,
> .can_auto_tdls = true,
> .can_ext_scan = true,
> + .fw_ready_extra_delay = false,
> };
>
> static const struct mwifiex_sdio_device mwifiex_sdio_sd8801 = {
> @@ -471,6 +480,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8801 = {
> .can_dump_fw = false,
> .can_auto_tdls = false,
> .can_ext_scan = true,
> + .fw_ready_extra_delay = false,
> };
>
> static struct memory_type_mapping generic_mem_type_map[] = {
> @@ -563,6 +573,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct sdio_device_id *id)
> card->fw_dump_enh = data->fw_dump_enh;
> card->can_auto_tdls = data->can_auto_tdls;
> card->can_ext_scan = data->can_ext_scan;
> + card->fw_ready_extra_delay = data->fw_ready_extra_delay;
> INIT_WORK(&card->work, mwifiex_sdio_work);
> }
>
> @@ -766,6 +777,7 @@ mwifiex_sdio_read_fw_status(struct mwifiex_adapter *adapter, u16 *dat)
> static int mwifiex_check_fw_status(struct mwifiex_adapter *adapter,
> u32 poll_num)
> {
> + struct sdio_mmc_card *card = adapter->card;
> int ret = 0;
> u16 firmware_stat;
> u32 tries;
> @@ -783,6 +795,13 @@ static int mwifiex_check_fw_status(struct mwifiex_adapter *adapter,
> ret = -1;
> }
>
> + if (card->fw_ready_extra_delay &&
> + firmware_stat == FIRMWARE_READY_SDIO)
> + /* firmware might pretend to be ready, when it's not.
> + * Wait a little bit more as a workaround.
> + */
> + msleep(100);
> +
> return ret;
> }
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h b/drivers/net/wireless/marvell/mwifiex/sdio.h
> index b86a9263a6a8..cb63ad55d675 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.h
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
> @@ -255,6 +255,7 @@ struct sdio_mmc_card {
> bool fw_dump_enh;
> bool can_auto_tdls;
> bool can_ext_scan;
> + bool fw_ready_extra_delay;
>
> struct mwifiex_sdio_mpa_tx mpa_tx;
> struct mwifiex_sdio_mpa_rx mpa_rx;
> @@ -278,6 +279,7 @@ struct mwifiex_sdio_device {
> bool fw_dump_enh;
> bool can_auto_tdls;
> bool can_ext_scan;
> + bool fw_ready_extra_delay;
> };
>
> /*
>
> base-commit: 783004b6dbda2cfe9a552a4cc9c1d168a2068f6c
Powered by blists - more mailing lists