Messages in this thread |  | | Subject | Re: [PATCH v12] i2c: virtio: add a virtio i2c frontend driver | From | Jie Deng <> | Date | Mon, 5 Jul 2021 14:21:52 +0800 |
| |
On 2021/7/5 10:43, Viresh Kumar wrote: > On 02-07-21, 12:58, Andy Shevchenko wrote: >> On Fri, Jul 02, 2021 at 04:46:47PM +0800, Jie Deng wrote: >>> +static int virtio_i2c_complete_reqs(struct virtqueue *vq, >>> + struct virtio_i2c_req *reqs, >>> + struct i2c_msg *msgs, int nr, >>> + bool fail) >>> +{ >>> + struct virtio_i2c_req *req; >>> + bool failed = fail; > Jie, you can actually get rid of this variable too. Jut rename fail to failed > and everything shall work as you want.
Oh, You are not right. I just found we can't remove this variable. The "fail" and "failed" have different
meanings for this function. We need fail to return the result.
>>> + unsigned int len; >>> + int i, j = 0; >>> + >>> + for (i = 0; i < nr; i++) { >>> + /* Detach the ith request from the vq */ >>> + req = virtqueue_get_buf(vq, &len); >>> + >>> + /* >>> + * Condition (req && req == &reqs[i]) should always meet since >>> + * we have total nr requests in the vq. >>> + */ >>> + if (!failed && (WARN_ON(!(req && req == &reqs[i])) || >>> + (req->in_hdr.status != VIRTIO_I2C_MSG_OK))) >>> + failed = true; >> ...and after failed is true, we are continuing the loop, why? > Actually this function can be called with fail set to true. We proceed as we > need to call i2c_put_dma_safe_msg_buf() for all buffers we allocated earlier. > >>> + i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed); >>> + if (!failed) >>> + ++j; >> Besides better to read j++ the j itself can be renamed to something more >> verbose. >> >>> + } >>> + return (fail ? -ETIMEDOUT : j); >> Redundant parentheses. >> >>> +}
|  |