Commit 538be0aa authored by Arnd Bergmann's avatar Arnd Bergmann Committed by Jiri Kosina

HID: intel_ish-hid: clarify locking in client code

I was trying to understand this code while working on a warning
fix and the locking made no sense: spin_lock_irqsave() is pointless
when run inside of an interrupt handler or nested inside of another
spin_lock_irq() or spin_lock_irqsave().

Here it turned out that the comment above the function is wrong,
as both recv_ishtp_cl_msg_dma() and recv_ishtp_cl_msg() can in fact
be called from a work queue rather than an ISR, so we do have to
use the irqsave() version once.

This fixes the comments accordingly, removes the misleading 'dev_flags'
variable and modifies the inner spinlock to not use 'irqsave'.

No functional change is intended, this is just for readability and
it slightly simplifies the object code.
Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
Acked-by: default avatarSrinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
parent 1260662f
...@@ -803,7 +803,7 @@ void ishtp_cl_send_msg(struct ishtp_device *dev, struct ishtp_cl *cl) ...@@ -803,7 +803,7 @@ void ishtp_cl_send_msg(struct ishtp_device *dev, struct ishtp_cl *cl)
* @ishtp_hdr: Pointer to message header * @ishtp_hdr: Pointer to message header
* *
* Receive and dispatch ISHTP client messages. This function executes in ISR * Receive and dispatch ISHTP client messages. This function executes in ISR
* context * or work queue context
*/ */
void recv_ishtp_cl_msg(struct ishtp_device *dev, void recv_ishtp_cl_msg(struct ishtp_device *dev,
struct ishtp_msg_hdr *ishtp_hdr) struct ishtp_msg_hdr *ishtp_hdr)
...@@ -813,7 +813,6 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, ...@@ -813,7 +813,6 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev,
struct ishtp_cl_rb *new_rb; struct ishtp_cl_rb *new_rb;
unsigned char *buffer = NULL; unsigned char *buffer = NULL;
struct ishtp_cl_rb *complete_rb = NULL; struct ishtp_cl_rb *complete_rb = NULL;
unsigned long dev_flags;
unsigned long flags; unsigned long flags;
int rb_count; int rb_count;
...@@ -828,7 +827,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, ...@@ -828,7 +827,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev,
goto eoi; goto eoi;
} }
spin_lock_irqsave(&dev->read_list_spinlock, dev_flags); spin_lock_irqsave(&dev->read_list_spinlock, flags);
rb_count = -1; rb_count = -1;
list_for_each_entry(rb, &dev->read_list.list, list) { list_for_each_entry(rb, &dev->read_list.list, list) {
++rb_count; ++rb_count;
...@@ -840,8 +839,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, ...@@ -840,8 +839,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev,
/* If no Rx buffer is allocated, disband the rb */ /* If no Rx buffer is allocated, disband the rb */
if (rb->buffer.size == 0 || rb->buffer.data == NULL) { if (rb->buffer.size == 0 || rb->buffer.data == NULL) {
spin_unlock_irqrestore(&dev->read_list_spinlock, spin_unlock_irqrestore(&dev->read_list_spinlock, flags);
dev_flags);
dev_err(&cl->device->dev, dev_err(&cl->device->dev,
"Rx buffer is not allocated.\n"); "Rx buffer is not allocated.\n");
list_del(&rb->list); list_del(&rb->list);
...@@ -857,8 +855,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, ...@@ -857,8 +855,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev,
* back FC, so communication will be stuck anyway) * back FC, so communication will be stuck anyway)
*/ */
if (rb->buffer.size < ishtp_hdr->length + rb->buf_idx) { if (rb->buffer.size < ishtp_hdr->length + rb->buf_idx) {
spin_unlock_irqrestore(&dev->read_list_spinlock, spin_unlock_irqrestore(&dev->read_list_spinlock, flags);
dev_flags);
dev_err(&cl->device->dev, dev_err(&cl->device->dev,
"message overflow. size %d len %d idx %ld\n", "message overflow. size %d len %d idx %ld\n",
rb->buffer.size, ishtp_hdr->length, rb->buffer.size, ishtp_hdr->length,
...@@ -884,14 +881,13 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, ...@@ -884,14 +881,13 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev,
* the whole msg arrived, send a new FC, and add a new * the whole msg arrived, send a new FC, and add a new
* rb buffer for the next coming msg * rb buffer for the next coming msg
*/ */
spin_lock_irqsave(&cl->free_list_spinlock, flags); spin_lock(&cl->free_list_spinlock);
if (!list_empty(&cl->free_rb_list.list)) { if (!list_empty(&cl->free_rb_list.list)) {
new_rb = list_entry(cl->free_rb_list.list.next, new_rb = list_entry(cl->free_rb_list.list.next,
struct ishtp_cl_rb, list); struct ishtp_cl_rb, list);
list_del_init(&new_rb->list); list_del_init(&new_rb->list);
spin_unlock_irqrestore(&cl->free_list_spinlock, spin_unlock(&cl->free_list_spinlock);
flags);
new_rb->cl = cl; new_rb->cl = cl;
new_rb->buf_idx = 0; new_rb->buf_idx = 0;
INIT_LIST_HEAD(&new_rb->list); INIT_LIST_HEAD(&new_rb->list);
...@@ -900,8 +896,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, ...@@ -900,8 +896,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev,
ishtp_hbm_cl_flow_control_req(dev, cl); ishtp_hbm_cl_flow_control_req(dev, cl);
} else { } else {
spin_unlock_irqrestore(&cl->free_list_spinlock, spin_unlock(&cl->free_list_spinlock);
flags);
} }
} }
/* One more fragment in message (even if this was last) */ /* One more fragment in message (even if this was last) */
...@@ -914,7 +909,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, ...@@ -914,7 +909,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev,
break; break;
} }
spin_unlock_irqrestore(&dev->read_list_spinlock, dev_flags); spin_unlock_irqrestore(&dev->read_list_spinlock, flags);
/* If it's nobody's message, just read and discard it */ /* If it's nobody's message, just read and discard it */
if (!buffer) { if (!buffer) {
uint8_t rd_msg_buf[ISHTP_RD_MSG_BUF_SIZE]; uint8_t rd_msg_buf[ISHTP_RD_MSG_BUF_SIZE];
...@@ -941,7 +936,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, ...@@ -941,7 +936,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev,
* @hbm: hbm buffer * @hbm: hbm buffer
* *
* Receive and dispatch ISHTP client messages using DMA. This function executes * Receive and dispatch ISHTP client messages using DMA. This function executes
* in ISR context * in ISR or work queue context
*/ */
void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg, void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg,
struct dma_xfer_hbm *hbm) struct dma_xfer_hbm *hbm)
...@@ -951,10 +946,10 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg, ...@@ -951,10 +946,10 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg,
struct ishtp_cl_rb *new_rb; struct ishtp_cl_rb *new_rb;
unsigned char *buffer = NULL; unsigned char *buffer = NULL;
struct ishtp_cl_rb *complete_rb = NULL; struct ishtp_cl_rb *complete_rb = NULL;
unsigned long dev_flags;
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&dev->read_list_spinlock, dev_flags); spin_lock_irqsave(&dev->read_list_spinlock, flags);
list_for_each_entry(rb, &dev->read_list.list, list) { list_for_each_entry(rb, &dev->read_list.list, list) {
cl = rb->cl; cl = rb->cl;
if (!cl || !(cl->host_client_id == hbm->host_client_id && if (!cl || !(cl->host_client_id == hbm->host_client_id &&
...@@ -966,8 +961,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg, ...@@ -966,8 +961,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg,
* If no Rx buffer is allocated, disband the rb * If no Rx buffer is allocated, disband the rb
*/ */
if (rb->buffer.size == 0 || rb->buffer.data == NULL) { if (rb->buffer.size == 0 || rb->buffer.data == NULL) {
spin_unlock_irqrestore(&dev->read_list_spinlock, spin_unlock_irqrestore(&dev->read_list_spinlock, flags);
dev_flags);
dev_err(&cl->device->dev, dev_err(&cl->device->dev,
"response buffer is not allocated.\n"); "response buffer is not allocated.\n");
list_del(&rb->list); list_del(&rb->list);
...@@ -983,8 +977,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg, ...@@ -983,8 +977,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg,
* back FC, so communication will be stuck anyway) * back FC, so communication will be stuck anyway)
*/ */
if (rb->buffer.size < hbm->msg_length) { if (rb->buffer.size < hbm->msg_length) {
spin_unlock_irqrestore(&dev->read_list_spinlock, spin_unlock_irqrestore(&dev->read_list_spinlock, flags);
dev_flags);
dev_err(&cl->device->dev, dev_err(&cl->device->dev,
"message overflow. size %d len %d idx %ld\n", "message overflow. size %d len %d idx %ld\n",
rb->buffer.size, hbm->msg_length, rb->buf_idx); rb->buffer.size, hbm->msg_length, rb->buf_idx);
...@@ -1008,14 +1001,13 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg, ...@@ -1008,14 +1001,13 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg,
* the whole msg arrived, send a new FC, and add a new * the whole msg arrived, send a new FC, and add a new
* rb buffer for the next coming msg * rb buffer for the next coming msg
*/ */
spin_lock_irqsave(&cl->free_list_spinlock, flags); spin_lock(&cl->free_list_spinlock);
if (!list_empty(&cl->free_rb_list.list)) { if (!list_empty(&cl->free_rb_list.list)) {
new_rb = list_entry(cl->free_rb_list.list.next, new_rb = list_entry(cl->free_rb_list.list.next,
struct ishtp_cl_rb, list); struct ishtp_cl_rb, list);
list_del_init(&new_rb->list); list_del_init(&new_rb->list);
spin_unlock_irqrestore(&cl->free_list_spinlock, spin_unlock(&cl->free_list_spinlock);
flags);
new_rb->cl = cl; new_rb->cl = cl;
new_rb->buf_idx = 0; new_rb->buf_idx = 0;
INIT_LIST_HEAD(&new_rb->list); INIT_LIST_HEAD(&new_rb->list);
...@@ -1024,8 +1016,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg, ...@@ -1024,8 +1016,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg,
ishtp_hbm_cl_flow_control_req(dev, cl); ishtp_hbm_cl_flow_control_req(dev, cl);
} else { } else {
spin_unlock_irqrestore(&cl->free_list_spinlock, spin_unlock(&cl->free_list_spinlock);
flags);
} }
/* One more fragment in message (this is always last) */ /* One more fragment in message (this is always last) */
...@@ -1038,7 +1029,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg, ...@@ -1038,7 +1029,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg,
break; break;
} }
spin_unlock_irqrestore(&dev->read_list_spinlock, dev_flags); spin_unlock_irqrestore(&dev->read_list_spinlock, flags);
/* If it's nobody's message, just read and discard it */ /* If it's nobody's message, just read and discard it */
if (!buffer) { if (!buffer) {
dev_err(dev->devc, "Dropped Rx (DMA) msg - no request\n"); dev_err(dev->devc, "Dropped Rx (DMA) msg - no request\n");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment