Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

staging: wfx: drop async field from struct hif_cmd

The parameter "async" in wfx_cmd_send() allows to send command without
waiting for the reply. In this case, the mutex hif_cmd.lock is released
asynchronously in the context of the receiver workqueue.

However, "kbuild test robot" complains about this architecture[1] since
it is not able to follow the lock duration of hif_cmd.lock (and indeed,
the state of the driver if the hardware wouldn't reply is not well
defined).

Besides, this feature is not really necessary. It is only used by
hif_shutdown(). This function hijack the 'async' flag to run a command
that won't answer.

So, this patch removes the 'async' flag and introduces a 'no_reply' flag.
Thus, the mutex hif_cmd.lock is only acquired/released from
hif_cmd_send(). Therefore:
- hif_shutdown() does not have to touch the private data of the struct
hif_cmd
- Kbuild test robot should be happy
- the resulting code is simpler

[1] https://lore.kernel.org/driverdev-devel/alpine.DEB.2.21.1910041317381.2992@hadrien/

Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
Link: https://lore.kernel.org/r/20200907101521.66082-31-Jerome.Pouiller@silabs.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Jérôme Pouiller and committed by
Greg Kroah-Hartman
3768c74b c8fb8809

+10 -22
+1 -6
drivers/staging/wfx/hif_rx.c
··· 47 47 } 48 48 wdev->hif_cmd.ret = status; 49 49 50 - if (!wdev->hif_cmd.async) { 51 - complete(&wdev->hif_cmd.done); 52 - } else { 53 - wdev->hif_cmd.buf_send = NULL; 54 - mutex_unlock(&wdev->hif_cmd.lock); 55 - } 50 + complete(&wdev->hif_cmd.done); 56 51 return status; 57 52 } 58 53
+9 -15
drivers/staging/wfx/hif_tx.c
··· 47 47 } 48 48 49 49 int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request, 50 - void *reply, size_t reply_len, bool async) 50 + void *reply, size_t reply_len, bool no_reply) 51 51 { 52 52 const char *mib_name = ""; 53 53 const char *mib_sep = ""; 54 54 int cmd = request->id; 55 55 int vif = request->interface; 56 56 int ret; 57 - 58 - WARN(wdev->hif_cmd.buf_recv && wdev->hif_cmd.async, "API usage error"); 59 57 60 58 // Do not wait for any reply if chip is frozen 61 59 if (wdev->chip_frozen) ··· 67 69 wdev->hif_cmd.buf_send = request; 68 70 wdev->hif_cmd.buf_recv = reply; 69 71 wdev->hif_cmd.len_recv = reply_len; 70 - wdev->hif_cmd.async = async; 71 72 complete(&wdev->hif_cmd.ready); 72 73 73 74 wfx_bh_request_tx(wdev); 74 75 75 - // NOTE: no timeout is caught async is enabled 76 - if (async) 76 + if (no_reply) { 77 + // Chip won't reply. Give enough time to the wq to send the 78 + // buffer. 79 + msleep(100); 80 + wdev->hif_cmd.buf_send = NULL; 81 + mutex_unlock(&wdev->hif_cmd.lock); 77 82 return 0; 83 + } 78 84 79 85 if (wdev->poll_irq) 80 86 wfx_bh_poll_irq(wdev); ··· 120 118 } 121 119 122 120 // This function is special. After HIF_REQ_ID_SHUT_DOWN, chip won't reply to any 123 - // request anymore. We need to slightly hack struct wfx_hif_cmd for that job. Be 124 - // careful to only call this function during device unregister. 121 + // request anymore. Obviously, only call this function during device unregister. 125 122 int hif_shutdown(struct wfx_dev *wdev) 126 123 { 127 124 int ret; 128 125 struct hif_msg *hif; 129 126 130 - if (wdev->chip_frozen) 131 - return 0; 132 127 wfx_alloc_hif(0, &hif); 133 128 if (!hif) 134 129 return -ENOMEM; 135 130 wfx_fill_header(hif, -1, HIF_REQ_ID_SHUT_DOWN, 0); 136 131 ret = wfx_cmd_send(wdev, hif, NULL, 0, true); 137 - // After this command, chip won't reply. Be sure to give enough time to 138 - // bh to send buffer: 139 - msleep(100); 140 - wdev->hif_cmd.buf_send = NULL; 141 132 if (wdev->pdata.gpio_wakeup) 142 133 gpiod_set_value(wdev->pdata.gpio_wakeup, 0); 143 134 else 144 135 control_reg_write(wdev, 0); 145 - mutex_unlock(&wdev->hif_cmd.lock); 146 136 kfree(hif); 147 137 return ret; 148 138 }
-1
drivers/staging/wfx/hif_tx.h
··· 22 22 struct mutex lock; 23 23 struct completion ready; 24 24 struct completion done; 25 - bool async; 26 25 struct hif_msg *buf_send; 27 26 void *buf_recv; 28 27 size_t len_recv;