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 Reported-by: Lukas Stockmann Signed-off-by: Jérôme Pouiller --- 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/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..4e99fe7e5bb78 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) @@ -370,16 +369,24 @@ int wfx_probe(struct wfx_dev *wdev) wfx_bh_register(wdev); + if (!wdev->poll_irq) { + err = wdev->hwbus_ops->irq_subscribe(wdev->hwbus_priv); + if (err) + goto bh_unregister; + } + err = wfx_init_device(wdev); if (err) - goto bh_unregister; + goto irq_unsubscribe; + + if (wdev->poll_irq) + wfx_bh_poll_irq(wdev); - wfx_bh_poll_irq(wdev); 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 +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) { @@ -420,12 +427,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) { + err = wdev->hwbus_ops->irq_subscribe(wdev->hwbus_priv); + if (err) + goto bh_unregister; + wdev->poll_irq = false; + } err = wfx_hif_use_multi_tx_conf(wdev, true); if (err) @@ -474,7 +483,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