Re: [PATCH wireless-next v2 1/2] wifi: wfx: fix possible device hang during init

["Sverdlin, Alexander" <alexander.sverdlin@siemens.com>]

Hi Jérôme!

Thanks for the patch! Just one pre-existing issue below:

On Tue, 2026-09-01 at 10:33 +0200, Jérôme Pouiller wrote:
> The WFxxx device provides an SDIO interface. In addition to the usual
> in-band SDIO IRQ, the device also supports out-of-band IRQ line. OOB IRQ
> requires a specific initialization described in commit f00dc1d789e1c
> ("staging: wfx: poll IRQ during init"):
> 
>     When the chip starts in SDIO mode, the external IRQ (aka Out-Of-Band
>     IRQ) cannot be used before to configure it. Therefore, the first
>     exchanges with the chip have to be done without the OOB IRQ.
> 
>     This patch allow to poll the data until the OOB IRQ is correctly
>     setup. In order to keep the code simpler, this patch also poll data
>     even if OOB IRQ is not used.
> 
> Commit 57aa557f110d9 ("staging: wfx: introduce a way to poll IRQ")
> announce some limitation about the data polling mechanism:
> 
>     This function [wfx_bh_poll_irq()] must used with care: if an IRQ
>     fires while the host reads control register, the IRQ can be lost.
> 
> However, with last analysis, it seems it does not only impact the IRQ.
> When this condition happen the hardware is lost and won't reply the next
> commands. The only way to recover the error is to pull the reset pin and
> bind the device again.
> 
> Currently wfx_bh_poll_irq() is only used during the two first exchanges
> with the hardware. Therefore, it has been reported to only happen after
> a few hours of bind/rebind in a loop. This explain why it has not been
> reported before.
> 
> Fortunately, wfx_bh_poll_irq() is only required for OOB IRQ, which is
> probably very marginal. The In-Band IRQ case also uses this function,
> but only to simplify the code.
> 
> This patch limits the use of wfx_bh_poll_irq() to the OOB IRQ. The bus
> driver now tells wfx_probe() whether polling is necessary. When it is
> not (in-band SDIO IRQ and SPI), the IRQ is subscribed before the first
> exchange with the device and wfx_bh_poll_irq() is never called. When it
> is (SDIO with OOB IRQ), the behavior is unchanged: the control register
> is polled until the PDS file has configured the IRQ line.
> 
> To conclude, In-Band users are now safe. Users of OOB IRQ will be still
> impacted by the bug, but there is nothing we can do (and nobody
> complained during the last 6 years).
> 
> Fixes: f00dc1d789e1c ("staging: wfx: poll IRQ during init")
> Reported-by: Gerard Salvatella <gerard.salvatella@siemens.com>
> Reported-by: Lukas Stockmann <lukas.stockmann@siemens.com>
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/net/wireless/silabs/wfx/bh.c       |  6 ++--
>  drivers/net/wireless/silabs/wfx/bus_sdio.c |  1 +
>  drivers/net/wireless/silabs/wfx/main.c     | 34 ++++++++++++++--------
>  3 files changed, 26 insertions(+), 15 deletions(-)

[]

> diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
> index dda36e41eed13..4e99fe7e5bb78 100644
> --- a/drivers/net/wireless/silabs/wfx/main.c
> +++ b/drivers/net/wireless/silabs/wfx/main.c
> @@ -399,12 +406,12 @@ int wfx_probe(struct wfx_dev *wdev)
>  		dev_err(wdev->dev, "unsupported firmware API version (expect 1 while firmware returns %d)\n",
>  			wdev->hw_caps.api_version_major);
>  		err = -EOPNOTSUPP;
> -		goto bh_unregister;
> +		goto irq_unsubscribe;
>  	}
>  
>  	if (wdev->hw_caps.link_mode == SEC_LINK_ENFORCED) {
>  		dev_err(wdev->dev, "chip require secure_link, but can't negotiate it\n");
> -		goto bh_unregister;
> +		goto irq_unsubscribe;
>  	}
>  
>  	if (wdev->hw_caps.region_sel_mode) {

At this point err still holds the (positive) return value from wait_for_completion_timeout,
so wfx_probe() would return a positive value (as failure).

But maybe it deserves a separate patch.

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com