Devicetree
 help / color / mirror / Atom feed
* [PATCH wireless-next 0/2] wifi: wfx: fix possible device hang during init
@ 2026-08-31 16:40 Jérôme Pouiller
  2026-08-31 16:40 ` [PATCH wireless-next 1/2] " Jérôme Pouiller
  2026-08-31 16:40 ` [PATCH wireless-next 2/2] dt-bindings: net: wireless: wfx: discourage OOB IRQ with SDIO Jérôme Pouiller
  0 siblings, 2 replies; 5+ messages in thread
From: Jérôme Pouiller @ 2026-08-31 16:40 UTC (permalink / raw)
  To: linux-wireless, devicetree
  Cc: linux-kernel, linux-devel, Johannes Berg, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Lukas Stockmann,
	Gerard Salvatella, Jérôme Pouiller

The WFxxx devices provide an SDIO interface. Besides the usual in-band
SDIO IRQ, they also support an out-of-band (OOB) IRQ line declared in the
Device Tree.

The OOB IRQ line is not usable until the device has been configured by
the PDS file. Therefore, since commit f00dc1d789e1c ("staging: wfx: poll
IRQ during init"), the driver polls the control register during the first
exchanges with the device instead of relying on an IRQ. To keep the code
simpler, this polling was applied unconditionally, including when an IRQ
was readily available (in-band SDIO IRQ and SPI).

Unfortunately, this polling is unsafe. Commit 57aa557f110d9 ("staging:
wfx: introduce a way to poll IRQ") already mentioned that an IRQ could be
lost if it fires while the host reads the control register. A recent
analysis shows the consequences are worse than documented: the device
itself gets stuck and stops answering subsequent commands. Only pulling
the reset pin and rebinding the device recovers it.

Since the polling only happens during the two first exchanges, the issue
is rare. It was caught by user while binding/rebinding the device in a loop
for a few hours.

Patch 1 restricts the polling to the case where it is actually required
(SDIO with OOB IRQ). The bus driver now tells wfx_probe() whether polling
is necessary; in every other case the IRQ is subscribed before the first
exchange and the racy path is never taken. This makes the vast majority
of the integrations safe.

The SDIO + OOB IRQ combination remains affected. There is no way to fix
it in software: polling is the only way to bootstrap that setup. Note that
nobody reported using it in the last 6 years.

Patch 2 documents this hardware limitation in the binding, so that new
designs do not pick this configuration.

[Copilot generated the introduction letter and reviewed the code]

Jérôme Pouiller (2):
  wifi: wfx: fix possible device hang during init
  dt-bindings: net: wireless: wfx: discourage OOB IRQ with SDIO

 .../bindings/net/wireless/silabs,wfx.yaml     |  3 ++
 drivers/net/wireless/silabs/wfx/bh.c          |  6 ++--
 drivers/net/wireless/silabs/wfx/bus_sdio.c    |  1 +
 drivers/net/wireless/silabs/wfx/main.c        | 31 ++++++++++++-------
 4 files changed, 27 insertions(+), 14 deletions(-)

-- 
2.47.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH wireless-next 1/2] wifi: wfx: fix possible device hang during init
  2026-08-31 16:40 [PATCH wireless-next 0/2] wifi: wfx: fix possible device hang during init Jérôme Pouiller
@ 2026-08-31 16:40 ` Jérôme Pouiller
  2026-08-31 19:23   ` sashiko-bot
  2026-08-31 16:40 ` [PATCH wireless-next 2/2] dt-bindings: net: wireless: wfx: discourage OOB IRQ with SDIO Jérôme Pouiller
  1 sibling, 1 reply; 5+ messages in thread
From: Jérôme Pouiller @ 2026-08-31 16:40 UTC (permalink / raw)
  To: linux-wireless, devicetree
  Cc: linux-kernel, linux-devel, Johannes Berg, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Lukas Stockmann,
	Gerard Salvatella, Jérôme Pouiller

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     | 31 ++++++++++++++--------
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/silabs/wfx/bh.c b/drivers/net/wireless/silabs/wfx/bh.c
index 21dfdcf9cc273..0071659bc9c98 100644
--- a/drivers/net/wireless/silabs/wfx/bh.c
+++ b/drivers/net/wireless/silabs/wfx/bh.c
@@ -284,10 +284,10 @@ void wfx_bh_request_tx(struct wfx_dev *wdev)
 }
 
 /* If IRQ is not available, this function allow to manually poll the control register and simulate
- * an IRQ ahen an event happened.
+ * an IRQ when an event happened.
  *
- * Note that the device has a bug: If an IRQ raise while host read control register, the IRQ is
- * lost. So, use this function carefully (only duing device initialisation).
+ * Note that the device has a bug: If an IRQ raise while host read control register, the device is
+ * lost. Unfortunately, this is the only way to initialize the SDIO with out-of-band IRQ.
  */
 void wfx_bh_poll_irq(struct wfx_dev *wdev)
 {
diff --git a/drivers/net/wireless/silabs/wfx/bus_sdio.c b/drivers/net/wireless/silabs/wfx/bus_sdio.c
index ab0793b9908f4..1ca2cc262feb2 100644
--- a/drivers/net/wireless/silabs/wfx/bus_sdio.c
+++ b/drivers/net/wireless/silabs/wfx/bus_sdio.c
@@ -279,6 +279,7 @@ static int wfx_sdio_probe(struct sdio_func *func, const struct sdio_device_id *i
 		goto sdio_release;
 	}
 
+	bus->core->poll_irq = !!bus->of_irq;
 	ret = wfx_probe(bus->core);
 	if (ret)
 		goto sdio_release;
diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
index dda36e41eed13..da30e8f481ada 100644
--- a/drivers/net/wireless/silabs/wfx/main.c
+++ b/drivers/net/wireless/silabs/wfx/main.c
@@ -362,7 +362,6 @@ int wfx_probe(struct wfx_dev *wdev)
 	 */
 	gpio_saved = wdev->pdata.gpio_wakeup;
 	wdev->pdata.gpio_wakeup = NULL;
-	wdev->poll_irq = true;
 
 	wdev->bh_wq = alloc_workqueue("wfx_bh_wq", WQ_HIGHPRI | WQ_PERCPU, 0);
 	if (!wdev->bh_wq)
@@ -374,12 +373,19 @@ int wfx_probe(struct wfx_dev *wdev)
 	if (err)
 		goto bh_unregister;
 
-	wfx_bh_poll_irq(wdev);
+	if (wdev->poll_irq) {
+		wfx_bh_poll_irq(wdev);
+	} else {
+		err = wdev->hwbus_ops->irq_subscribe(wdev->hwbus_priv);
+		if (err)
+			goto bh_unregister;
+	}
+
 	err = wait_for_completion_timeout(&wdev->firmware_ready, 1 * HZ);
 	if (err == 0) {
 		dev_err(wdev->dev, "timeout while waiting for startup indication\n");
 		err = -ETIMEDOUT;
-		goto bh_unregister;
+		goto irq_unsubscribe;
 	}
 
 	/* FIXME: fill wiphy::hw_version */
@@ -399,12 +405,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) {
@@ -420,12 +426,14 @@ int wfx_probe(struct wfx_dev *wdev)
 	dev_dbg(wdev->dev, "sending configuration file %s\n", wdev->pdata.file_pds);
 	err = wfx_send_pdata_pds(wdev);
 	if (err < 0 && err != -ENOENT)
-		goto bh_unregister;
+		goto irq_unsubscribe;
 
-	wdev->poll_irq = false;
-	err = wdev->hwbus_ops->irq_subscribe(wdev->hwbus_priv);
-	if (err)
-		goto bh_unregister;
+	if (wdev->poll_irq) {
+		wdev->poll_irq = false;
+		err = wdev->hwbus_ops->irq_subscribe(wdev->hwbus_priv);
+		if (err)
+			goto bh_unregister;
+	}
 
 	err = wfx_hif_use_multi_tx_conf(wdev, true);
 	if (err)
@@ -474,7 +482,8 @@ int wfx_probe(struct wfx_dev *wdev)
 ieee80211_unregister:
 	ieee80211_unregister_hw(wdev->hw);
 irq_unsubscribe:
-	wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv);
+	if (!wdev->poll_irq)
+		wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv);
 bh_unregister:
 	wfx_bh_unregister(wdev);
 	destroy_workqueue(wdev->bh_wq);
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH wireless-next 2/2] dt-bindings: net: wireless: wfx: discourage OOB IRQ with SDIO
  2026-08-31 16:40 [PATCH wireless-next 0/2] wifi: wfx: fix possible device hang during init Jérôme Pouiller
  2026-08-31 16:40 ` [PATCH wireless-next 1/2] " Jérôme Pouiller
@ 2026-08-31 16:40 ` Jérôme Pouiller
  1 sibling, 0 replies; 5+ messages in thread
From: Jérôme Pouiller @ 2026-08-31 16:40 UTC (permalink / raw)
  To: linux-wireless, devicetree
  Cc: linux-kernel, linux-devel, Johannes Berg, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Lukas Stockmann,
	Gerard Salvatella, Jérôme Pouiller

The WFxxx device provides an SDIO interface. In addition to the usual
in-band SDIO IRQ, the device also supports an out-of-band IRQ line.

However, using the OOB IRQ with SDIO requires to poll the control
register during the device initialization. This polling is unreliable:
if an IRQ is raised while the host reads the control register, the
device is lost and only a reset allows to recover it.

There is no way to fix this in software. So, warn the user that this
setup should be reserved to debug.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml b/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
index 6c0888ae4c4e1..60dfe257eadc2 100644
--- a/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
@@ -50,6 +50,9 @@ properties:
       used, this property is required. When SDIO is used, the "in-band"
       interrupt provided by the SDIO bus is used unless an interrupt is defined
       in the Device Tree.
+
+      The hardware suffers from a limitation when this property is used with
+      SDIO. OOB IRQ with SDIO should only be used for debug.
     maxItems: 1
 
   reset-gpios:
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH wireless-next 1/2] wifi: wfx: fix possible device hang during init
  2026-08-31 16:40 ` [PATCH wireless-next 1/2] " Jérôme Pouiller
@ 2026-08-31 19:23   ` sashiko-bot
  2026-08-31 19:35     ` Jérôme Pouiller
  0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-08-31 19:23 UTC (permalink / raw)
  To: Jérôme Pouiller; +Cc: conor+dt, devicetree, robh

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missed firmware startup interrupt on SPI platforms due to late IRQ subscription.
--

commit b2aaae6022fa921df221babdf79ea3e5f12e766f
Author: Jérôme Pouiller <jerome.pouiller@silabs.com>

wifi: wfx: fix possible device hang during init

This patch modifies the device initialization sequence to limit data
polling to out-of-band IRQ setups. It attempts to prevent a hardware lockup
that occurs if an IRQ fires while the host is reading the control register.

> diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
> index dda36e41eed13..da30e8f481ada 100644
> --- a/drivers/net/wireless/silabs/wfx/main.c
> +++ b/drivers/net/wireless/silabs/wfx/main.c

[ ... ]

> @@ -374,12 +373,19 @@ int wfx_probe(struct wfx_dev *wdev)
>  	if (err)
>  		goto bh_unregister;
>  
> -	wfx_bh_poll_irq(wdev);
> +	if (wdev->poll_irq) {
> +		wfx_bh_poll_irq(wdev);
> +	} else {
> +		err = wdev->hwbus_ops->irq_subscribe(wdev->hwbus_priv);
> +		if (err)
> +			goto bh_unregister;
> +	}
> +
>  	err = wait_for_completion_timeout(&wdev->firmware_ready, 1 * HZ);

[Severity: High]
The commit message states that the IRQ is subscribed before the first
exchange with the device. However, looking at wfx_probe(), the code just
above this snippet calls wfx_init_device(), which conducts the first
exchanges and unmasks device-side interrupts.

Since irq_subscribe() is only called after wfx_init_device() finishes, is
there a race window here?

If the device asserts the firmware-ready interrupt during this gap before
the host OS registers the IRQ handler, could the edge be permanently lost
(e.g., on SPI platforms using edge-triggered interrupts)? This seems like it
could cause the subsequent wait_for_completion_timeout() to timeout and 
hang the initialization.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831164056.47683-1-jerome.pouiller@silabs.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH wireless-next 1/2] wifi: wfx: fix possible device hang during init
  2026-08-31 19:23   ` sashiko-bot
@ 2026-08-31 19:35     ` Jérôme Pouiller
  0 siblings, 0 replies; 5+ messages in thread
From: Jérôme Pouiller @ 2026-08-31 19:35 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: conor+dt, devicetree, robh

On Monday 31 August 2026 21:23:41 Central European Summer Time sashiko-bot@kernel.org wrote:
[...]
> 
> commit b2aaae6022fa921df221babdf79ea3e5f12e766f
> Author: Jérôme Pouiller <jerome.pouiller@silabs.com>
> 
> wifi: wfx: fix possible device hang during init
> 
> This patch modifies the device initialization sequence to limit data
> polling to out-of-band IRQ setups. It attempts to prevent a hardware lockup
> that occurs if an IRQ fires while the host is reading the control register.
> 
> > diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
> > index dda36e41eed13..da30e8f481ada 100644
> > --- a/drivers/net/wireless/silabs/wfx/main.c
> > +++ b/drivers/net/wireless/silabs/wfx/main.c
> 
> [ ... ]
> 
> > @@ -374,12 +373,19 @@ int wfx_probe(struct wfx_dev *wdev)
> >       if (err)
> >               goto bh_unregister;
> >
> > -     wfx_bh_poll_irq(wdev);
> > +     if (wdev->poll_irq) {
> > +             wfx_bh_poll_irq(wdev);
> > +     } else {
> > +             err = wdev->hwbus_ops->irq_subscribe(wdev->hwbus_priv);
> > +             if (err)
> > +                     goto bh_unregister;
> > +     }
> > +
> >       err = wait_for_completion_timeout(&wdev->firmware_ready, 1 * HZ);
> 
> [Severity: High]
> The commit message states that the IRQ is subscribed before the first
> exchange with the device. However, looking at wfx_probe(), the code just
> above this snippet calls wfx_init_device(), which conducts the first
> exchanges and unmasks device-side interrupts.
> 
> Since irq_subscribe() is only called after wfx_init_device() finishes, is
> there a race window here?
> 
> If the device asserts the firmware-ready interrupt during this gap before
> the host OS registers the IRQ handler, could the edge be permanently lost
> (e.g., on SPI platforms using edge-triggered interrupts)? This seems like it
> could cause the subsequent wait_for_completion_timeout() to timeout and
> hang the initialization.

hmm... Indeed, maybe. I will sent a v2.



-- 
Jérôme Pouiller



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-31 19:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 16:40 [PATCH wireless-next 0/2] wifi: wfx: fix possible device hang during init Jérôme Pouiller
2026-08-31 16:40 ` [PATCH wireless-next 1/2] " Jérôme Pouiller
2026-08-31 19:23   ` sashiko-bot
2026-08-31 19:35     ` Jérôme Pouiller
2026-08-31 16:40 ` [PATCH wireless-next 2/2] dt-bindings: net: wireless: wfx: discourage OOB IRQ with SDIO Jérôme Pouiller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox