On 22-03-21, 21:35, Jie Deng wrote: > diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c > new file mode 100644 > index 0000000..316986e > --- /dev/null > +++ b/drivers/i2c/busses/i2c-virtio.c > @@ -0,0 +1,286 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Virtio I2C Bus Driver > + * > + * The Virtio I2C Specification: > + * https://raw.githubusercontent.com/oasis-tcs/virtio-spec/master/virtio-i2c.tex > + * > + * Copyright (c) 2021 Intel Corporation. All rights reserved. > + */ > + > +#include <linux/acpi.h> > +#include <linux/completion.h> > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/virtio.h> > +#include <linux/virtio_ids.h> > +#include <linux/virtio_config.h> > +#include <linux/virtio_i2c.h> > + > +/** > + * struct virtio_i2c - virtio I2C data > + * @vdev: virtio device for this controller > + * @completion: completion of virtio I2C message > + * @adap: I2C adapter for this controller > + * @i2c_lock: lock for virtqueue processing Name mismatch here. > + * @vq: the virtio virtqueue for communication > + */ > +struct virtio_i2c { > + struct virtio_device *vdev; > + struct completion completion; > + struct i2c_adapter adap; > + struct mutex lock; > + struct virtqueue *vq; > +}; > +static int virtio_i2c_complete_reqs(struct virtqueue *vq, > + struct virtio_i2c_req *reqs, > + struct i2c_msg *msgs, int nr, > + bool timeout) > +{ > + struct virtio_i2c_req *req; > + bool err_found = false; > + 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); > + > + if (timeout || err_found) { > + i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false); > + continue; > + } > + > + /* > + * Condition (req && req == &reqs[i]) should always meet since > + * we have total nr requests in the vq. > + */ > + if (WARN_ON(!(req && req == &reqs[i])) || > + (req->in_hdr.status != VIRTIO_I2C_MSG_OK)) { > + err_found = true; > + i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false); > + continue; > + } > + > + i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], true); > + ++j; > + } I think you can simplify the code like this here: bool err_found = timeout; 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 (!err_found && (WARN_ON(!(req && req == &reqs[i])) || (req->in_hdr.status != VIRTIO_I2C_MSG_OK))) { err_found = true; continue; } i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], err_found); if (!err_found) ++j; > + > + return (timeout ? -ETIMEDOUT : j); > +} > + > +static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) > +{ > + struct virtio_i2c *vi = i2c_get_adapdata(adap); > + struct virtqueue *vq = vi->vq; > + struct virtio_i2c_req *reqs; > + unsigned long time_left; > + int ret, nr; > + > + reqs = kcalloc(num, sizeof(*reqs), GFP_KERNEL); > + if (!reqs) > + return -ENOMEM; > + > + mutex_lock(&vi->lock); > + > + ret = virtio_i2c_send_reqs(vq, reqs, msgs, num); > + if (ret == 0) > + goto err_unlock_free; > + > + nr = ret; > + reinit_completion(&vi->completion); > + virtqueue_kick(vq); > + > + time_left = wait_for_completion_timeout(&vi->completion, adap->timeout); > + if (!time_left) { > + dev_err(&adap->dev, "virtio i2c backend timeout.\n"); > + ret = virtio_i2c_complete_reqs(vq, reqs, msgs, nr, true); > + goto err_unlock_free; > + } > + > + ret = virtio_i2c_complete_reqs(vq, reqs, msgs, nr, false); And this can be optimized as well: time_left = wait_for_completion_timeout(&vi->completion, adap->timeout); if (!time_left) dev_err(&adap->dev, "virtio i2c backend timeout.\n"); ret = virtio_i2c_complete_reqs(vq, reqs, msgs, nr, !time_left); -- viresh