[[PATCH staging] 5/7] staging: wfx: try 16-bit word mode first
WF200 chip works with 16-bit words over SPI, so use that word size if available. This avoids a bounce buffer usage for little-endian hosts. Signed-off-by: Michał Mirosław --- drivers/staging/wfx/bus_spi.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c index 2b108a9fa5ae..c5f78161234d 100644 --- a/drivers/staging/wfx/bus_spi.c +++ b/drivers/staging/wfx/bus_spi.c @@ -175,9 +175,12 @@ static int wfx_spi_probe(struct spi_device *func) struct wfx_spi_priv *bus; int ret; - if (!func->bits_per_word) - func->bits_per_word = 16; + func->bits_per_word = 16; ret = spi_setup(func); + if (ret == -EINVAL) { + func->bits_per_word = 8; + ret = spi_setup(func); + } if (ret) return ret; // Trace below is also displayed by spi_setup() if compiled with DEBUG -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[[PATCH staging] 7/7] staging: wfx: use more power-efficient sleep for reset
Replace udelay() with usleep_range() as all uses are in a sleepable context. Signed-off-by: Michał Mirosław --- 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 634b4e5bb055..14729af2c405 100644 --- a/drivers/staging/wfx/bus_spi.c +++ b/drivers/staging/wfx/bus_spi.c @@ -209,9 +209,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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[[PATCH staging] 6/7] staging: wfx: use sleeping gpio accessors
Driver calls GPIO get/set only from non-atomic context and so can use any GPIOs. Signed-off-by: Michał Mirosław --- 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 c5f78161234d..634b4e5bb055 100644 --- a/drivers/staging/wfx/bus_spi.c +++ b/drivers/staging/wfx/bus_spi.c @@ -208,9 +208,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 84adad64fc30..e8bdeb9aa3a9 100644 --- a/drivers/staging/wfx/main.c +++ b/drivers/staging/wfx/main.c @@ -420,7 +420,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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[[PATCH staging] 2/7] staging: wfx: follow compatible = vendor,chip format
As for SPI, follow "vendor,chip" format 'compatible' string also for SDIO bus. Signed-off-by: Michał Mirosław --- .../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 f8901164c206..29688b324b33 100644 --- a/drivers/staging/wfx/bus_sdio.c +++ b/drivers/staging/wfx/bus_sdio.c @@ -254,6 +254,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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[[PATCH staging] 1/7] staging: wfx: add proper "compatible" string
Add "compatible" string matching "vendor,chip" template and proper GPIO flags handling. Keep support for old name and reset polarity for older devicetrees. Cc: sta...@vger.kernel.org # d3a5bcb4a17f ("gpio: add gpiod_toggle_active_low()") Cc: sta...@vger.kernel.org Signed-off-by: Michał Mirosław --- .../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 = <4200>; }; diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c index 40bc33035de2..3ba705477ca8 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", @@ -201,9 +203,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); } @@ -244,14 +248,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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[[PATCH staging] 3/7] staging: wfx: fix init/remove vs IRQ race
Move interrupt request and free so to avoid following WARN on probe and possible use-after-free on remove. 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: sta...@vger.kernel.org Signed-off-by: Michał Mirosław --- drivers/staging/wfx/bus_sdio.c | 16 drivers/staging/wfx/bus_spi.c | 16 +--- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c index 29688b324b33..2bfaf61e8f62 100644 --- a/drivers/staging/wfx/bus_sdio.c +++ b/drivers/staging/wfx/bus_sdio.c @@ -200,16 +200,16 @@ 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 err1; + } + + ret = wfx_sdio_irq_subscribe(bus); + if (ret) goto err2; - } ret = wfx_probe(bus->core); if (ret) @@ -218,9 +218,9 @@ static int wfx_sdio_probe(struct sdio_func *func, return 0; err3: - wfx_free_common(bus->core); + wfx_sdio_irq_unsubscribe(bus); err2: - wfx_sdio_irq_unsubscribe(bus); + wfx_free_common(bus->core); err1: sdio_claim_host(func); sdio_disable_func(func); @@ -234,8 +234,8 @@ 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); + wfx_free_common(bus->core); sdio_claim_host(func); sdio_disable_func(func); sdio_release_host(func); diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c index 3ba705477ca8..2b108a9fa5ae 100644 --- a/drivers/staging/wfx/bus_spi.c +++ b/drivers/staging/wfx/bus_spi.c @@ -211,20 +211,22 @@ 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 = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler, + IRQF_TRIGGER_RISING, "wfx", bus); + if (ret) + return ret; + ret = wfx_probe(bus->core); - if (ret) + if (ret) { + devm_free_irq(&func->dev, func->irq, bus); wfx_free_common(bus->core); + } return ret; } @@ -234,11 +236,11 @@ 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); + wfx_free_common(bus->core); return 0; } -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[[PATCH staging] 4/7] staging: wfx: annotate nested gc_list vs tx queue locking
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: sta...@vger.kernel.org Signed-off-by: Michał Mirosław --- 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH staging 0/7] WF200 driver fixes
A set of fixes for WF200 WiFi module driver. Three are bug fixes, all the rest: cleanups and optimizations. Michał Mirosław (7): staging: wfx: add proper "compatible" string staging: wfx: follow compatible = vendor,chip format staging: wfx: fix init/remove vs IRQ race staging: wfx: annotate nested gc_list vs tx queue locking 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| 17 drivers/staging/wfx/bus_spi.c | 41 --- drivers/staging/wfx/hwio.c| 2 +- drivers/staging/wfx/main.c| 2 +- drivers/staging/wfx/queue.c | 16 7 files changed, 55 insertions(+), 42 deletions(-) -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [[PATCH staging] 3/7] staging: wfx: fix init/remove vs IRQ race
On Tue, Feb 11, 2020 at 09:46:54AM +0100, Michał Mirosław wrote: > @@ -218,9 +218,9 @@ static int wfx_sdio_probe(struct sdio_func *func, > return 0; > > err3: > - wfx_free_common(bus->core); > + wfx_sdio_irq_unsubscribe(bus); > err2: > - wfx_sdio_irq_unsubscribe(bus); > + wfx_free_common(bus->core); > err1: > sdio_claim_host(func); > sdio_disable_func(func); > @@ -234,8 +234,8 @@ 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); ^ This calls sdio_release_host(func); > + wfx_free_common(bus->core); > sdio_claim_host(func); > sdio_disable_func(func); > sdio_release_host(func); so is this a double free? > diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c > index 3ba705477ca8..2b108a9fa5ae 100644 > --- a/drivers/staging/wfx/bus_spi.c > +++ b/drivers/staging/wfx/bus_spi.c > @@ -211,20 +211,22 @@ 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 = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler, > +IRQF_TRIGGER_RISING, "wfx", bus); > + if (ret) > + return ret; Shouldn't this call wfx_free_common(bus->core); before returning? > + > ret = wfx_probe(bus->core); > - if (ret) > + if (ret) { > + devm_free_irq(&func->dev, func->irq, bus); We shouldn't have to free devm_ resource. > wfx_free_common(bus->core); > + } > > return ret; > } > @@ -234,11 +236,11 @@ 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); Is this devm_ free required? > flush_work(&bus->request_rx); > + wfx_free_common(bus->core); > return 0; > } regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: wilc1000: refactor p2p action frames handling API's
Hi Dan, On 11/02/20 12:21 pm, Dan Carpenter wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On Mon, Feb 10, 2020 at 06:36:01PM +, ajay.kat...@microchip.com wrote: >> + if (sta_ch == WILC_INVALID_CHANNEL) >> + return; >> >> while (index < len) { > > This range checking was there in the original code, but it's not > correct. index and len are in terms of bytes so we know that we can > read one byte from &buf[index] but we are reading a wilc_attr_entry > struct which is larger than a type. The struct is actually flexibly > sized so this should be something like: > > while (index + sizeof(struct wilc_attr_entry) <= len) { > e = (struct wilc_attr_entry *)&buf[index]; > if (index + sizeof(struct wilc_attr_entry) + > le16_to_cpu(e->attr_len) > len) > break; > Agree. I will correct the 'while' loop condition and submit the v2 patch series. Regards, Ajay ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/3] staging: wilc1000: remove use of vendor specific IE for p2p handling
From: Ajay Singh Remove the use of vendor specific IE parameter to decide p2p_GO/p2p_Client roles between two 'wilc' device. Previously p2p group formation between two 'wilc' device make use of vendor IE for roles decision. The role is decided based on the 'go_intent' value. Signed-off-by: Ajay Singh --- v2: No change drivers/staging/wilc1000/cfg80211.c | 72 ++--- drivers/staging/wilc1000/netdev.c | 2 - drivers/staging/wilc1000/netdev.h | 7 --- 3 files changed, 4 insertions(+), 77 deletions(-) diff --git a/drivers/staging/wilc1000/cfg80211.c b/drivers/staging/wilc1000/cfg80211.c index 4863e516ff13..7afbc475b3ea 100644 --- a/drivers/staging/wilc1000/cfg80211.c +++ b/drivers/staging/wilc1000/cfg80211.c @@ -68,7 +68,6 @@ struct wilc_p2p_mgmt_data { }; static const u8 p2p_oui[] = {0x50, 0x6f, 0x9A, 0x09}; -static const u8 p2p_vendor_spec[] = {0xdd, 0x05, 0x00, 0x08, 0x40, 0x03}; static void cfg_scan_result(enum scan_event scan_event, struct wilc_rcvd_net_info *info, void *user_void) @@ -172,9 +171,6 @@ static void cfg_connect_result(enum conn_event conn_disconn_evt, u8 mac_status, } else if (conn_disconn_evt == CONN_DISCONN_EVENT_DISCONN_NOTIF) { u16 reason = 0; - priv->p2p.local_random = 0x01; - priv->p2p.recv_random = 0x00; - priv->p2p.is_wilc_ie = false; eth_zero_addr(priv->associated_bss); wilc_wlan_set_bssid(priv->dev, NULL, WILC_STATION_MODE); @@ -446,9 +442,6 @@ static int disconnect(struct wiphy *wiphy, struct net_device *dev, wilc->sta_ch = WILC_INVALID_CHANNEL; wilc_wlan_set_bssid(priv->dev, NULL, WILC_STATION_MODE); - priv->p2p.local_random = 0x01; - priv->p2p.recv_random = 0x00; - priv->p2p.is_wilc_ie = false; priv->hif_drv->p2p_timeout = 0; ret = wilc_disconnect(vif); @@ -934,9 +927,6 @@ static void wilc_wfi_cfg_parse_rx_action(u8 *buf, u32 len, u8 sta_ch) u8 channel_list_attr_index = 0; while (index < len) { - if (buf[index] == GO_INTENT_ATTR_ID) - buf[index + 3] = (buf[index + 3] & 0x01) | (0x00 << 1); - if (buf[index] == CHANLIST_ATTR_ID) channel_list_attr_index = index; else if (buf[index] == OPERCHAN_ATTR_ID) @@ -956,12 +946,6 @@ static void wilc_wfi_cfg_parse_tx_action(u8 *buf, u32 len, bool oper_ch, u8 channel_list_attr_index = 0; while (index < len) { - if (buf[index] == GO_INTENT_ATTR_ID) { - buf[index + 3] = (buf[index + 3] & 0x01) | (0x0f << 1); - - break; - } - if (buf[index] == CHANLIST_ATTR_ID) channel_list_attr_index = index; else if (buf[index] == OPERCHAN_ATTR_ID) @@ -981,24 +965,6 @@ static void wilc_wfi_cfg_parse_rx_vendor_spec(struct wilc_priv *priv, u8 *buff, struct wilc_vif *vif = netdev_priv(priv->dev); subtype = buff[P2P_PUB_ACTION_SUBTYPE]; - if ((subtype == GO_NEG_REQ || subtype == GO_NEG_RSP) && - !priv->p2p.is_wilc_ie) { - for (i = P2P_PUB_ACTION_SUBTYPE; i < size; i++) { - if (!memcmp(p2p_vendor_spec, &buff[i], 6)) { - priv->p2p.recv_random = buff[i + 6]; - priv->p2p.is_wilc_ie = true; - break; - } - } - } - - if (priv->p2p.local_random <= priv->p2p.recv_random) { - netdev_dbg(vif->ndev, - "PEER WILL BE GO LocaRand=%02x RecvRand %02x\n", - priv->p2p.local_random, priv->p2p.recv_random); - return; - } - if (subtype == GO_NEG_REQ || subtype == GO_NEG_RSP || subtype == P2P_INV_REQ || subtype == P2P_INV_RSP) { for (i = P2P_PUB_ACTION_SUBTYPE + 2; i < size; i++) { @@ -1051,8 +1017,6 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size) return; } if (buff[ACTION_CAT_ID] == PUB_ACTION_ATTR_ID) { - u8 subtype = buff[P2P_PUB_ACTION_SUBTYPE]; - switch (buff[ACTION_SUBTYPE_ID]) { case GAS_INITIAL_REQ: case GAS_INITIAL_RSP: @@ -1063,10 +1027,6 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size) wilc_wfi_cfg_parse_rx_vendor_spec(priv, buff, size); - if ((subtype == GO_NEG_REQ || subtype == GO_NEG_RSP) && - priv->p2p.is_wilc_ie) - size -= 7; - break; default: @@ -1167,17 +1127,8 @@ static void wilc_wfi_cfg_tx_vendor_spec(struct
[PATCH v2 3/3] staging: wilc1000: refactor p2p action frames handling API's
From: Ajay Singh Refactor handling of P2P specific action frames. Make use of 'struct' to handle the P2P frames instead of manipulating using 'buf' pointer. Signed-off-by: Ajay Singh --- v2: corrected 'while' condition by adding 'struct' size as suggested by Dan. drivers/staging/wilc1000/cfg80211.c | 300 1 file changed, 128 insertions(+), 172 deletions(-) diff --git a/drivers/staging/wilc1000/cfg80211.c b/drivers/staging/wilc1000/cfg80211.c index 7afbc475b3ea..d9c7bed2e6fb 100644 --- a/drivers/staging/wilc1000/cfg80211.c +++ b/drivers/staging/wilc1000/cfg80211.c @@ -6,29 +6,17 @@ #include "cfg80211.h" -#define FRAME_TYPE_ID 0 -#define ACTION_CAT_ID 24 -#define ACTION_SUBTYPE_ID 25 -#define P2P_PUB_ACTION_SUBTYPE 30 - -#define ACTION_FRAME 0xd0 -#define GO_INTENT_ATTR_ID 0x04 -#define CHANLIST_ATTR_ID 0x0b -#define OPERCHAN_ATTR_ID 0x11 -#define PUB_ACTION_ATTR_ID 0x04 -#define P2PELEM_ATTR_ID0xdd - #define GO_NEG_REQ 0x00 #define GO_NEG_RSP 0x01 #define GO_NEG_CONF0x02 #define P2P_INV_REQ0x03 #define P2P_INV_RSP0x04 -#define PUBLIC_ACT_VENDORSPEC 0x09 -#define GAS_INITIAL_REQ0x0a -#define GAS_INITIAL_RSP0x0b #define WILC_INVALID_CHANNEL 0 +/* Operation at 2.4 GHz with channels 1-13 */ +#define WILC_WLAN_OPERATING_CLASS_2_4GHZ 0x51 + static const struct ieee80211_txrx_stypes wilc_wfi_cfg80211_mgmt_types[NUM_NL80211_IFTYPES] = { [NL80211_IFTYPE_STATION] = { @@ -67,7 +55,50 @@ struct wilc_p2p_mgmt_data { u8 *buff; }; -static const u8 p2p_oui[] = {0x50, 0x6f, 0x9A, 0x09}; +struct wilc_p2p_pub_act_frame { + u8 category; + u8 action; + u8 oui[3]; + u8 oui_type; + u8 oui_subtype; + u8 dialog_token; + u8 elem[0]; +} __packed; + +struct wilc_vendor_specific_ie { + u8 tag_number; + u8 tag_len; + u8 oui[3]; + u8 oui_type; + u8 attr[0]; +} __packed; + +struct wilc_attr_entry { + u8 attr_type; + __le16 attr_len; + u8 val[0]; +} __packed; + +struct wilc_attr_oper_ch { + u8 attr_type; + __le16 attr_len; + u8 country_code[IEEE80211_COUNTRY_STRING_LEN]; + u8 op_class; + u8 op_channel; +} __packed; + +struct wilc_attr_ch_list { + u8 attr_type; + __le16 attr_len; + u8 country_code[IEEE80211_COUNTRY_STRING_LEN]; + u8 elem[0]; +} __packed; + +struct wilc_ch_list_elem { + u8 op_class; + u8 no_of_channels; + u8 ch_list[0]; +} __packed; static void cfg_scan_result(enum scan_event scan_event, struct wilc_rcvd_net_info *info, void *user_void) @@ -896,87 +927,51 @@ static int flush_pmksa(struct wiphy *wiphy, struct net_device *netdev) return 0; } -static inline void wilc_wfi_cfg_parse_ch_attr(u8 *buf, u8 ch_list_attr_idx, - u8 op_ch_attr_idx, u8 sta_ch) +static inline void wilc_wfi_cfg_parse_ch_attr(u8 *buf, u32 len, u8 sta_ch) { - int i = 0; - int j = 0; - - if (ch_list_attr_idx) { - u8 limit = ch_list_attr_idx + 3 + buf[ch_list_attr_idx + 1]; + struct wilc_attr_entry *e; + struct wilc_attr_ch_list *ch_list; + struct wilc_attr_oper_ch *op_ch; + u32 index = 0; + u8 ch_list_idx = 0; + u8 op_ch_idx = 0; - for (i = ch_list_attr_idx + 3; i < limit; i++) { - if (buf[i] == 0x51) { - for (j = i + 2; j < ((i + 2) + buf[i + 1]); j++) - buf[j] = sta_ch; - break; - } - } - } + if (sta_ch == WILC_INVALID_CHANNEL) + return; - if (op_ch_attr_idx) { - buf[op_ch_attr_idx + 6] = 0x51; - buf[op_ch_attr_idx + 7] = sta_ch; + while (index + sizeof(*e) <= len) { + e = (struct wilc_attr_entry *)&buf[index]; + if (e->attr_type == IEEE80211_P2P_ATTR_CHANNEL_LIST) + ch_list_idx = index; + else if (e->attr_type == IEEE80211_P2P_ATTR_OPER_CHANNEL) + op_ch_idx = index; + if (ch_list_idx && op_ch_idx) + break; + index += le16_to_cpu(e->attr_len) + sizeof(*e); } -} -static void wilc_wfi_cfg_parse_rx_action(u8 *buf, u32 len, u8 sta_ch) -{ - u32 index = 0; - u8 op_channel_attr_index = 0; - u8 channel_list_attr_index = 0; - - while (index < len) { - if (buf[index] == CHANLIST_ATTR_ID) - channel_list_attr_index
[PATCH v2 2/3] staging: wilc1000: directly fetch 'priv' handler from 'vif' instance
From: Ajay Singh The 'priv' handler is already present in 'vif' struct so directly fetch its value from vif handler in wilc_handle_roc_expired(). Signed-off-by: Ajay Singh --- v2: No change drivers/staging/wilc1000/hif.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/wilc1000/hif.c b/drivers/staging/wilc1000/hif.c index 658790bd465b..c8c41c2df4ec 100644 --- a/drivers/staging/wilc1000/hif.c +++ b/drivers/staging/wilc1000/hif.c @@ -861,9 +861,8 @@ static int wilc_handle_roc_expired(struct wilc_vif *vif, u64 cookie) struct wid wid; int result; struct host_if_drv *hif_drv = vif->hif_drv; - struct wilc_priv *priv = wdev_priv(vif->ndev->ieee80211_ptr); - if (priv->p2p_listen_state) { + if (vif->priv.p2p_listen_state) { remain_on_chan_flag = false; wid.id = WID_REMAIN_ON_CHAN; wid.type = WID_STR; -- 2.24.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: qlge: qlge_main.c: fix style issues
This patch fixes "WARNING: Missing a blank line after declarations" generated from checkpatch.pl by adding a blank line after declarations. Signed-off-by: Mohana Datta Yelugoti --- drivers/staging/qlge/qlge_main.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c index 86b9b7314a40..c712e1af90de 100644 --- a/drivers/staging/qlge/qlge_main.c +++ b/drivers/staging/qlge/qlge_main.c @@ -143,6 +143,7 @@ static int ql_sem_trylock(struct ql_adapter *qdev, u32 sem_mask) int ql_sem_spinlock(struct ql_adapter *qdev, u32 sem_mask) { unsigned int wait_count = 30; + do { if (!ql_sem_trylock(qdev, sem_mask)) return 0; @@ -1210,6 +1211,7 @@ static void ql_unmap_send(struct ql_adapter *qdev, struct tx_ring_desc *tx_ring_desc, int mapped) { int i; + for (i = 0; i < mapped; i++) { if (i == 0 || (i == 7 && mapped > 7)) { /* @@ -1290,6 +1292,7 @@ static int ql_map_send(struct ql_adapter *qdev, */ for (frag_idx = 0; frag_idx < frag_cnt; frag_idx++, map_idx++) { skb_frag_t *frag = &skb_shinfo(skb)->frags[frag_idx]; + tbd++; if (frag_idx == 6 && frag_cnt > 7) { /* Let's tack on an sglist. @@ -1649,6 +1652,7 @@ static void ql_process_mac_rx_skb(struct ql_adapter *qdev, (ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_V4)) { /* Unfragmented ipv4 UDP frame. */ struct iphdr *iph = (struct iphdr *) skb->data; + if (!(iph->frag_off & htons(IP_MF|IP_OFFSET))) { skb->ip_summed = CHECKSUM_UNNECESSARY; @@ -1818,6 +1822,7 @@ static struct sk_buff *ql_build_rx_skb(struct ql_adapter *qdev, * eventually be in trouble. */ int size, i = 0; + sbq_desc = qlge_get_curr_buf(&rx_ring->sbq); pci_unmap_single(qdev->pdev, sbq_desc->dma_addr, SMALL_BUF_MAP_SIZE, PCI_DMA_FROMDEVICE); @@ -1936,6 +1941,7 @@ static void ql_process_mac_split_rx_intr(struct ql_adapter *qdev, (ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_V4)) { /* Unfragmented ipv4 UDP frame. */ struct iphdr *iph = (struct iphdr *) skb->data; + if (!(iph->frag_off & htons(IP_MF|IP_OFFSET))) { skb->ip_summed = CHECKSUM_UNNECESSARY; @@ -2391,6 +2397,7 @@ static void qlge_restore_vlan(struct ql_adapter *qdev) static irqreturn_t qlge_msix_rx_isr(int irq, void *dev_id) { struct rx_ring *rx_ring = dev_id; + napi_schedule(&rx_ring->napi); return IRQ_HANDLED; } @@ -2497,6 +2504,7 @@ static int ql_tso(struct sk_buff *skb, struct ob_mac_tso_iocb_req *mac_iocb_ptr) mac_iocb_ptr->flags2 |= OB_MAC_TSO_IOCB_LSO; if (likely(l3_proto == htons(ETH_P_IP))) { struct iphdr *iph = ip_hdr(skb); + iph->check = 0; mac_iocb_ptr->flags1 |= OB_MAC_TSO_IOCB_IP4; tcp_hdr(skb)->check = ~csum_tcpudp_magic(iph->saddr, @@ -2521,6 +2529,7 @@ static void ql_hw_csum_setup(struct sk_buff *skb, int len; struct iphdr *iph = ip_hdr(skb); __sum16 *check; + mac_iocb_ptr->opcode = OPCODE_OB_MAC_TSO_IOCB; mac_iocb_ptr->frame_len = cpu_to_le32((u32) skb->len); mac_iocb_ptr->net_trans_offset = @@ -4265,6 +4274,7 @@ static int qlge_set_mac_address(struct net_device *ndev, void *p) static void qlge_tx_timeout(struct net_device *ndev, unsigned int txqueue) { struct ql_adapter *qdev = netdev_priv(ndev); + ql_queue_asic_error(qdev); } @@ -4273,6 +4283,7 @@ static void ql_asic_reset_work(struct work_struct *work) struct ql_adapter *qdev = container_of(work, struct ql_adapter, asic_reset_work.work); int status; + rtnl_lock(); status = ql_adapter_down(qdev); if (status) @@ -4344,6 +4355,7 @@ static int ql_get_alt_pcie_func(struct ql_adapter *qdev) static int ql_get_board_info(struct ql_adapter *qdev) { int status; + qdev->func = (ql_read32(qdev, STS) & STS_FUNC_ID_MASK) >> STS_FUNC_ID_SHIFT; if (qdev->func > 3) @@ -4652,6 +4664,7 @@ static void qlge_remove(struct pci_dev *pdev) { struct net_device *ndev = pci_get_drvdata(pdev); struct ql_adapter *qdev = netdev_priv(ndev); + del_timer_sync(&qdev->timer); ql_cancel_all_work_sync(qdev); unregister_netdev(ndev); -- 2.17.1 ___ devel mailing list de...@linuxdriverproj
[PATCH v2 3/6] staging: wfx: add proper "compatible" string
Add "compatible" string matching "vendor,chip" template and proper GPIO flags handling. Keep support for old name and reset polarity for older devicetrees. Cc: sta...@vger.kernel.org # d3a5bcb4a17f ("gpio: add gpiod_toggle_active_low()") Cc: sta...@vger.kernel.org Fixes: 0096214a59a7 ("staging: wfx: add support for I/O access") Signed-off-by: Michał Mirosław --- .../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 = <4200>; }; 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/6] staging: WF200 driver fixes
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: From: =?UTF-8?q?Micha=C5=82=20Miros=C5=82aw?= 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 , Greg Kroah-Hartman Cc: de...@driverdev.osuosl.org, linux-ker...@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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [[PATCH staging] 4/7] staging: wfx: annotate nested gc_list vs tx queue locking
On Tuesday 11 February 2020 09:46:55 CET Michał Mirosław wrote: > 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: sta...@vger.kernel.org > Signed-off-by: Michał Mirosław > --- > 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 > Reviewed-by: Jérôme Pouiller -- Jérôme Pouiller ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race
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: sta...@vger.kernel.org Fixes: 0096214a59a7 ("staging: wfx: add support for I/O access") Signed-off-by: Michał Mirosław --- 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)) +
[PATCH v2 2/6] staging: wfx: annotate nested gc_list vs tx queue locking
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: sta...@vger.kernel.org Fixes: 9bca45f3d692 ("staging: wfx: allow to send 802.11 frames") Signed-off-by: Michał Mirosław --- 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/6] staging: wfx: use more power-efficient sleep for reset
Replace udelay() with usleep_range() as all uses are in a sleepable context. Signed-off-by: Michał Mirosław --- 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/6] staging: wfx: use sleeping gpio accessors
Driver calls GPIO get/set only from non-atomic context and so can use any GPIOs. Signed-off-by: Michał Mirosław --- 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/6] staging: wfx: follow compatible = vendor,chip format
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 --- .../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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [[PATCH staging] 3/7] staging: wfx: fix init/remove vs IRQ race
On Tue, Feb 11, 2020 at 12:23:54PM +0300, Dan Carpenter wrote: > On Tue, Feb 11, 2020 at 09:46:54AM +0100, Michał Mirosław wrote: > > @@ -234,8 +234,8 @@ 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); > ^ > > This calls sdio_release_host(func); > > > + wfx_free_common(bus->core); > > sdio_claim_host(func); > > sdio_disable_func(func); > > sdio_release_host(func); > > so is this a double free? This is paired with sdio_claim_host() just above. Best Regards, Michał Mirosław ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [[PATCH staging] 6/7] staging: wfx: use sleeping gpio accessors
On Tuesday 11 February 2020 09:46:55 CET Michał Mirosław wrote: > Driver calls GPIO get/set only from non-atomic context and so can use any > GPIOs. > > Signed-off-by: Michał Mirosław > --- > 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 c5f78161234d..634b4e5bb055 100644 > --- a/drivers/staging/wfx/bus_spi.c > +++ b/drivers/staging/wfx/bus_spi.c > @@ -208,9 +208,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 84adad64fc30..e8bdeb9aa3a9 100644 > --- a/drivers/staging/wfx/main.c > +++ b/drivers/staging/wfx/main.c > @@ -420,7 +420,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 > Reviewed-by: Jérôme Pouiller -- Jérôme Pouiller ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [[PATCH staging] 7/7] staging: wfx: use more power-efficient sleep for reset
On Tuesday 11 February 2020 09:46:56 CET Michał Mirosław wrote: > > Replace udelay() with usleep_range() as all uses are in a sleepable context. > > Signed-off-by: Michał Mirosław > --- > 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 634b4e5bb055..14729af2c405 100644 > --- a/drivers/staging/wfx/bus_spi.c > +++ b/drivers/staging/wfx/bus_spi.c > @@ -209,9 +209,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 > Reviewed-by: Jérôme Pouiller -- Jérôme Pouiller ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Official Winnings Notification
Dear : Winner We are pleased to inform you that as a result of our recent DRAWS held on the 8th of February,2020.You have officially Won $1.700.000,00 USD , Kindly Contact Mrs.Maria Lucento ,for claims with the following details 1. Full Name:2.Gender/Age--- 3.Address-:4.Mobile--- 5.Occupation Kindly Contact Mrs.Maria Lucento Claims Processing Officer: Mrs.Maria Lucento e-Mail: infoclaim...@gmail.com === Sincerely yours, SuperEnalotto / EuroMillion/ PowerBall Lotto ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race
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: sta...@vger.kernel.org > Fixes: 0096214a59a7 ("staging: wfx: add support for I/O access") > Signed-off-by: Michał Mirosław > --- > 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, >
[PATCH RESEND] Staging: remove wusbcore and UWB from the kernel tree.
It's been over 6 months, and no one has noticed that these drivers are deleted, probably because no one actually has this hardware. As no one has volunteered to maintain the code, let's drop it for good. Signed-off-by: Greg Kroah-Hartman --- [resending and deleting the actual diffs as the patch was too big for the lists] MAINTAINERS | 10 - drivers/staging/Kconfig |3 - drivers/staging/Makefile |2 - drivers/staging/uwb/Kconfig | 72 - drivers/staging/uwb/Makefile | 32 - drivers/staging/uwb/TODO |8 - drivers/staging/uwb/address.c | 352 -- drivers/staging/uwb/allocator.c | 374 --- drivers/staging/uwb/beacon.c | 595 drivers/staging/uwb/driver.c | 143 - drivers/staging/uwb/drp-avail.c | 278 -- drivers/staging/uwb/drp-ie.c | 305 -- drivers/staging/uwb/drp.c | 842 - drivers/staging/uwb/est.c | 450 --- drivers/staging/uwb/hwa-rc.c | 929 -- drivers/staging/uwb/i1480/Makefile|2 - drivers/staging/uwb/i1480/dfu/Makefile| 10 - drivers/staging/uwb/i1480/dfu/dfu.c | 198 -- drivers/staging/uwb/i1480/dfu/i1480-dfu.h | 246 -- drivers/staging/uwb/i1480/dfu/mac.c | 496 --- drivers/staging/uwb/i1480/dfu/phy.c | 190 -- drivers/staging/uwb/i1480/dfu/usb.c | 448 --- drivers/staging/uwb/i1480/i1480-est.c | 85 - drivers/staging/uwb/ie-rcv.c | 42 - drivers/staging/uwb/ie.c | 366 --- drivers/staging/uwb/include/debug-cmd.h | 57 - drivers/staging/uwb/include/spec.h| 767 - drivers/staging/uwb/include/umc.h | 192 -- drivers/staging/uwb/include/whci.h| 102 - drivers/staging/uwb/lc-dev.c | 457 --- drivers/staging/uwb/lc-rc.c | 569 drivers/staging/uwb/neh.c | 606 drivers/staging/uwb/pal.c | 128 - drivers/staging/uwb/radio.c | 196 -- drivers/staging/uwb/reset.c | 379 --- drivers/staging/uwb/rsv.c | 1000 -- drivers/staging/uwb/scan.c| 120 - drivers/staging/uwb/umc-bus.c | 211 -- drivers/staging/uwb/umc-dev.c | 94 - drivers/staging/uwb/umc-drv.c | 31 - drivers/staging/uwb/uwb-debug.c | 354 -- drivers/staging/uwb/uwb-internal.h| 366 --- drivers/staging/uwb/uwb.h | 817 - drivers/staging/uwb/uwbd.c| 356 -- drivers/staging/uwb/whc-rc.c | 467 --- drivers/staging/uwb/whci.c| 257 -- .../staging/wusbcore/Documentation/wusb-cbaf | 130 - .../Documentation/wusb-design-overview.rst| 457 --- drivers/staging/wusbcore/Kconfig | 39 - drivers/staging/wusbcore/Makefile | 28 - drivers/staging/wusbcore/TODO |8 - drivers/staging/wusbcore/cbaf.c | 645 drivers/staging/wusbcore/crypto.c | 441 --- drivers/staging/wusbcore/dev-sysfs.c | 124 - drivers/staging/wusbcore/devconnect.c | 1085 -- drivers/staging/wusbcore/host/Kconfig | 28 - drivers/staging/wusbcore/host/Makefile|3 - drivers/staging/wusbcore/host/hwa-hc.c| 875 - drivers/staging/wusbcore/host/whci/Makefile | 14 - drivers/staging/wusbcore/host/whci/asl.c | 376 --- drivers/staging/wusbcore/host/whci/debug.c| 153 - drivers/staging/wusbcore/host/whci/hcd.c | 356 -- drivers/staging/wusbcore/host/whci/hw.c | 93 - drivers/staging/wusbcore/host/whci/init.c | 177 - drivers/staging/wusbcore/host/whci/int.c | 82 - drivers/staging/wusbcore/host/whci/pzl.c | 404 --- drivers/staging/wusbcore/host/whci/qset.c | 831 - drivers/staging/wusbcore/host/whci/whcd.h | 202 -- drivers/staging/wusbcore/host/whci/whci-hc.h | 401 --- drivers/staging/wusbcore/host/whci/wusb.c | 210 -- .../staging/wusbcore/include/association.h| 151 - drivers/staging/wusbcore/include/wusb-wa.h| 304 -- drivers/staging/wusbcore/include/wusb.h | 362 -- drivers/staging/wusbcore/mmc.c| 303 -- drivers/staging/wusbcore/pal.c| 45 - drivers/staging/wusbcore/reservation.c| 110 - drivers/staging/wusbcore/rh.c | 426 --- drivers/staging/wusbcore/security.c | 599 drivers/staging/wusbcore/wa-hc.c | 88 - drivers/staging/wusbcore/wa-hc.h | 467 --- drivers/staging/wusbcore/wa-nep.c | 289 -- drivers/staging/wusbcore/wa-rpipe.
Re: [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race
On Tue, Feb 11, 2020 at 11:19:18AM +, 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
Re: [[PATCH staging] 3/7] staging: wfx: fix init/remove vs IRQ race
On Tue, Feb 11, 2020 at 11:39:31AM +0100, Michał Mirosław wrote: > On Tue, Feb 11, 2020 at 12:23:54PM +0300, Dan Carpenter wrote: > > On Tue, Feb 11, 2020 at 09:46:54AM +0100, Michał Mirosław wrote: > > > @@ -234,8 +234,8 @@ 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); > > ^ > > > > This calls sdio_release_host(func); > > > > > + wfx_free_common(bus->core); > > > sdio_claim_host(func); > > > sdio_disable_func(func); > > > sdio_release_host(func); > > > > so is this a double free? > > This is paired with sdio_claim_host() just above. Ah... I misread wfx_sdio_irq_unsubscribe(), sorry. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH -next] staging: vc04_services: remove set but unused variable 'local_entity_uc'
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function vchiq_use_internal: drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:2346:16: warning: variable local_entity_uc set but not used [-Wunused-but-set-variable] commit bd8aa2850f00 ("staging: vc04_services: Get of even more suspend/resume states") left behind this unused variable. Reported-by: Hulk Robot Signed-off-by: YueHaibing --- drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index c456ced..d30d24d 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2343,7 +2343,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, enum vchiq_status ret = VCHIQ_SUCCESS; char entity[16]; int *entity_uc; - int local_uc, local_entity_uc; + int local_uc; if (!arm_state) goto out; @@ -2367,7 +2367,6 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, write_lock_bh(&arm_state->susp_res_lock); local_uc = ++arm_state->videocore_use_count; - local_entity_uc = ++(*entity_uc); vchiq_log_trace(vchiq_susp_log_level, "%s %s count %d, state count %d", -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/6] staging: rtl8723bs: Fix potential security hole
Hi, [This is an automated email] This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all The bot has tested the following trees: v5.5.2, v5.4.18, v4.19.102, v4.14.170, v4.9.213, v4.4.213. v5.5.2: Build OK! v4.19.102: Build OK! v4.14.170: Build OK! v4.9.213: Failed to apply! Possible dependencies: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver") v4.4.213: Failed to apply! Possible dependencies: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver") NOTE: The patch will not be queued to stable trees until it is upstream. How should we proceed with this patch? -- Thanks, Sasha ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next] staging: vc04_services: remove set but unused variable 'local_entity_uc'
On Tue, Feb 11, 2020 at 09:43:56PM +0800, YueHaibing wrote: > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function > vchiq_use_internal: > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:2346:16: > warning: variable local_entity_uc set but not used > [-Wunused-but-set-variable] > > commit bd8aa2850f00 ("staging: vc04_services: Get of even more suspend/resume > states") > left behind this unused variable. > > Reported-by: Hulk Robot > Signed-off-by: YueHaibing > --- > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > index c456ced..d30d24d 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > @@ -2343,7 +2343,7 @@ vchiq_use_internal(struct vchiq_state *state, struct > vchiq_service *service, > enum vchiq_status ret = VCHIQ_SUCCESS; > char entity[16]; > int *entity_uc; > - int local_uc, local_entity_uc; > + int local_uc; > > if (!arm_state) > goto out; > @@ -2367,7 +2367,6 @@ vchiq_use_internal(struct vchiq_state *state, struct > vchiq_service *service, > > write_lock_bh(&arm_state->susp_res_lock); > local_uc = ++arm_state->videocore_use_count; > - local_entity_uc = ++(*entity_uc); ^^ This ++ is required. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next] staging: vc04_services: remove set but unused variable 'local_entity_uc'
On 2020/2/11 22:24, Dan Carpenter wrote: > On Tue, Feb 11, 2020 at 09:43:56PM +0800, YueHaibing wrote: >> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function >> vchiq_use_internal: >> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:2346:16: >> warning: variable local_entity_uc set but not used >> [-Wunused-but-set-variable] >> >> commit bd8aa2850f00 ("staging: vc04_services: Get of even more >> suspend/resume states") >> left behind this unused variable. >> >> Reported-by: Hulk Robot >> Signed-off-by: YueHaibing >> --- >> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c >> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c >> index c456ced..d30d24d 100644 >> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c >> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c >> @@ -2343,7 +2343,7 @@ vchiq_use_internal(struct vchiq_state *state, struct >> vchiq_service *service, >> enum vchiq_status ret = VCHIQ_SUCCESS; >> char entity[16]; >> int *entity_uc; >> -int local_uc, local_entity_uc; >> +int local_uc; >> >> if (!arm_state) >> goto out; >> @@ -2367,7 +2367,6 @@ vchiq_use_internal(struct vchiq_state *state, struct >> vchiq_service *service, >> >> write_lock_bh(&arm_state->susp_res_lock); >> local_uc = ++arm_state->videocore_use_count; >> -local_entity_uc = ++(*entity_uc); > ^^ > This ++ is required. oops..., Thanks! > > regards, > dan carpenter > > > . > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH -next] staging: wfx: remove set but not used variable 'tx_priv'
drivers/staging/wfx/queue.c: In function wfx_tx_queues_get: drivers/staging/wfx/queue.c:484:28: warning: variable tx_priv set but not used [-Wunused-but-set-variable] commit 2e57865e79cf ("staging: wfx: pspoll_mask make no sense") left behind this unused variable. Reported-by: Hulk Robot Signed-off-by: YueHaibing --- drivers/staging/wfx/queue.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c index 0bcc61f..c73d158 100644 --- a/drivers/staging/wfx/queue.c +++ b/drivers/staging/wfx/queue.c @@ -481,7 +481,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev) struct wfx_queue *vif_queue = NULL; u32 tx_allowed_mask = 0; u32 vif_tx_allowed_mask = 0; - const struct wfx_tx_priv *tx_priv = NULL; struct wfx_vif *wvif; int not_found; int burst; @@ -541,7 +540,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev) skb = wfx_tx_queue_get(wdev, queue, tx_allowed_mask); if (!skb) continue; - tx_priv = wfx_skb_tx_priv(skb); hif = (struct hif_msg *) skb->data; wvif = wdev_to_wvif(wdev, hif->interface); WARN_ON(!wvif); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/6] staging: wfx: fix init/remove vs IRQ race
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 +, 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 *hwb
Re: [PATCH -next] staging: wfx: remove set but not used variable 'tx_priv'
On Tuesday 11 February 2020 15:03:34 CET YueHaibing wrote: > drivers/staging/wfx/queue.c: In function wfx_tx_queues_get: > drivers/staging/wfx/queue.c:484:28: warning: variable tx_priv set but not > used [-Wunused-but-set-variable] > > commit 2e57865e79cf ("staging: wfx: pspoll_mask make no sense") > left behind this unused variable. > > Reported-by: Hulk Robot > Signed-off-by: YueHaibing > --- > drivers/staging/wfx/queue.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c > index 0bcc61f..c73d158 100644 > --- a/drivers/staging/wfx/queue.c > +++ b/drivers/staging/wfx/queue.c > @@ -481,7 +481,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev) > struct wfx_queue *vif_queue = NULL; > u32 tx_allowed_mask = 0; > u32 vif_tx_allowed_mask = 0; > - const struct wfx_tx_priv *tx_priv = NULL; > struct wfx_vif *wvif; > int not_found; > int burst; > @@ -541,7 +540,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev) > skb = wfx_tx_queue_get(wdev, queue, tx_allowed_mask); > if (!skb) > continue; > - tx_priv = wfx_skb_tx_priv(skb); > hif = (struct hif_msg *) skb->data; > wvif = wdev_to_wvif(wdev, hif->interface); > WARN_ON(!wvif); > -- > 2.7.4 Reviewed-by: Jérôme Pouiller -- Jérôme Pouiller ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next] staging: wfx: remove set but not used variable 'tx_priv'
On Tuesday 11 February 2020 16:21:41 CET Jérôme Pouiller wrote: > On Tuesday 11 February 2020 15:03:34 CET YueHaibing wrote: > > drivers/staging/wfx/queue.c: In function wfx_tx_queues_get: > > drivers/staging/wfx/queue.c:484:28: warning: variable tx_priv set but not > > used [-Wunused-but-set-variable] > > > > commit 2e57865e79cf ("staging: wfx: pspoll_mask make no sense") > > left behind this unused variable. > > > > Reported-by: Hulk Robot > > Signed-off-by: YueHaibing Maybe it could make sens to add a Fixes tag with the commit id that introduce the warning? -- Jérôme Pouiller ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 3/3] staging: wilc1000: refactor p2p action frames handling API's
On Tue, Feb 11, 2020 at 09:57:10AM +, ajay.kat...@microchip.com wrote: > From: Ajay Singh > > Refactor handling of P2P specific action frames. Make use of 'struct' to > handle the P2P frames instead of manipulating using 'buf' pointer. > > Signed-off-by: Ajay Singh > --- > v2: corrected 'while' condition by adding 'struct' size as suggested by Dan. This patch doesn't apply to my tree :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 00/19] Renaming some identifiers.
On Tue, Feb 11, 2020 at 12:05:39AM +0530, Pragat Pandya wrote: > This patchset renames following nineteen variables in exfat.h > Fix checkpatch warning: Avoid CamelCase > -Year->year > -Day->day > -Hour->hour > -Minute->minute > -Second->second > -Millisecond->millisecond > -FatType->fat_type > -ClusterSize->cluster_size > -NumClusters->num_clusters > -FreeClusters->free_clusters > -UsedClusters->used_clusters > -Name->name > -ShortName->short_name > -Attr->attr > -NumSubdirs->num_subdirs > -CreateTimestamp->create_timestamp > -ModifyTimestamp->modify_timestamp > -AccessTimestamp->access_timestamp > > v2: > -Correct misplaced quatation character in subject line(s). > -Remove unnecessary '_'(underscore) character in renaming of identifier > MilliSecond. > -Drop commits renaming unused structure members. Not all of these patches applied, so can you please rebase against my testing tree and resend the remaining patches? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: greybus: Replace zero-length array with flexible-array member
The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertenly introduced[3] to the codebase from now on. This issue was found with the help of Coccinelle. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://github.com/KSPP/linux/issues/21 [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") Signed-off-by: Gustavo A. R. Silva --- drivers/staging/greybus/raw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c index 838acbe84ca0..2b301b2aa107 100644 --- a/drivers/staging/greybus/raw.c +++ b/drivers/staging/greybus/raw.c @@ -30,7 +30,7 @@ struct gb_raw { struct raw_data { struct list_head entry; u32 len; - u8 data[0]; + u8 data[]; }; static struct class *raw_class; -- 2.25.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: unisys: visorinput: Replace zero-length array with flexible-array member
The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertenly introduced[3] to the codebase from now on. This issue was found with the help of Coccinelle. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://github.com/KSPP/linux/issues/21 [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") Signed-off-by: Gustavo A. R. Silva --- drivers/staging/unisys/visorinput/visorinput.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c index 9693fb559052..6d202cba8575 100644 --- a/drivers/staging/unisys/visorinput/visorinput.c +++ b/drivers/staging/unisys/visorinput/visorinput.c @@ -111,7 +111,7 @@ struct visorinput_devdata { /* size of following array */ unsigned int keycode_table_bytes; /* for keyboard devices: visorkbd_keycode[] + visorkbd_ext_keycode[] */ - unsigned char keycode_table[0]; + unsigned char keycode_table[]; }; static const guid_t visor_keyboard_channel_guid = VISOR_KEYBOARD_CHANNEL_GUID; -- 2.25.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [greybus-dev] [PATCH] staging: greybus: Replace zero-length array with flexible-array member
On 2/11/20 3:12 PM, Gustavo A. R. Silva wrote: > The current codebase makes use of the zero-length array language > extension to the C90 standard, but the preferred mechanism to declare > variable-length types such as these ones is a flexible array member[1][2], > introduced in C99: > > struct foo { > int stuff; > struct boo array[]; > }; > > By making use of the mechanism above, we will get a compiler warning > in case the flexible array does not occur last in the structure, which > will help us prevent some kind of undefined behavior bugs from being > inadvertenly introduced[3] to the codebase from now on. > > This issue was found with the help of Coccinelle. > > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html > [2] https://github.com/KSPP/linux/issues/21 > [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") > > Signed-off-by: Gustavo A. R. Silva > --- > drivers/staging/greybus/raw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c > index 838acbe84ca0..2b301b2aa107 100644 > --- a/drivers/staging/greybus/raw.c > +++ b/drivers/staging/greybus/raw.c > @@ -30,7 +30,7 @@ struct gb_raw { > struct raw_data { > struct list_head entry; > u32 len; > - u8 data[0]; > + u8 data[]; > }; > > static struct class *raw_class; > Does the kamlloc() call in receive_data() have any problems with the sizeof(*raw_data) passed as its argument? I'm not entirely sure what sizeof(struct-with-flexible-array-member) produces. -Alex ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [greybus-dev] [PATCH] staging: greybus: Replace zero-length array with flexible-array member
On 2/11/20 16:15, Alex Elder wrote: > On 2/11/20 3:12 PM, Gustavo A. R. Silva wrote: >> The current codebase makes use of the zero-length array language >> extension to the C90 standard, but the preferred mechanism to declare >> variable-length types such as these ones is a flexible array member[1][2], >> introduced in C99: >> >> struct foo { >> int stuff; >> struct boo array[]; >> }; >> >> By making use of the mechanism above, we will get a compiler warning >> in case the flexible array does not occur last in the structure, which >> will help us prevent some kind of undefined behavior bugs from being >> inadvertenly introduced[3] to the codebase from now on. >> >> This issue was found with the help of Coccinelle. >> >> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html >> [2] https://github.com/KSPP/linux/issues/21 >> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") >> >> Signed-off-by: Gustavo A. R. Silva >> --- >> drivers/staging/greybus/raw.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c >> index 838acbe84ca0..2b301b2aa107 100644 >> --- a/drivers/staging/greybus/raw.c >> +++ b/drivers/staging/greybus/raw.c >> @@ -30,7 +30,7 @@ struct gb_raw { >> struct raw_data { >> struct list_head entry; >> u32 len; >> -u8 data[0]; >> +u8 data[]; >> }; >> >> static struct class *raw_class; >> > > Does the kamlloc() call in receive_data() have any problems > with the sizeof(*raw_data) passed as its argument? > Not in this case. It'd be different with a one-element array (u8 data[1]), though. > I'm not entirely sure what sizeof(struct-with-flexible-array-member) > produces. > The same as sizeof(struct-with-zero-length-array): "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html -- Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [greybus-dev] [PATCH] staging: greybus: Replace zero-length array with flexible-array member
On 2/11/20 4:47 PM, Gustavo A. R. Silva wrote: > > > On 2/11/20 16:15, Alex Elder wrote: >> On 2/11/20 3:12 PM, Gustavo A. R. Silva wrote: >>> The current codebase makes use of the zero-length array language >>> extension to the C90 standard, but the preferred mechanism to declare >>> variable-length types such as these ones is a flexible array member[1][2], >>> introduced in C99: >>> >>> struct foo { >>> int stuff; >>> struct boo array[]; >>> }; >>> >>> By making use of the mechanism above, we will get a compiler warning >>> in case the flexible array does not occur last in the structure, which >>> will help us prevent some kind of undefined behavior bugs from being >>> inadvertenly introduced[3] to the codebase from now on. >>> >>> This issue was found with the help of Coccinelle. >>> >>> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html >>> [2] https://github.com/KSPP/linux/issues/21 >>> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") >>> >>> Signed-off-by: Gustavo A. R. Silva >>> --- >>> drivers/staging/greybus/raw.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c >>> index 838acbe84ca0..2b301b2aa107 100644 >>> --- a/drivers/staging/greybus/raw.c >>> +++ b/drivers/staging/greybus/raw.c >>> @@ -30,7 +30,7 @@ struct gb_raw { >>> struct raw_data { >>> struct list_head entry; >>> u32 len; >>> - u8 data[0]; >>> + u8 data[]; >>> }; >>> >>> static struct class *raw_class; >>> >> >> Does the kamlloc() call in receive_data() have any problems >> with the sizeof(*raw_data) passed as its argument? >> > > Not in this case. It'd be different with a one-element array (u8 data[1]), > though. > >> I'm not entirely sure what sizeof(struct-with-flexible-array-member) >> produces. >> > > The same as sizeof(struct-with-zero-length-array): > > "Flexible array members have incomplete type, and so the sizeof operator > may not be applied. As a quirk of the original implementation of > zero-length arrays, sizeof evaluates to zero."[1] > > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html I saw that, but I wondered what the standard says (and whether Clang produces the same result). I found this in a draft standard, and I guess we can assume it applies here: "...the size of the structure is as if the flexible array member were omitted except that it may have more trailing padding than the omission would imply." Looks good to me. Reviewed-by: Alex Elder > > -- > Gustavo > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/22] staging: exfat: Rename variable "MilliSecond" to "milli_second"
On Mon, 2020-01-27 at 14:55 +0300, Dan Carpenter wrote: > On Mon, Jan 27, 2020 at 03:43:28PM +0530, Pragat Pandya wrote: > > Change all the occurrences of "MilliSecond" to "milli_second" in exfat. > > > > Signed-off-by: Pragat Pandya > > --- > > drivers/staging/exfat/exfat.h | 2 +- > > drivers/staging/exfat/exfat_super.c | 16 > > 2 files changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/staging/exfat/exfat.h b/drivers/staging/exfat/exfat.h > > index 85fbea44219a..5c207d715f44 100644 > > --- a/drivers/staging/exfat/exfat.h > > +++ b/drivers/staging/exfat/exfat.h > > @@ -228,7 +228,7 @@ struct date_time_t { > > u16 hour; > > u16 minute; > > u16 second; > > - u16 MilliSecond; > > + u16 milli_second; > > Normally we would just call it "ms". msec is a bit more common. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[driver-core:debugfs_remove_return_value] BUILD SUCCESS 898362bb172007ce38474fd471a4ad32356140b3
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git debugfs_remove_return_value branch HEAD: 898362bb172007ce38474fd471a4ad32356140b3 debugfs: remove return value of debugfs_create_regset32() elapsed time: 2883m configs tested: 258 configs skipped: 0 The following configs have been built successfully. More configs may be tested in the coming days. arm allmodconfig arm allnoconfig arm allyesconfig arm at91_dt_defconfig arm efm32_defconfig arm exynos_defconfig armmulti_v5_defconfig armmulti_v7_defconfig armshmobile_defconfig arm sunxi_defconfig arm64allmodconfig arm64 allnoconfig arm64allyesconfig arm64 defconfig sparcallyesconfig cskydefconfig s390defconfig s390 debug_defconfig sh allmodconfig mips fuloong2e_defconfig s390 zfcpdump_defconfig nds32 defconfig alpha defconfig sh rsk7269_defconfig ia64defconfig ia64 allyesconfig xtensa iss_defconfig riscv allnoconfig mips malta_kvm_defconfig s390 allyesconfig xtensa common_defconfig microblaze mmu_defconfig riscv defconfig powerpc defconfig riscvallyesconfig m68k multi_defconfig h8300 edosk2674_defconfig h8300 h8s-sim_defconfig sparc64 allmodconfig riscvnommu_virt_defconfig s390 alldefconfig h8300h8300h-sim_defconfig mips allnoconfig mips allyesconfig ia64 allnoconfig sparc64 defconfig powerpc allnoconfig i386 alldefconfig i386 allnoconfig i386 allyesconfig i386defconfig ia64 alldefconfig ia64 allmodconfig c6x allyesconfig c6xevmc6678_defconfig nios2 10m50_defconfig nios2 3c120_defconfig openriscor1ksim_defconfig openrisc simple_smp_defconfig nds32 allnoconfig m68k allmodconfig m68k m5475evb_defconfig m68k sun3_defconfig arc allyesconfig arc defconfig microblazenommu_defconfig powerpc ppc64_defconfig powerpc rhel-kconfig mips 32r2_defconfig mips 64r6el_defconfig mips allmodconfig pariscallnoconfig parisc allyesconfig parisc b180_defconfig pariscc3000_defconfig parisc defconfig x86_64 randconfig-a001-20200212 x86_64 randconfig-a002-20200212 x86_64 randconfig-a003-20200212 i386 randconfig-a001-20200212 i386 randconfig-a002-20200212 i386 randconfig-a003-20200212 x86_64 randconfig-a001-20200211 x86_64 randconfig-a002-20200211 x86_64 randconfig-a003-20200211 i386 randconfig-a001-20200211 i386 randconfig-a002-20200211 i386 randconfig-a003-20200211 alpharandconfig-a001-20200210 m68k randconfig-a001-20200210 mips randconfig-a001-20200210 nds32randconfig-a001-20200210 parisc randconfig-a001-20200210 riscvrandconfig-a001-20200210 alpharandconfig-a001-20200211 m68k randconfig-a001-20200211 mips randconfig-a001-20200211 nds32randconfig-a001-20200211 parisc randconfig-a001-20200211 riscvrandconfig-a001-20200211 c6x
[driver-core:debugfs_cleanup] BUILD SUCCESS 8d3008b80ad7f4048eef7fe2b27215f56a5d11db
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git debugfs_cleanup branch HEAD: 8d3008b80ad7f4048eef7fe2b27215f56a5d11db powerpc: powernv: no need to check return value of debugfs_create functions elapsed time: 2884m configs tested: 216 configs skipped: 0 The following configs have been built successfully. More configs may be tested in the coming days. arm allmodconfig arm allnoconfig arm allyesconfig arm at91_dt_defconfig arm efm32_defconfig arm exynos_defconfig armmulti_v5_defconfig armmulti_v7_defconfig armshmobile_defconfig arm sunxi_defconfig arm64allmodconfig arm64 allnoconfig arm64allyesconfig arm64 defconfig sparcallyesconfig cskydefconfig s390defconfig s390 debug_defconfig mips fuloong2e_defconfig sh rsk7269_defconfig i386defconfig shallnoconfig ia64defconfig xtensa iss_defconfig riscv allnoconfig um x86_64_defconfig nds32 defconfig m68k multi_defconfig h8300 edosk2674_defconfig m68k sun3_defconfig sparc64 allmodconfig alpha defconfig riscvnommu_virt_defconfig h8300h8300h-sim_defconfig mips allnoconfig mips allyesconfig i386 alldefconfig i386 allnoconfig i386 allyesconfig ia64 alldefconfig ia64 allmodconfig ia64 allnoconfig ia64 allyesconfig c6x allyesconfig c6xevmc6678_defconfig nios2 10m50_defconfig nios2 3c120_defconfig openriscor1ksim_defconfig openrisc simple_smp_defconfig xtensa common_defconfig nds32 allnoconfig h8300 h8s-sim_defconfig m68k allmodconfig m68k m5475evb_defconfig arc allyesconfig microblazenommu_defconfig microblaze mmu_defconfig arc defconfig powerpc allnoconfig powerpc defconfig powerpc ppc64_defconfig powerpc rhel-kconfig mips 32r2_defconfig mips 64r6el_defconfig mips allmodconfig mips malta_kvm_defconfig pariscallnoconfig parisc allyesconfig parisc b180_defconfig pariscc3000_defconfig parisc defconfig x86_64 randconfig-a001-20200212 x86_64 randconfig-a002-20200212 x86_64 randconfig-a003-20200212 i386 randconfig-a001-20200212 i386 randconfig-a002-20200212 i386 randconfig-a003-20200212 riscvrandconfig-a001-20200210 parisc randconfig-a001-20200210 m68k randconfig-a001-20200210 nds32randconfig-a001-20200210 mips randconfig-a001-20200210 alpharandconfig-a001-20200210 h8300randconfig-a001-20200210 nios2randconfig-a001-20200210 microblaze randconfig-a001-20200210 sparc64 randconfig-a001-20200210 c6x randconfig-a001-20200210 c6x randconfig-a001-20200211 h8300randconfig-a001-20200211 microblaze randconfig-a001-20200211 nios2randconfig-a001-20200211 sparc64 randconfig-a001-20200211 csky randconfig-a001-20200211 openrisc randconfig-a001-20200211 s390 randconfig-a001-20200211 sh randconfig-a001-20200211 xtensa randconfig-a001-20200211 csky randconfig-a001-20200212 openrisc randconfig-a001-20200212
REPLY
Warren Buffett Foundation picked you for a $1 500,000 donation. For more details reply ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: Replace zero-length array with flexible-array member
On Tue, Feb 11, 2020 at 03:12:19PM -0600, Gustavo A. R. Silva wrote: > The current codebase makes use of the zero-length array language > extension to the C90 standard, but the preferred mechanism to declare > variable-length types such as these ones is a flexible array member[1][2], > introduced in C99: > > struct foo { > int stuff; > struct boo array[]; > }; > > By making use of the mechanism above, we will get a compiler warning > in case the flexible array does not occur last in the structure, which > will help us prevent some kind of undefined behavior bugs from being > inadvertenly introduced[3] to the codebase from now on. > > This issue was found with the help of Coccinelle. Looks like the scripts may need to be updated. You missed at least a couple of instances: $ git grep '\[0\];' drivers/staging/greybus/ drivers/staging/greybus/audio_apbridgea.h: __u8data[0]; ... drivers/staging/greybus/usb.c: u8 buf[0]; Would you mind replacing these as well so that we really do this in one patch per subsystem? > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html > [2] https://github.com/KSPP/linux/issues/21 > [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") > > Signed-off-by: Gustavo A. R. Silva > --- > drivers/staging/greybus/raw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/greybus/raw.c b/drivers/staging/greybus/raw.c > index 838acbe84ca0..2b301b2aa107 100644 > --- a/drivers/staging/greybus/raw.c > +++ b/drivers/staging/greybus/raw.c > @@ -30,7 +30,7 @@ struct gb_raw { > struct raw_data { > struct list_head entry; > u32 len; > - u8 data[0]; > + u8 data[]; > }; > > static struct class *raw_class; Thanks, Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel