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

["Jérôme Pouiller" <jerome.pouiller@silabs.com>]

On Tuesday 1 September 2026 12:43:22 Central European Summer Time Sverdlin, Alexander wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> 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.

To avoid confusion in the reviews (since this an unrelated issue), I will
place this change in a second series.

-- 
Jérôme Pouiller