| Message ID | 20220225112405.355599-1-Jerome.Pouiller@silabs.com |
|---|---|
| Headers | show |
| Series | staging: wfx: usual maintenance | expand |
On 2/28/2022 5:12 PM, Joe Perches wrote: > On Fri, 2022-02-25 at 12:23 +0100, Jerome Pouiller wrote: >> From: Jérôme Pouiller <jerome.pouiller@silabs.com> >> >> A few comments were not yet formatted on 100 columns. > > IMO, none of these changes are necessary or good changes. > > 80 columns is preferred. > > Really comments should most always use 80 columns, and > only occasionally should code be more than 80 columns > and almost never should code be more than 100 columns. That was my reaction as well. Just because we've relaxed rules so that we *can* exceed 80 columns, it doesn't mean we *should*, and definitely doesn't mean we should *strive* to do so.
On Monday 2 May 2022 10:16:06 CEST Tetsuo Handa 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. > > > Flushing system-wide workqueues is dangerous and will be forbidden. > Replace system_highpri_wq with per "struct wfx_dev" bh_wq. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > --- > Note: This patch is only compile tested. > > Changes in v2: > Use per "struct wfx_dev" workqueue. > > drivers/net/wireless/silabs/wfx/bh.c | 6 +++--- > drivers/net/wireless/silabs/wfx/hif_tx.c | 2 +- > drivers/net/wireless/silabs/wfx/main.c | 6 ++++++ > drivers/net/wireless/silabs/wfx/wfx.h | 1 + > 4 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/silabs/wfx/bh.c b/drivers/net/wireless/silabs/wfx/bh.c > index bcea9d5b119c..21dfdcf9cc27 100644 > --- a/drivers/net/wireless/silabs/wfx/bh.c > +++ b/drivers/net/wireless/silabs/wfx/bh.c > @@ -267,7 +267,7 @@ void wfx_bh_request_rx(struct wfx_dev *wdev) > wfx_control_reg_read(wdev, &cur); > prev = atomic_xchg(&wdev->hif.ctrl_reg, cur); > complete(&wdev->hif.ctrl_ready); > - queue_work(system_highpri_wq, &wdev->hif.bh); > + queue_work(wdev->bh_wq, &wdev->hif.bh); > > if (!(cur & CTRL_NEXT_LEN_MASK)) > dev_err(wdev->dev, "unexpected control register value: length field is 0: %04x\n", > @@ -280,7 +280,7 @@ void wfx_bh_request_rx(struct wfx_dev *wdev) > /* Driver want to send data */ > void wfx_bh_request_tx(struct wfx_dev *wdev) > { > - queue_work(system_highpri_wq, &wdev->hif.bh); > + queue_work(wdev->bh_wq, &wdev->hif.bh); > } > > /* If IRQ is not available, this function allow to manually poll the control register and simulate > @@ -295,7 +295,7 @@ void wfx_bh_poll_irq(struct wfx_dev *wdev) > u32 reg; > > WARN(!wdev->poll_irq, "unexpected IRQ polling can mask IRQ"); > - flush_workqueue(system_highpri_wq); > + flush_workqueue(wdev->bh_wq); > start = ktime_get(); > for (;;) { > wfx_control_reg_read(wdev, ®); > diff --git a/drivers/net/wireless/silabs/wfx/hif_tx.c b/drivers/net/wireless/silabs/wfx/hif_tx.c > index 9c653d0e9034..d35dd940d968 100644 > --- a/drivers/net/wireless/silabs/wfx/hif_tx.c > +++ b/drivers/net/wireless/silabs/wfx/hif_tx.c > @@ -73,7 +73,7 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct wfx_hif_msg *request, > > if (no_reply) { > /* Chip won't reply. Ensure the wq has send the buffer before to continue. */ > - flush_workqueue(system_highpri_wq); > + flush_workqueue(wdev->bh_wq); > ret = 0; > goto end; > } > diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c > index e575a81ca2ca..e015bfb8d221 100644 > --- a/drivers/net/wireless/silabs/wfx/main.c > +++ b/drivers/net/wireless/silabs/wfx/main.c > @@ -345,6 +345,10 @@ int wfx_probe(struct wfx_dev *wdev) > wdev->pdata.gpio_wakeup = NULL; > wdev->poll_irq = true; > > + wdev->bh_wq = alloc_workqueue("wfx_bh_wq", WQ_HIGHPRI, 0); > + if (!wdev->bh_wq) > + return -ENOMEM; > + > wfx_bh_register(wdev); > > err = wfx_init_device(wdev); > @@ -458,6 +462,7 @@ int wfx_probe(struct wfx_dev *wdev) > wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv); > bh_unregister: > wfx_bh_unregister(wdev); > + destroy_workqueue(wdev->bh_wq); > return err; > } > > @@ -467,6 +472,7 @@ void wfx_release(struct wfx_dev *wdev) > wfx_hif_shutdown(wdev); > wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv); > wfx_bh_unregister(wdev); > + destroy_workqueue(wdev->bh_wq); > } > > static int __init wfx_core_init(void) > diff --git a/drivers/net/wireless/silabs/wfx/wfx.h b/drivers/net/wireless/silabs/wfx/wfx.h > index 6594cc647c2f..6f5e95dae21f 100644 > --- a/drivers/net/wireless/silabs/wfx/wfx.h > +++ b/drivers/net/wireless/silabs/wfx/wfx.h > @@ -57,6 +57,7 @@ struct wfx_dev { > struct mutex rx_stats_lock; > struct wfx_hif_tx_power_loop_info tx_power_loop_info; > struct mutex tx_power_loop_info_lock; > + struct workqueue_struct *bh_wq; > }; > > struct wfx_vif { > -- > 2.34.1 > > > It's right for me. Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
Jérôme Pouiller <jerome.pouiller@silabs.com> writes: > On Monday 2 May 2022 10:16:06 CEST Tetsuo Handa 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. >> >> >> Flushing system-wide workqueues is dangerous and will be forbidden. >> Replace system_highpri_wq with per "struct wfx_dev" bh_wq. >> >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> [...] > It's right for me. > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> Thanks, but I'll change this to Acked-by: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by Also please edit your quotes, otherwise patchwork is hard to use.
From: Jérôme Pouiller <jerome.pouiller@silabs.com> This PR contains fixes for various small defects I have seen these last weeks. Jérôme Pouiller (10): staging: wfx: sta.o was linked twice staging: wfx: fix struct alignment staging: wfx: format comments on 100 columns staging: wfx: format code on 100 columns staging: wfx: remove useless variable staging: wfx: drop useless include staging: wfx: remove duplicated code in wfx_cmd_send() staging: wfx: prefer to wait for an event instead to sleep staging: wfx: ensure HIF request has been sent before polling staging: wfx: flags for SPI IRQ were ignored drivers/staging/wfx/Makefile | 1 - drivers/staging/wfx/bh.c | 1 + drivers/staging/wfx/bus_spi.c | 30 +++++++++++++++--------------- drivers/staging/wfx/data_tx.c | 8 ++------ drivers/staging/wfx/fwio.c | 3 +-- drivers/staging/wfx/hif_api_cmd.h | 2 -- drivers/staging/wfx/hif_rx.c | 3 +-- drivers/staging/wfx/hif_tx.c | 13 ++++++------- drivers/staging/wfx/hif_tx_mib.c | 3 +-- drivers/staging/wfx/hwio.c | 9 +++------ drivers/staging/wfx/queue.c | 9 ++++----- drivers/staging/wfx/sta.c | 11 +++++------ drivers/staging/wfx/sta.h | 3 +-- 13 files changed, 40 insertions(+), 56 deletions(-)