Re: [PATCH v13] i2c: virtio: add a virtio i2c frontend driver
On 05-07-21, 12:38, Andy Shevchenko wrote: > Because we do not have "uapi" in the path in /usr/include on the real > system where the linux-headers (or kernel-headers) package is > installed. > > It's still possible that our installation hooks will remove that > "uapi" from the headers, but I think it makes things too complicated. Ahh, right. Yeah, I completely missed that part. -- viresh ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v13] i2c: virtio: add a virtio i2c frontend driver
On Mon, Jul 5, 2021 at 11:56 AM Viresh Kumar wrote: > On 05-07-21, 11:45, Andy Shevchenko wrote: > > On Mon, Jul 5, 2021 at 11:03 AM Viresh Kumar > > wrote: > > > On 05-07-21, 14:53, Jie Deng wrote: > > > > > > +#include > > > > +#include > > > > > > Both of these need to be the uapi headers as Andy said earlier > > > > They are already since this header _is_ UAPI, > > Ahh, there is some tricky header inclusion there :) > > > what you are suggesting is gonna not work, > > Why ? Because we do not have "uapi" in the path in /usr/include on the real system where the linux-headers (or kernel-headers) package is installed. It's still possible that our installation hooks will remove that "uapi" from the headers, but I think it makes things too complicated. > > although it's correct for in-kernel users of UAPI > > headers. -- With Best Regards, Andy Shevchenko ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v13] i2c: virtio: add a virtio i2c frontend driver
On 05-07-21, 11:45, Andy Shevchenko wrote: > On Mon, Jul 5, 2021 at 11:03 AM Viresh Kumar wrote: > > On 05-07-21, 14:53, Jie Deng wrote: > > > > +#include > > > +#include > > > > Both of these need to be the uapi headers as Andy said earlier > > They are already since this header _is_ UAPI, Ahh, there is some tricky header inclusion there :) > what you are suggesting is gonna not work, Why ? > although it's correct for in-kernel users of UAPI > headers. -- viresh ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v13] i2c: virtio: add a virtio i2c frontend driver
On Mon, Jul 5, 2021 at 11:03 AM Viresh Kumar wrote: > On 05-07-21, 14:53, Jie Deng wrote: > > +#include > > +#include > > Both of these need to be the uapi headers as Andy said earlier They are already since this header _is_ UAPI, what you are suggesting is gonna not work, although it's correct for in-kernel users of UAPI headers. > and they better > be in alphabetical order. I prefer that as well. > #include > #include -- With Best Regards, Andy Shevchenko ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v13] i2c: virtio: add a virtio i2c frontend driver
Hi Jie.
On 05-07-21, 14:53, Jie Deng wrote:
> diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> +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;
> + 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, );
Calling this for cases where virtio_i2c_prepare_reqs() itself has failed will
always return NULL, should we even try to call this then ?
virtqueue_get_buf() returns the next used buffer only, i.e. returned by the host
?
> +
> + /*
> + * Condition (req && req == [i]) should always meet since
> + * we have total nr requests in the vq.
> + */
> + if (!failed && (WARN_ON(!(req && req == [i])) ||
Mentioning again for completeness of the review, reqs[i] can never be NULL here
though req can be. And even in that case you never need to check req here.
Feel free to ignore it if you want, we can always send a fixup patch later and
discuss it further :)
> + (req->in_hdr.status != VIRTIO_I2C_MSG_OK)))
> + failed = true;
> +
> + i2c_put_dma_safe_msg_buf(reqs[i].buf, [i], !failed);
> + if (!failed)
> + j++;
> + }
> +
> + return fail ? 0 : j;
Since you don't return ETIMEDOUT anymore, you can simply return j now. And so we
can work with a single failed argument and don't need both fail and failed.
> +}
> +
> +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;
> +
> + reqs = kcalloc(num, sizeof(*reqs), GFP_KERNEL);
> + if (!reqs)
> + return -ENOMEM;
> +
> + ret = virtio_i2c_prepare_reqs(vq, reqs, msgs, num);
> + if (ret != num) {
> + ret = virtio_i2c_complete_reqs(vq, reqs, msgs, ret, true);
> + goto err_free;
> + }
> +
> + reinit_completion(>completion);
> + virtqueue_kick(vq);
> + time_left = wait_for_completion_timeout(>completion, adap->timeout);
> + ret = virtio_i2c_complete_reqs(vq, reqs, msgs, num, !time_left);
> +
> + if (!time_left) {
> + ret = -ETIMEDOUT;
> + dev_err(>dev, "virtio i2c backend timeout.\n");
> + }
> +
> +err_free:
> + kfree(reqs);
> + return ret;
> +}
> diff --git a/include/uapi/linux/virtio_i2c.h b/include/uapi/linux/virtio_i2c.h
> new file mode 100644
> index 000..df936a2
> --- /dev/null
> +++ b/include/uapi/linux/virtio_i2c.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later WITH Linux-syscall-note */
> +/*
> + * Definitions for virtio I2C Adpter
> + *
> + * Copyright (c) 2021 Intel Corporation. All rights reserved.
> + */
> +
> +#ifndef _UAPI_LINUX_VIRTIO_I2C_H
> +#define _UAPI_LINUX_VIRTIO_I2C_H
> +
> +#include
> +#include
Both of these need to be the uapi headers as Andy said earlier and they better
be in alphabetical order.
#include
#include
Though in your support, I do see a lot of files in uapi/linux using the standard
types.h, which looks wrong as that types.h is not a userspace ABI.
--
viresh
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v13] i2c: virtio: add a virtio i2c frontend driver
Add an I2C bus driver for virtio para-virtualization.
The controller can be emulated by the backend driver in
any device model software by following the virtio protocol.
The device specification can be found on
https://lists.oasis-open.org/archives/virtio-comment/202101/msg8.html.
By following the specification, people may implement different
backend drivers to emulate different controllers according to
their needs.
Co-developed-by: Conghui Chen
Signed-off-by: Conghui Chen
Signed-off-by: Jie Deng
---
Changes v12 -> v13
- Use _BITUL() instead of BIT()
- Rename "virtio_i2c_send_reqs" to "virtio_i2c_prepare_reqs"
- Optimize the return value of "virtio_i2c_complete_reqs"
Changes v11 -> v12
- Do not sent msg_buf for zero-length request.
- Send requests to host only if all the number of transfers requested
prepared successfully.
- Remove the line #include in virtio_i2c.h
Changes v10 -> v11
- Remove vi->adap.class = I2C_CLASS_DEPRECATED.
- Use #ifdef CONFIG_PM_SLEEP to replace the "__maybe_unused".
- Remove "struct mutex lock" in "struct virtio_i2c".
- Support zero-length request.
- Remove unnecessary logs.
- Remove vi->adap.timeout = HZ / 10, just use the default value.
- Use BIT(0) to define VIRTIO_I2C_FLAGS_FAIL_NEXT.
- Add the virtio_device index to adapter's naming mechanism.
drivers/i2c/busses/Kconfig | 11 ++
drivers/i2c/busses/Makefile | 3 +
drivers/i2c/busses/i2c-virtio.c | 269
include/uapi/linux/virtio_i2c.h | 41 ++
include/uapi/linux/virtio_ids.h | 1 +
5 files changed, 325 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-virtio.c
create mode 100644 include/uapi/linux/virtio_i2c.h
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 10acece..e47616a 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -21,6 +21,17 @@ config I2C_ALI1535
This driver can also be built as a module. If so, the module
will be called i2c-ali1535.
+config I2C_VIRTIO
+ tristate "Virtio I2C Adapter"
+ select VIRTIO
+ help
+ If you say yes to this option, support will be included for the virtio
+ I2C adapter driver. The hardware can be emulated by any device model
+ software according to the virtio protocol.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-virtio.
+
config I2C_ALI1563
tristate "ALI 1563"
depends on PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 69e9963..9843756 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -147,4 +147,7 @@ obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
obj-$(CONFIG_I2C_FSI) += i2c-fsi.o
+# VIRTIO I2C host controller driver
+obj-$(CONFIG_I2C_VIRTIO) += i2c-virtio.o
+
ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
new file mode 100644
index 000..731267d
--- /dev/null
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -0,0 +1,269 @@
+// 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * struct virtio_i2c - virtio I2C data
+ * @vdev: virtio device for this controller
+ * @completion: completion of virtio I2C message
+ * @adap: I2C adapter for this controller
+ * @vq: the virtio virtqueue for communication
+ */
+struct virtio_i2c {
+ struct virtio_device *vdev;
+ struct completion completion;
+ struct i2c_adapter adap;
+ struct virtqueue *vq;
+};
+
+/**
+ * struct virtio_i2c_req - the virtio I2C request structure
+ * @out_hdr: the OUT header of the virtio I2C message
+ * @buf: the buffer into which data is read, or from which it's written
+ * @in_hdr: the IN header of the virtio I2C message
+ */
+struct virtio_i2c_req {
+ struct virtio_i2c_out_hdr out_hdr cacheline_aligned;
+ uint8_t *bufcacheline_aligned;
+ struct virtio_i2c_in_hdr in_hdr cacheline_aligned;
+};
+
+static void virtio_i2c_msg_done(struct virtqueue *vq)
+{
+ struct virtio_i2c *vi = vq->vdev->priv;
+
+ complete(>completion);
+}
+
+static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
+ struct virtio_i2c_req *reqs,
+ struct i2c_msg *msgs, int nr)
+{
+ struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr;
+ int i, outcnt, incnt, err = 0;
+
+
