* [PATCH v2 0/6] staging: WF200 driver fixes
@ 2020-02-11 10:34 Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race Michał Mirosław
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Michał Mirosław @ 2020-02-11 10:34 UTC (permalink / raw)
To: Jérôme Pouiller, Greg Kroah-Hartman; +Cc: devel, linux-kernel
A set of fixes for WF200 WiFi module driver. Three are bug fixes,
all the rest: cleanups and optimizations.
v2:
use devres for fixing init/interrupt race, as suggested by Dan Carpenter
add Fixes: tags dropped by accident
remove 8-bit SPI fall-back patch
Michał Mirosław (6):
staging: wfx: fix init/remove vs IRQ race
staging: wfx: annotate nested gc_list vs tx queue locking
staging: wfx: add proper "compatible" string
staging: wfx: follow compatible = vendor,chip format
staging: wfx: use sleeping gpio accessors
staging: wfx: use more power-efficient sleep for reset
.../bindings/net/wireless/siliabs,wfx.txt | 11 ++---
drivers/staging/wfx/bh.c | 8 ++--
drivers/staging/wfx/bus_sdio.c | 16 +++----
drivers/staging/wfx/bus_spi.c | 45 +++++++++++--------
drivers/staging/wfx/hwio.c | 2 +-
drivers/staging/wfx/main.c | 23 ++++++----
drivers/staging/wfx/main.h | 1 -
drivers/staging/wfx/queue.c | 16 +++----
8 files changed, 66 insertions(+), 56 deletions(-)
--
2.20.1
From 08a65da2e2ce94be8a7d00cc692166404ea3e08d Mon Sep 17 00:00:00 2001
Message-Id: <cover.1581416380.git.mirq-linux@rere.qmqm.pl>
From: =?UTF-8?q?Micha=C5=82=20Miros=C5=82aw?= <mirq-linux@rere.qmqm.pl>
Date: Tue, 11 Feb 2020 11:19:40 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
To: =?UTF-8?B?SsOpcsO0bWU=?= Pouiller <jerome.pouiller@silabs.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: devel@driverdev.osuosl.org,
linux-kernel@vger.kernel.org
Michał Mirosław (7):
staging: wfx: fix init/remove vs IRQ race
staging: wfx: annotate nested gc_list vs tx queue locking
staging: wfx: add proper "compatible" string
staging: wfx: follow compatible = vendor,chip format
staging: wfx: try 16-bit word mode first
staging: wfx: use sleeping gpio accessors
staging: wfx: use more power-efficient sleep for reset
.../bindings/net/wireless/siliabs,wfx.txt | 11 ++--
drivers/staging/wfx/bh.c | 8 +--
drivers/staging/wfx/bus_sdio.c | 16 +++---
drivers/staging/wfx/bus_spi.c | 52 +++++++++++--------
drivers/staging/wfx/hwio.c | 2 +-
drivers/staging/wfx/main.c | 23 ++++----
drivers/staging/wfx/main.h | 1 -
drivers/staging/wfx/queue.c | 16 +++---
8 files changed, 71 insertions(+), 58 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/6] staging: wfx: annotate nested gc_list vs tx queue locking
2020-02-11 10:34 [PATCH v2 0/6] staging: WF200 driver fixes Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race Michał Mirosław
@ 2020-02-11 10:35 ` Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 4/6] staging: wfx: follow compatible = vendor,chip format Michał Mirosław
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Michał Mirosław @ 2020-02-11 10:35 UTC (permalink / raw)
To: Jérôme Pouiller, Greg Kroah-Hartman; +Cc: devel, linux-kernel
Lockdep is complaining about recursive locking, because it can't make
a difference between locked skb_queues. Annotate nested locks and avoid
double bh_disable/enable.
[...]
insmod/815 is trying to acquire lock:
cb7d6418 (&(&list->lock)->rlock){+...}, at: wfx_tx_queues_clear+0xfc/0x198 [wfx]
but task is already holding lock:
cb7d61f4 (&(&list->lock)->rlock){+...}, at: wfx_tx_queues_clear+0xa0/0x198 [wfx]
[...]
Possible unsafe locking scenario:
CPU0
----
lock(&(&list->lock)->rlock);
lock(&(&list->lock)->rlock);
Cc: stable@vger.kernel.org
Fixes: 9bca45f3d692 ("staging: wfx: allow to send 802.11 frames")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/staging/wfx/queue.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 0bcc61feee1d..51d6c55ae91f 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -130,12 +130,12 @@ static void wfx_tx_queue_clear(struct wfx_dev *wdev, struct wfx_queue *queue,
spin_lock_bh(&queue->queue.lock);
while ((item = __skb_dequeue(&queue->queue)) != NULL)
skb_queue_head(gc_list, item);
- spin_lock_bh(&stats->pending.lock);
+ spin_lock_nested(&stats->pending.lock, 1);
for (i = 0; i < ARRAY_SIZE(stats->link_map_cache); ++i) {
stats->link_map_cache[i] -= queue->link_map_cache[i];
queue->link_map_cache[i] = 0;
}
- spin_unlock_bh(&stats->pending.lock);
+ spin_unlock(&stats->pending.lock);
spin_unlock_bh(&queue->queue.lock);
}
@@ -207,9 +207,9 @@ void wfx_tx_queue_put(struct wfx_dev *wdev, struct wfx_queue *queue,
++queue->link_map_cache[tx_priv->link_id];
- spin_lock_bh(&stats->pending.lock);
+ spin_lock_nested(&stats->pending.lock, 1);
++stats->link_map_cache[tx_priv->link_id];
- spin_unlock_bh(&stats->pending.lock);
+ spin_unlock(&stats->pending.lock);
spin_unlock_bh(&queue->queue.lock);
}
@@ -237,11 +237,11 @@ static struct sk_buff *wfx_tx_queue_get(struct wfx_dev *wdev,
__skb_unlink(skb, &queue->queue);
--queue->link_map_cache[tx_priv->link_id];
- spin_lock_bh(&stats->pending.lock);
+ spin_lock_nested(&stats->pending.lock, 1);
__skb_queue_tail(&stats->pending, skb);
if (!--stats->link_map_cache[tx_priv->link_id])
wakeup_stats = true;
- spin_unlock_bh(&stats->pending.lock);
+ spin_unlock(&stats->pending.lock);
}
spin_unlock_bh(&queue->queue.lock);
if (wakeup_stats)
@@ -259,10 +259,10 @@ int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
spin_lock_bh(&queue->queue.lock);
++queue->link_map_cache[tx_priv->link_id];
- spin_lock_bh(&stats->pending.lock);
+ spin_lock_nested(&stats->pending.lock, 1);
++stats->link_map_cache[tx_priv->link_id];
__skb_unlink(skb, &stats->pending);
- spin_unlock_bh(&stats->pending.lock);
+ spin_unlock(&stats->pending.lock);
__skb_queue_tail(&queue->queue, skb);
spin_unlock_bh(&queue->queue.lock);
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race
2020-02-11 10:34 [PATCH v2 0/6] staging: WF200 driver fixes Michał Mirosław
@ 2020-02-11 10:35 ` Michał Mirosław
2020-02-11 11:19 ` Jérôme Pouiller
2020-02-11 10:35 ` [PATCH v2 2/6] staging: wfx: annotate nested gc_list vs tx queue locking Michał Mirosław
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Michał Mirosław @ 2020-02-11 10:35 UTC (permalink / raw)
To: Jérôme Pouiller, Greg Kroah-Hartman; +Cc: devel, linux-kernel
Current code races in init/exit with interrupt handlers. This is noticed
by the warning below. Fix it by using devres for ordering allocations and
IRQ de/registration.
WARNING: CPU: 0 PID: 827 at drivers/staging/wfx/bus_spi.c:142 wfx_spi_irq_handler+0x5c/0x64 [wfx]
race condition in driver init/deinit
Cc: stable@vger.kernel.org
Fixes: 0096214a59a7 ("staging: wfx: add support for I/O access")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
v2: use devres to fix the races
---
drivers/staging/wfx/bus_sdio.c | 15 ++++++---------
drivers/staging/wfx/bus_spi.c | 27 ++++++++++++++-------------
drivers/staging/wfx/main.c | 21 +++++++++++++--------
drivers/staging/wfx/main.h | 1 -
4 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
index f8901164c206..5450bd5e1b5d 100644
--- a/drivers/staging/wfx/bus_sdio.c
+++ b/drivers/staging/wfx/bus_sdio.c
@@ -200,25 +200,23 @@ static int wfx_sdio_probe(struct sdio_func *func,
if (ret)
goto err0;
- ret = wfx_sdio_irq_subscribe(bus);
- if (ret)
- goto err1;
-
bus->core = wfx_init_common(&func->dev, &wfx_sdio_pdata,
&wfx_sdio_hwbus_ops, bus);
if (!bus->core) {
ret = -EIO;
- goto err2;
+ goto err1;
}
+ ret = wfx_sdio_irq_subscribe(bus);
+ if (ret)
+ goto err1;
+
ret = wfx_probe(bus->core);
if (ret)
- goto err3;
+ goto err2;
return 0;
-err3:
- wfx_free_common(bus->core);
err2:
wfx_sdio_irq_unsubscribe(bus);
err1:
@@ -234,7 +232,6 @@ static void wfx_sdio_remove(struct sdio_func *func)
struct wfx_sdio_priv *bus = sdio_get_drvdata(func);
wfx_release(bus->core);
- wfx_free_common(bus->core);
wfx_sdio_irq_unsubscribe(bus);
sdio_claim_host(func);
sdio_disable_func(func);
diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
index 40bc33035de2..605ad74068b7 100644
--- a/drivers/staging/wfx/bus_spi.c
+++ b/drivers/staging/wfx/bus_spi.c
@@ -154,6 +154,11 @@ static void wfx_spi_request_rx(struct work_struct *work)
wfx_bh_request_rx(bus->core);
}
+static void wfx_flush_irq_work(void *w)
+{
+ flush_work(w);
+}
+
static size_t wfx_spi_align_size(void *priv, size_t size)
{
// Most of SPI controllers avoid DMA if buffer size is not 32bit aligned
@@ -207,22 +212,23 @@ static int wfx_spi_probe(struct spi_device *func)
udelay(2000);
}
- ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
- IRQF_TRIGGER_RISING, "wfx", bus);
- if (ret)
- return ret;
-
INIT_WORK(&bus->request_rx, wfx_spi_request_rx);
bus->core = wfx_init_common(&func->dev, &wfx_spi_pdata,
&wfx_spi_hwbus_ops, bus);
if (!bus->core)
return -EIO;
- ret = wfx_probe(bus->core);
+ ret = devm_add_action_or_reset(&func->dev, wfx_flush_irq_work,
+ &bus->request_rx);
if (ret)
- wfx_free_common(bus->core);
+ return ret;
- return ret;
+ ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
+ IRQF_TRIGGER_RISING, "wfx", bus);
+ if (ret)
+ return ret;
+
+ return wfx_probe(bus->core);
}
static int wfx_spi_remove(struct spi_device *func)
@@ -230,11 +236,6 @@ static int wfx_spi_remove(struct spi_device *func)
struct wfx_spi_priv *bus = spi_get_drvdata(func);
wfx_release(bus->core);
- wfx_free_common(bus->core);
- // A few IRQ will be sent during device release. Hopefully, no IRQ
- // should happen after wdev/wvif are released.
- devm_free_irq(&func->dev, func->irq, bus);
- flush_work(&bus->request_rx);
return 0;
}
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 84adad64fc30..76b2ff7fc7fe 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -262,6 +262,16 @@ static int wfx_send_pdata_pds(struct wfx_dev *wdev)
return ret;
}
+static void wfx_free_common(void *data)
+{
+ struct wfx_dev *wdev = data;
+
+ mutex_destroy(&wdev->rx_stats_lock);
+ mutex_destroy(&wdev->conf_mutex);
+ wfx_tx_queues_deinit(wdev);
+ ieee80211_free_hw(wdev->hw);
+}
+
struct wfx_dev *wfx_init_common(struct device *dev,
const struct wfx_platform_data *pdata,
const struct hwbus_ops *hwbus_ops,
@@ -332,17 +342,12 @@ struct wfx_dev *wfx_init_common(struct device *dev,
wfx_init_hif_cmd(&wdev->hif_cmd);
wfx_tx_queues_init(wdev);
+ if (devm_add_action_or_reset(dev, wfx_free_common, wdev))
+ return NULL;
+
return wdev;
}
-void wfx_free_common(struct wfx_dev *wdev)
-{
- mutex_destroy(&wdev->rx_stats_lock);
- mutex_destroy(&wdev->conf_mutex);
- wfx_tx_queues_deinit(wdev);
- ieee80211_free_hw(wdev->hw);
-}
-
int wfx_probe(struct wfx_dev *wdev)
{
int i;
diff --git a/drivers/staging/wfx/main.h b/drivers/staging/wfx/main.h
index 875f8c227803..9c9410072def 100644
--- a/drivers/staging/wfx/main.h
+++ b/drivers/staging/wfx/main.h
@@ -34,7 +34,6 @@ struct wfx_dev *wfx_init_common(struct device *dev,
const struct wfx_platform_data *pdata,
const struct hwbus_ops *hwbus_ops,
void *hwbus_priv);
-void wfx_free_common(struct wfx_dev *wdev);
int wfx_probe(struct wfx_dev *wdev);
void wfx_release(struct wfx_dev *wdev);
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/6] staging: wfx: follow compatible = vendor,chip format
2020-02-11 10:34 [PATCH v2 0/6] staging: WF200 driver fixes Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 2/6] staging: wfx: annotate nested gc_list vs tx queue locking Michał Mirosław
@ 2020-02-11 10:35 ` Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 3/6] staging: wfx: add proper "compatible" string Michał Mirosław
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Michał Mirosław @ 2020-02-11 10:35 UTC (permalink / raw)
To: Jérôme Pouiller, Greg Kroah-Hartman; +Cc: devel, linux-kernel
As for SPI, follow "vendor,chip" format 'compatible' string also for
SDIO bus.
Fixes: 0096214a59a7 ("staging: wfx: add support for I/O access")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
.../devicetree/bindings/net/wireless/siliabs,wfx.txt | 4 ++--
drivers/staging/wfx/bus_sdio.c | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt b/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
index 52f97673da1e..ffec79c14786 100644
--- a/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
+++ b/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
@@ -45,7 +45,7 @@ case. Thus declaring WFxxx chip in device tree is strongly recommended (and may
become mandatory in the future).
Required properties:
- - compatible: Should be "silabs,wfx-sdio"
+ - compatible: Should be "silabs,wf200"
- reg: Should be 1
In addition, it is recommended to declare a mmc-pwrseq on SDIO host above WFx.
@@ -71,7 +71,7 @@ Example:
#size = <0>;
mmc@1 {
- compatible = "silabs,wfx-sdio";
+ compatible = "silabs,wf200";
reg = <1>;
pinctrl-names = "default";
pinctrl-0 = <&wfx_wakeup>;
diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
index 5450bd5e1b5d..dedc3ff58d3e 100644
--- a/drivers/staging/wfx/bus_sdio.c
+++ b/drivers/staging/wfx/bus_sdio.c
@@ -251,6 +251,7 @@ MODULE_DEVICE_TABLE(sdio, wfx_sdio_ids);
#ifdef CONFIG_OF
static const struct of_device_id wfx_sdio_of_match[] = {
{ .compatible = "silabs,wfx-sdio" },
+ { .compatible = "silabs,wf200" },
{ },
};
MODULE_DEVICE_TABLE(of, wfx_sdio_of_match);
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/6] staging: wfx: add proper "compatible" string
2020-02-11 10:34 [PATCH v2 0/6] staging: WF200 driver fixes Michał Mirosław
` (2 preceding siblings ...)
2020-02-11 10:35 ` [PATCH v2 4/6] staging: wfx: follow compatible = vendor,chip format Michał Mirosław
@ 2020-02-11 10:35 ` Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 5/6] staging: wfx: use sleeping gpio accessors Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 6/6] staging: wfx: use more power-efficient sleep for reset Michał Mirosław
5 siblings, 0 replies; 10+ messages in thread
From: Michał Mirosław @ 2020-02-11 10:35 UTC (permalink / raw)
To: Jérôme Pouiller, Greg Kroah-Hartman; +Cc: devel, linux-kernel
Add "compatible" string matching "vendor,chip" template and proper
GPIO flags handling. Keep support for old name and reset polarity
for older devicetrees.
Cc: stable@vger.kernel.org # d3a5bcb4a17f ("gpio: add gpiod_toggle_active_low()")
Cc: stable@vger.kernel.org
Fixes: 0096214a59a7 ("staging: wfx: add support for I/O access")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
.../bindings/net/wireless/siliabs,wfx.txt | 7 ++++---
drivers/staging/wfx/bus_spi.c | 14 ++++++++++----
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt b/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
index 26de6762b942..52f97673da1e 100644
--- a/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
+++ b/drivers/staging/wfx/Documentation/devicetree/bindings/net/wireless/siliabs,wfx.txt
@@ -6,7 +6,7 @@ SPI
You have to declare the WFxxx chip in your device tree.
Required properties:
- - compatible: Should be "silabs,wfx-spi"
+ - compatible: Should be "silabs,wf200"
- reg: Chip select address of device
- spi-max-frequency: Maximum SPI clocking speed of device in Hz
- interrupts-extended: Should contain interrupt line (interrupt-parent +
@@ -15,6 +15,7 @@ Required properties:
Optional properties:
- reset-gpios: phandle of gpio that will be used to reset chip during probe.
Without this property, you may encounter issues with warm boot.
+ (Legacy: when compatible == "silabs,wfx-spi", the gpio is inverted.)
Please consult Documentation/devicetree/bindings/spi/spi-bus.txt for optional
SPI connection related properties,
@@ -23,12 +24,12 @@ Example:
&spi1 {
wfx {
- compatible = "silabs,wfx-spi";
+ compatible = "silabs,wf200";
pinctrl-names = "default";
pinctrl-0 = <&wfx_irq &wfx_gpios>;
interrupts-extended = <&gpio 16 IRQ_TYPE_EDGE_RISING>;
wakeup-gpios = <&gpio 12 GPIO_ACTIVE_HIGH>;
- reset-gpios = <&gpio 13 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
reg = <0>;
spi-max-frequency = <42000000>;
};
diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
index 605ad74068b7..d6a75bd61595 100644
--- a/drivers/staging/wfx/bus_spi.c
+++ b/drivers/staging/wfx/bus_spi.c
@@ -27,6 +27,8 @@ MODULE_PARM_DESC(gpio_reset, "gpio number for reset. -1 for none.");
#define SET_WRITE 0x7FFF /* usage: and operation */
#define SET_READ 0x8000 /* usage: or operation */
+#define WFX_RESET_INVERTED 1
+
static const struct wfx_platform_data wfx_spi_pdata = {
.file_fw = "wfm_wf200",
.file_pds = "wf200.pds",
@@ -206,9 +208,11 @@ static int wfx_spi_probe(struct spi_device *func)
if (!bus->gpio_reset) {
dev_warn(&func->dev, "try to load firmware anyway\n");
} else {
- gpiod_set_value(bus->gpio_reset, 0);
- udelay(100);
+ if (spi_get_device_id(func)->driver_data & WFX_RESET_INVERTED)
+ gpiod_toggle_active_low(bus->gpio_reset);
gpiod_set_value(bus->gpio_reset, 1);
+ udelay(100);
+ gpiod_set_value(bus->gpio_reset, 0);
udelay(2000);
}
@@ -245,14 +249,16 @@ static int wfx_spi_remove(struct spi_device *func)
* stripped.
*/
static const struct spi_device_id wfx_spi_id[] = {
- { "wfx-spi", 0 },
+ { "wfx-spi", WFX_RESET_INVERTED },
+ { "wf200", 0 },
{ },
};
MODULE_DEVICE_TABLE(spi, wfx_spi_id);
#ifdef CONFIG_OF
static const struct of_device_id wfx_spi_of_match[] = {
- { .compatible = "silabs,wfx-spi" },
+ { .compatible = "silabs,wfx-spi", .data = (void *)WFX_RESET_INVERTED },
+ { .compatible = "silabs,wf200" },
{ },
};
MODULE_DEVICE_TABLE(of, wfx_spi_of_match);
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 5/6] staging: wfx: use sleeping gpio accessors
2020-02-11 10:34 [PATCH v2 0/6] staging: WF200 driver fixes Michał Mirosław
` (3 preceding siblings ...)
2020-02-11 10:35 ` [PATCH v2 3/6] staging: wfx: add proper "compatible" string Michał Mirosław
@ 2020-02-11 10:35 ` Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 6/6] staging: wfx: use more power-efficient sleep for reset Michał Mirosław
5 siblings, 0 replies; 10+ messages in thread
From: Michał Mirosław @ 2020-02-11 10:35 UTC (permalink / raw)
To: Jérôme Pouiller, Greg Kroah-Hartman; +Cc: devel, linux-kernel
Driver calls GPIO get/set only from non-atomic context and so can use any
GPIOs.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/staging/wfx/bh.c | 6 +++---
drivers/staging/wfx/bus_spi.c | 4 ++--
drivers/staging/wfx/main.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 983c41d1fe7c..c6319ab7e71a 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -20,10 +20,10 @@ static void device_wakeup(struct wfx_dev *wdev)
{
if (!wdev->pdata.gpio_wakeup)
return;
- if (gpiod_get_value(wdev->pdata.gpio_wakeup))
+ if (gpiod_get_value_cansleep(wdev->pdata.gpio_wakeup))
return;
- gpiod_set_value(wdev->pdata.gpio_wakeup, 1);
+ gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup, 1);
if (wfx_api_older_than(wdev, 1, 4)) {
if (!completion_done(&wdev->hif.ctrl_ready))
udelay(2000);
@@ -45,7 +45,7 @@ static void device_release(struct wfx_dev *wdev)
if (!wdev->pdata.gpio_wakeup)
return;
- gpiod_set_value(wdev->pdata.gpio_wakeup, 0);
+ gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup, 0);
}
static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
index d6a75bd61595..e3cd12592662 100644
--- a/drivers/staging/wfx/bus_spi.c
+++ b/drivers/staging/wfx/bus_spi.c
@@ -210,9 +210,9 @@ static int wfx_spi_probe(struct spi_device *func)
} else {
if (spi_get_device_id(func)->driver_data & WFX_RESET_INVERTED)
gpiod_toggle_active_low(bus->gpio_reset);
- gpiod_set_value(bus->gpio_reset, 1);
+ gpiod_set_value_cansleep(bus->gpio_reset, 1);
udelay(100);
- gpiod_set_value(bus->gpio_reset, 0);
+ gpiod_set_value_cansleep(bus->gpio_reset, 0);
udelay(2000);
}
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 76b2ff7fc7fe..3c4c240229ad 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -425,7 +425,7 @@ int wfx_probe(struct wfx_dev *wdev)
"enable 'quiescent' power mode with gpio %d and PDS file %s\n",
desc_to_gpio(wdev->pdata.gpio_wakeup),
wdev->pdata.file_pds);
- gpiod_set_value(wdev->pdata.gpio_wakeup, 1);
+ gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup, 1);
control_reg_write(wdev, 0);
hif_set_operational_mode(wdev, HIF_OP_POWER_MODE_QUIESCENT);
} else {
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 6/6] staging: wfx: use more power-efficient sleep for reset
2020-02-11 10:34 [PATCH v2 0/6] staging: WF200 driver fixes Michał Mirosław
` (4 preceding siblings ...)
2020-02-11 10:35 ` [PATCH v2 5/6] staging: wfx: use sleeping gpio accessors Michał Mirosław
@ 2020-02-11 10:35 ` Michał Mirosław
5 siblings, 0 replies; 10+ messages in thread
From: Michał Mirosław @ 2020-02-11 10:35 UTC (permalink / raw)
To: Jérôme Pouiller, Greg Kroah-Hartman; +Cc: devel, linux-kernel
Replace udelay() with usleep_range() as all uses are in a sleepable context.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/staging/wfx/bh.c | 2 +-
drivers/staging/wfx/bus_spi.c | 4 ++--
drivers/staging/wfx/hwio.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index c6319ab7e71a..9fcab00a3733 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -26,7 +26,7 @@ static void device_wakeup(struct wfx_dev *wdev)
gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup, 1);
if (wfx_api_older_than(wdev, 1, 4)) {
if (!completion_done(&wdev->hif.ctrl_ready))
- udelay(2000);
+ usleep_range(2000, 2500);
} else {
// completion.h does not provide any function to wait
// completion without consume it (a kind of
diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
index e3cd12592662..61e99b09decb 100644
--- a/drivers/staging/wfx/bus_spi.c
+++ b/drivers/staging/wfx/bus_spi.c
@@ -211,9 +211,9 @@ static int wfx_spi_probe(struct spi_device *func)
if (spi_get_device_id(func)->driver_data & WFX_RESET_INVERTED)
gpiod_toggle_active_low(bus->gpio_reset);
gpiod_set_value_cansleep(bus->gpio_reset, 1);
- udelay(100);
+ usleep_range(100, 150);
gpiod_set_value_cansleep(bus->gpio_reset, 0);
- udelay(2000);
+ usleep_range(2000, 2500);
}
INIT_WORK(&bus->request_rx, wfx_spi_request_rx);
diff --git a/drivers/staging/wfx/hwio.c b/drivers/staging/wfx/hwio.c
index 47e04c59ed93..d3a141d95a0e 100644
--- a/drivers/staging/wfx/hwio.c
+++ b/drivers/staging/wfx/hwio.c
@@ -142,7 +142,7 @@ static int indirect_read(struct wfx_dev *wdev, int reg, u32 addr, void *buf,
goto err;
if (!(cfg & prefetch))
break;
- udelay(200);
+ usleep_range(200, 250);
}
if (i == 20) {
ret = -ETIMEDOUT;
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race
2020-02-11 10:35 ` [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race Michał Mirosław
@ 2020-02-11 11:19 ` Jérôme Pouiller
2020-02-11 12:49 ` Michał Mirosław
0 siblings, 1 reply; 10+ messages in thread
From: Jérôme Pouiller @ 2020-02-11 11:19 UTC (permalink / raw)
To: Michał Mirosław
Cc: Greg Kroah-Hartman, devel@driverdev.osuosl.org,
linux-kernel@vger.kernel.org
On Tuesday 11 February 2020 11:35:01 CET Michał Mirosław wrote:
> Current code races in init/exit with interrupt handlers. This is noticed
> by the warning below. Fix it by using devres for ordering allocations and
> IRQ de/registration.
>
> WARNING: CPU: 0 PID: 827 at drivers/staging/wfx/bus_spi.c:142 wfx_spi_irq_handler+0x5c/0x64 [wfx]
> race condition in driver init/deinit
>
> Cc: stable@vger.kernel.org
> Fixes: 0096214a59a7 ("staging: wfx: add support for I/O access")
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> v2: use devres to fix the races
> ---
> drivers/staging/wfx/bus_sdio.c | 15 ++++++---------
> drivers/staging/wfx/bus_spi.c | 27 ++++++++++++++-------------
> drivers/staging/wfx/main.c | 21 +++++++++++++--------
> drivers/staging/wfx/main.h | 1 -
> 4 files changed, 33 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
> index f8901164c206..5450bd5e1b5d 100644
> --- a/drivers/staging/wfx/bus_sdio.c
> +++ b/drivers/staging/wfx/bus_sdio.c
> @@ -200,25 +200,23 @@ static int wfx_sdio_probe(struct sdio_func *func,
> if (ret)
> goto err0;
>
> - ret = wfx_sdio_irq_subscribe(bus);
> - if (ret)
> - goto err1;
> -
> bus->core = wfx_init_common(&func->dev, &wfx_sdio_pdata,
> &wfx_sdio_hwbus_ops, bus);
> if (!bus->core) {
> ret = -EIO;
> - goto err2;
> + goto err1;
> }
>
> + ret = wfx_sdio_irq_subscribe(bus);
> + if (ret)
> + goto err1;
> +
> ret = wfx_probe(bus->core);
> if (ret)
> - goto err3;
> + goto err2;
>
> return 0;
>
> -err3:
> - wfx_free_common(bus->core);
> err2:
> wfx_sdio_irq_unsubscribe(bus);
> err1:
> @@ -234,7 +232,6 @@ static void wfx_sdio_remove(struct sdio_func *func)
> struct wfx_sdio_priv *bus = sdio_get_drvdata(func);
>
> wfx_release(bus->core);
> - wfx_free_common(bus->core);
> wfx_sdio_irq_unsubscribe(bus);
> sdio_claim_host(func);
> sdio_disable_func(func);
> diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
> index 40bc33035de2..605ad74068b7 100644
> --- a/drivers/staging/wfx/bus_spi.c
> +++ b/drivers/staging/wfx/bus_spi.c
> @@ -154,6 +154,11 @@ static void wfx_spi_request_rx(struct work_struct *work)
> wfx_bh_request_rx(bus->core);
> }
>
> +static void wfx_flush_irq_work(void *w)
> +{
> + flush_work(w);
> +}
> +
> static size_t wfx_spi_align_size(void *priv, size_t size)
> {
> // Most of SPI controllers avoid DMA if buffer size is not 32bit aligned
> @@ -207,22 +212,23 @@ static int wfx_spi_probe(struct spi_device *func)
> udelay(2000);
> }
>
> - ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
> - IRQF_TRIGGER_RISING, "wfx", bus);
> - if (ret)
> - return ret;
> -
> INIT_WORK(&bus->request_rx, wfx_spi_request_rx);
> bus->core = wfx_init_common(&func->dev, &wfx_spi_pdata,
> &wfx_spi_hwbus_ops, bus);
> if (!bus->core)
> return -EIO;
>
> - ret = wfx_probe(bus->core);
> + ret = devm_add_action_or_reset(&func->dev, wfx_flush_irq_work,
> + &bus->request_rx);
> if (ret)
> - wfx_free_common(bus->core);
> + return ret;
>
> - return ret;
> + ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
> + IRQF_TRIGGER_RISING, "wfx", bus);
> + if (ret)
> + return ret;
> +
> + return wfx_probe(bus->core);
> }
>
> static int wfx_spi_remove(struct spi_device *func)
> @@ -230,11 +236,6 @@ static int wfx_spi_remove(struct spi_device *func)
> struct wfx_spi_priv *bus = spi_get_drvdata(func);
>
> wfx_release(bus->core);
> - wfx_free_common(bus->core);
> - // A few IRQ will be sent during device release. Hopefully, no IRQ
> - // should happen after wdev/wvif are released.
> - devm_free_irq(&func->dev, func->irq, bus);
> - flush_work(&bus->request_rx);
> return 0;
> }
>
> diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
> index 84adad64fc30..76b2ff7fc7fe 100644
> --- a/drivers/staging/wfx/main.c
> +++ b/drivers/staging/wfx/main.c
> @@ -262,6 +262,16 @@ static int wfx_send_pdata_pds(struct wfx_dev *wdev)
> return ret;
> }
>
> +static void wfx_free_common(void *data)
> +{
> + struct wfx_dev *wdev = data;
> +
> + mutex_destroy(&wdev->rx_stats_lock);
> + mutex_destroy(&wdev->conf_mutex);
> + wfx_tx_queues_deinit(wdev);
> + ieee80211_free_hw(wdev->hw);
> +}
> +
> struct wfx_dev *wfx_init_common(struct device *dev,
> const struct wfx_platform_data *pdata,
> const struct hwbus_ops *hwbus_ops,
> @@ -332,17 +342,12 @@ struct wfx_dev *wfx_init_common(struct device *dev,
> wfx_init_hif_cmd(&wdev->hif_cmd);
> wfx_tx_queues_init(wdev);
>
> + if (devm_add_action_or_reset(dev, wfx_free_common, wdev))
> + return NULL;
> +
> return wdev;
> }
>
> -void wfx_free_common(struct wfx_dev *wdev)
> -{
> - mutex_destroy(&wdev->rx_stats_lock);
> - mutex_destroy(&wdev->conf_mutex);
> - wfx_tx_queues_deinit(wdev);
> - ieee80211_free_hw(wdev->hw);
> -}
> -
> int wfx_probe(struct wfx_dev *wdev)
> {
> int i;
> diff --git a/drivers/staging/wfx/main.h b/drivers/staging/wfx/main.h
> index 875f8c227803..9c9410072def 100644
> --- a/drivers/staging/wfx/main.h
> +++ b/drivers/staging/wfx/main.h
> @@ -34,7 +34,6 @@ struct wfx_dev *wfx_init_common(struct device *dev,
> const struct wfx_platform_data *pdata,
> const struct hwbus_ops *hwbus_ops,
> void *hwbus_priv);
> -void wfx_free_common(struct wfx_dev *wdev);
>
> int wfx_probe(struct wfx_dev *wdev);
> void wfx_release(struct wfx_dev *wdev);
> --
> 2.20.1
>
Are you sure you can completely drop wfx_free_common()? If you want to
use devres, I think that wfx_flush_irq_work() should call
wfx_free_common(), no?
--
Jérôme Pouiller
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race
2020-02-11 11:19 ` Jérôme Pouiller
@ 2020-02-11 12:49 ` Michał Mirosław
2020-02-11 15:20 ` Jérôme Pouiller
0 siblings, 1 reply; 10+ messages in thread
From: Michał Mirosław @ 2020-02-11 12:49 UTC (permalink / raw)
To: Jérôme Pouiller
Cc: Greg Kroah-Hartman, devel@driverdev.osuosl.org,
linux-kernel@vger.kernel.org
On Tue, Feb 11, 2020 at 11:19:18AM +0000, Jérôme Pouiller wrote:
> On Tuesday 11 February 2020 11:35:01 CET Michał Mirosław wrote:
> > Current code races in init/exit with interrupt handlers. This is noticed
> > by the warning below. Fix it by using devres for ordering allocations and
> > IRQ de/registration.
> >
> > WARNING: CPU: 0 PID: 827 at drivers/staging/wfx/bus_spi.c:142 wfx_spi_irq_handler+0x5c/0x64 [wfx]
> > race condition in driver init/deinit
[...]
> > --- a/drivers/staging/wfx/bus_spi.c
> > +++ b/drivers/staging/wfx/bus_spi.c
> > @@ -154,6 +154,11 @@ static void wfx_spi_request_rx(struct work_struct *work)
> > wfx_bh_request_rx(bus->core);
> > }
> >
> > +static void wfx_flush_irq_work(void *w)
> > +{
> > + flush_work(w);
> > +}
> > +
> > static size_t wfx_spi_align_size(void *priv, size_t size)
> > {
> > // Most of SPI controllers avoid DMA if buffer size is not 32bit aligned
> > @@ -207,22 +212,23 @@ static int wfx_spi_probe(struct spi_device *func)
> > udelay(2000);
> > }
> >
> > - ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
> > - IRQF_TRIGGER_RISING, "wfx", bus);
> > - if (ret)
> > - return ret;
> > -
> > INIT_WORK(&bus->request_rx, wfx_spi_request_rx);
> > bus->core = wfx_init_common(&func->dev, &wfx_spi_pdata,
> > &wfx_spi_hwbus_ops, bus);
> > if (!bus->core)
> > return -EIO;
> >
> > - ret = wfx_probe(bus->core);
> > + ret = devm_add_action_or_reset(&func->dev, wfx_flush_irq_work,
> > + &bus->request_rx);
> > if (ret)
> > - wfx_free_common(bus->core);
> > + return ret;
> >
> > - return ret;
> > + ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
> > + IRQF_TRIGGER_RISING, "wfx", bus);
> > + if (ret)
> > + return ret;
> > +
> > + return wfx_probe(bus->core);
> > }
> >
> > static int wfx_spi_remove(struct spi_device *func)
> > @@ -230,11 +236,6 @@ static int wfx_spi_remove(struct spi_device *func)
> > struct wfx_spi_priv *bus = spi_get_drvdata(func);
> >
> > wfx_release(bus->core);
> > - wfx_free_common(bus->core);
> > - // A few IRQ will be sent during device release. Hopefully, no IRQ
> > - // should happen after wdev/wvif are released.
> > - devm_free_irq(&func->dev, func->irq, bus);
> > - flush_work(&bus->request_rx);
> > return 0;
> > }
> >
> > diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
> > index 84adad64fc30..76b2ff7fc7fe 100644
> > --- a/drivers/staging/wfx/main.c
> > +++ b/drivers/staging/wfx/main.c
> > @@ -262,6 +262,16 @@ static int wfx_send_pdata_pds(struct wfx_dev *wdev)
> > return ret;
> > }
> >
> > +static void wfx_free_common(void *data)
> > +{
> > + struct wfx_dev *wdev = data;
> > +
> > + mutex_destroy(&wdev->rx_stats_lock);
> > + mutex_destroy(&wdev->conf_mutex);
> > + wfx_tx_queues_deinit(wdev);
> > + ieee80211_free_hw(wdev->hw);
> > +}
> > +
> > struct wfx_dev *wfx_init_common(struct device *dev,
> > const struct wfx_platform_data *pdata,
> > const struct hwbus_ops *hwbus_ops,
> > @@ -332,17 +342,12 @@ struct wfx_dev *wfx_init_common(struct device *dev,
> > wfx_init_hif_cmd(&wdev->hif_cmd);
> > wfx_tx_queues_init(wdev);
> >
> > + if (devm_add_action_or_reset(dev, wfx_free_common, wdev))
> > + return NULL;
> > +
> > return wdev;
> > }
> >
> > -void wfx_free_common(struct wfx_dev *wdev)
> > -{
> > - mutex_destroy(&wdev->rx_stats_lock);
> > - mutex_destroy(&wdev->conf_mutex);
> > - wfx_tx_queues_deinit(wdev);
> > - ieee80211_free_hw(wdev->hw);
> > -}
> > -
> > int wfx_probe(struct wfx_dev *wdev)
> > {
> > int i;
> > diff --git a/drivers/staging/wfx/main.h b/drivers/staging/wfx/main.h
> > index 875f8c227803..9c9410072def 100644
> > --- a/drivers/staging/wfx/main.h
> > +++ b/drivers/staging/wfx/main.h
> > @@ -34,7 +34,6 @@ struct wfx_dev *wfx_init_common(struct device *dev,
> > const struct wfx_platform_data *pdata,
> > const struct hwbus_ops *hwbus_ops,
> > void *hwbus_priv);
> > -void wfx_free_common(struct wfx_dev *wdev);
> >
> > int wfx_probe(struct wfx_dev *wdev);
> > void wfx_release(struct wfx_dev *wdev);
> > --
> > 2.20.1
> >
>
> Are you sure you can completely drop wfx_free_common()? If you want to
> use devres, I think that wfx_flush_irq_work() should call
> wfx_free_common(), no?
wfx_free_common() will be called by devres in the right order. That's ensured
by devm_add_action_or_reset() at the end of wfx_init_common().
Best Regards,
Michał Mirosław
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race
2020-02-11 12:49 ` Michał Mirosław
@ 2020-02-11 15:20 ` Jérôme Pouiller
0 siblings, 0 replies; 10+ messages in thread
From: Jérôme Pouiller @ 2020-02-11 15:20 UTC (permalink / raw)
To: Michał Mirosław
Cc: Greg Kroah-Hartman, devel@driverdev.osuosl.org,
linux-kernel@vger.kernel.org
On Tuesday 11 February 2020 13:49:22 CET Michał Mirosław 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.
>
>
> On Tue, Feb 11, 2020 at 11:19:18AM +0000, Jérôme Pouiller wrote:
> > On Tuesday 11 February 2020 11:35:01 CET Michał Mirosław wrote:
> > > Current code races in init/exit with interrupt handlers. This is noticed
> > > by the warning below. Fix it by using devres for ordering allocations and
> > > IRQ de/registration.
> > >
> > > WARNING: CPU: 0 PID: 827 at drivers/staging/wfx/bus_spi.c:142 wfx_spi_irq_handler+0x5c/0x64 [wfx]
> > > race condition in driver init/deinit
> [...]
> > > --- a/drivers/staging/wfx/bus_spi.c
> > > +++ b/drivers/staging/wfx/bus_spi.c
> > > @@ -154,6 +154,11 @@ static void wfx_spi_request_rx(struct work_struct *work)
> > > wfx_bh_request_rx(bus->core);
> > > }
> > >
> > > +static void wfx_flush_irq_work(void *w)
> > > +{
> > > + flush_work(w);
> > > +}
> > > +
> > > static size_t wfx_spi_align_size(void *priv, size_t size)
> > > {
> > > // Most of SPI controllers avoid DMA if buffer size is not 32bit aligned
> > > @@ -207,22 +212,23 @@ static int wfx_spi_probe(struct spi_device *func)
> > > udelay(2000);
> > > }
> > >
> > > - ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
> > > - IRQF_TRIGGER_RISING, "wfx", bus);
> > > - if (ret)
> > > - return ret;
> > > -
> > > INIT_WORK(&bus->request_rx, wfx_spi_request_rx);
> > > bus->core = wfx_init_common(&func->dev, &wfx_spi_pdata,
> > > &wfx_spi_hwbus_ops, bus);
> > > if (!bus->core)
> > > return -EIO;
> > >
> > > - ret = wfx_probe(bus->core);
> > > + ret = devm_add_action_or_reset(&func->dev, wfx_flush_irq_work,
> > > + &bus->request_rx);
> > > if (ret)
> > > - wfx_free_common(bus->core);
> > > + return ret;
> > >
> > > - return ret;
> > > + ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
> > > + IRQF_TRIGGER_RISING, "wfx", bus);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + return wfx_probe(bus->core);
> > > }
> > >
> > > static int wfx_spi_remove(struct spi_device *func)
> > > @@ -230,11 +236,6 @@ static int wfx_spi_remove(struct spi_device *func)
> > > struct wfx_spi_priv *bus = spi_get_drvdata(func);
> > >
> > > wfx_release(bus->core);
> > > - wfx_free_common(bus->core);
> > > - // A few IRQ will be sent during device release. Hopefully, no IRQ
> > > - // should happen after wdev/wvif are released.
> > > - devm_free_irq(&func->dev, func->irq, bus);
> > > - flush_work(&bus->request_rx);
> > > return 0;
> > > }
> > >
> > > diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
> > > index 84adad64fc30..76b2ff7fc7fe 100644
> > > --- a/drivers/staging/wfx/main.c
> > > +++ b/drivers/staging/wfx/main.c
> > > @@ -262,6 +262,16 @@ static int wfx_send_pdata_pds(struct wfx_dev *wdev)
> > > return ret;
> > > }
> > >
> > > +static void wfx_free_common(void *data)
> > > +{
> > > + struct wfx_dev *wdev = data;
> > > +
> > > + mutex_destroy(&wdev->rx_stats_lock);
> > > + mutex_destroy(&wdev->conf_mutex);
> > > + wfx_tx_queues_deinit(wdev);
> > > + ieee80211_free_hw(wdev->hw);
> > > +}
> > > +
> > > struct wfx_dev *wfx_init_common(struct device *dev,
> > > const struct wfx_platform_data *pdata,
> > > const struct hwbus_ops *hwbus_ops,
> > > @@ -332,17 +342,12 @@ struct wfx_dev *wfx_init_common(struct device *dev,
> > > wfx_init_hif_cmd(&wdev->hif_cmd);
> > > wfx_tx_queues_init(wdev);
> > >
> > > + if (devm_add_action_or_reset(dev, wfx_free_common, wdev))
> > > + return NULL;
> > > +
> > > return wdev;
> > > }
> > >
> > > -void wfx_free_common(struct wfx_dev *wdev)
> > > -{
> > > - mutex_destroy(&wdev->rx_stats_lock);
> > > - mutex_destroy(&wdev->conf_mutex);
> > > - wfx_tx_queues_deinit(wdev);
> > > - ieee80211_free_hw(wdev->hw);
> > > -}
> > > -
> > > int wfx_probe(struct wfx_dev *wdev)
> > > {
> > > int i;
> > > diff --git a/drivers/staging/wfx/main.h b/drivers/staging/wfx/main.h
> > > index 875f8c227803..9c9410072def 100644
> > > --- a/drivers/staging/wfx/main.h
> > > +++ b/drivers/staging/wfx/main.h
> > > @@ -34,7 +34,6 @@ struct wfx_dev *wfx_init_common(struct device *dev,
> > > const struct wfx_platform_data *pdata,
> > > const struct hwbus_ops *hwbus_ops,
> > > void *hwbus_priv);
> > > -void wfx_free_common(struct wfx_dev *wdev);
> > >
> > > int wfx_probe(struct wfx_dev *wdev);
> > > void wfx_release(struct wfx_dev *wdev);
> > > --
> > > 2.20.1
> > >
> >
> > Are you sure you can completely drop wfx_free_common()? If you want to
> > use devres, I think that wfx_flush_irq_work() should call
> > wfx_free_common(), no?
>
> wfx_free_common() will be called by devres in the right order. That's ensured
> by devm_add_action_or_reset() at the end of wfx_init_common().
Indeed.
Reviewed-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
--
Jérôme Pouiller
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-02-11 15:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-11 10:34 [PATCH v2 0/6] staging: WF200 driver fixes Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race Michał Mirosław
2020-02-11 11:19 ` Jérôme Pouiller
2020-02-11 12:49 ` Michał Mirosław
2020-02-11 15:20 ` Jérôme Pouiller
2020-02-11 10:35 ` [PATCH v2 2/6] staging: wfx: annotate nested gc_list vs tx queue locking Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 4/6] staging: wfx: follow compatible = vendor,chip format Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 3/6] staging: wfx: add proper "compatible" string Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 5/6] staging: wfx: use sleeping gpio accessors Michał Mirosław
2020-02-11 10:35 ` [PATCH v2 6/6] staging: wfx: use more power-efficient sleep for reset Michał Mirosław
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.