diff mbox series

[v4] i2c: virtio: add a virtio i2c frontend driver

Message ID 7c5e44c534b3fd07b855af22d8d4b78bc44cd7a4.1602465440.git.jie.deng@intel.com
State New
Headers show
Series [v4] i2c: virtio: add a virtio i2c frontend driver | expand

Commit Message

Jie Deng Oct. 12, 2020, 1:55 a.m. UTC
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.

This driver communicates with the backend driver through a
virtio I2C message structure which includes following parts:

- Header: i2c_msg addr, flags, len.
- Data buffer: the pointer to the I2C msg data.
- Status: the processing result from the backend.

People may implement different backend drivers to emulate
different controllers according to their needs. A backend
example can be found in the device model of the open source
project ACRN. For more information, please refer to
https://projectacrn.org.

The virtio device ID 34 is used for this I2C adpter since IDs
before 34 have been reserved by other virtio devices.

Co-developed-by: Conghui Chen <conghui.chen@intel.com>
Signed-off-by: Conghui Chen <conghui.chen@intel.com>
Signed-off-by: Jie Deng <jie.deng@intel.com>
Reviewed-by: Shuo Liu <shuo.a.liu@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
The device ID request:
        https://github.com/oasis-tcs/virtio-spec/issues/85

The specification:
	https://lists.oasis-open.org/archives/virtio-comment/202009/msg00021.html

Changes in v4:
	- Use (!(vmsg && vmsg == &vi->vmsg)) instead of ((!vmsg) || (vmsg != &vi->vmsg))

Changes in v3:
        - Move the interface into uAPI according to Jason.
        - Fix issues reported by Dan Carpenter.
	- Fix typo reported by Randy.

Changes in v2:
        - Addressed comments received from Michael, Andy and Jason.

 drivers/i2c/busses/Kconfig      |  11 ++
 drivers/i2c/busses/Makefile     |   3 +
 drivers/i2c/busses/i2c-virtio.c | 256 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/virtio_i2c.h |  31 +++++
 include/uapi/linux/virtio_ids.h |   1 +
 5 files changed, 302 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-virtio.c
 create mode 100644 include/uapi/linux/virtio_i2c.h

Comments

kernel test robot Oct. 12, 2020, 5:45 a.m. UTC | #1
Hi Jie,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on vhost/linux-next linus/master linux/master v5.9 next-20201009]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jie-Deng/i2c-virtio-add-a-virtio-i2c-frontend-driver/20201012-095821
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: x86_64-randconfig-a004-20201012 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 9e72d3eaf38f217698f72cb8fdc969a6e72dad3a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/152ef3f6057acca49f276933274b09df8880243d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jie-Deng/i2c-virtio-add-a-virtio-i2c-frontend-driver/20201012-095821
        git checkout 152ef3f6057acca49f276933274b09df8880243d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> error: include/uapi/linux/virtio_i2c.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/virtio_i2c.h] Error 1
   make[2]: Target '__headers' not remade because of errors.
   make[1]: *** [Makefile:1286: headers] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Michael S. Tsirkin Nov. 25, 2020, 9:35 a.m. UTC | #2
On Mon, Oct 12, 2020 at 09:55:55AM +0800, Jie Deng wrote:
> 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.

> 

> This driver communicates with the backend driver through a

> virtio I2C message structure which includes following parts:

> 

> - Header: i2c_msg addr, flags, len.

> - Data buffer: the pointer to the I2C msg data.

> - Status: the processing result from the backend.

> 

> People may implement different backend drivers to emulate

> different controllers according to their needs. A backend

> example can be found in the device model of the open source

> project ACRN. For more information, please refer to

> https://projectacrn.org.

> 

> The virtio device ID 34 is used for this I2C adpter since IDs

> before 34 have been reserved by other virtio devices.

> 

> Co-developed-by: Conghui Chen <conghui.chen@intel.com>

> Signed-off-by: Conghui Chen <conghui.chen@intel.com>

> Signed-off-by: Jie Deng <jie.deng@intel.com>

> Reviewed-by: Shuo Liu <shuo.a.liu@intel.com>

> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


I assume this will be updated once the specification is acked
by the virtio tc. Holding off on this one for now since
we know there will be host/guest ABI changes.

> ---

> The device ID request:

>         https://github.com/oasis-tcs/virtio-spec/issues/85

> 

> The specification:

> 	https://lists.oasis-open.org/archives/virtio-comment/202009/msg00021.html

> 

> Changes in v4:

> 	- Use (!(vmsg && vmsg == &vi->vmsg)) instead of ((!vmsg) || (vmsg != &vi->vmsg))

> 

> Changes in v3:

>         - Move the interface into uAPI according to Jason.

>         - Fix issues reported by Dan Carpenter.

> 	- Fix typo reported by Randy.

> 

> Changes in v2:

>         - Addressed comments received from Michael, Andy and Jason.

> 

>  drivers/i2c/busses/Kconfig      |  11 ++

>  drivers/i2c/busses/Makefile     |   3 +

>  drivers/i2c/busses/i2c-virtio.c | 256 ++++++++++++++++++++++++++++++++++++++++

>  include/uapi/linux/virtio_i2c.h |  31 +++++

>  include/uapi/linux/virtio_ids.h |   1 +

>  5 files changed, 302 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 293e7a0..f2f6543 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"

> +	depends on 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 19aff0e..821acfa 100644

> --- a/drivers/i2c/busses/Makefile

> +++ b/drivers/i2c/busses/Makefile

> @@ -6,6 +6,9 @@

>  # ACPI drivers

>  obj-$(CONFIG_I2C_SCMI)		+= i2c-scmi.o

>  

> +# VIRTIO I2C host controller driver

> +obj-$(CONFIG_I2C_VIRTIO)	+= i2c-virtio.o

> +

>  # PC SMBus host controller drivers

>  obj-$(CONFIG_I2C_ALI1535)	+= i2c-ali1535.o

>  obj-$(CONFIG_I2C_ALI1563)	+= i2c-ali1563.o

> diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c

> new file mode 100644

> index 0000000..36d8c68

> --- /dev/null

> +++ b/drivers/i2c/busses/i2c-virtio.c

> @@ -0,0 +1,256 @@

> +// SPDX-License-Identifier: GPL-2.0-or-later

> +/*

> + * Virtio I2C Bus Driver

> + *

> + * Copyright (c) 2020 Intel Corporation. All rights reserved.

> + */

> +

> +#include <linux/acpi.h>

> +#include <linux/completion.h>

> +#include <linux/err.h>

> +#include <linux/i2c.h>

> +#include <linux/io.h>

> +#include <linux/jiffies.h>

> +#include <linux/kernel.h>

> +#include <linux/module.h>

> +#include <linux/platform_device.h>

> +#include <linux/wait.h>

> +

> +#include <linux/virtio.h>

> +#include <linux/virtio_i2c.h>

> +

> +/**

> + * struct virtio_i2c_msg - the virtio I2C message structure

> + * @hdr: the virtio I2C message header

> + * @buf: virtio I2C message data buffer

> + * @status: the processing result from the backend

> + */

> +struct virtio_i2c_msg {

> +	struct virtio_i2c_hdr hdr;

> +	u8 *buf;

> +	u8 status;

> +};

> +

> +/**

> + * struct virtio_i2c - virtio I2C data

> + * @vdev: virtio device for this controller

> + * @completion: completion of virtio I2C message

> + * @vmsg: the virtio I2C message for communication

> + * @adap: I2C adapter for this controller

> + * @i2c_lock: lock for virtqueue processing

> + * @vq: the virtio virtqueue for communication

> + */

> +struct virtio_i2c {

> +	struct virtio_device *vdev;

> +	struct completion completion;

> +	struct virtio_i2c_msg vmsg;

> +	struct i2c_adapter adap;

> +	struct mutex i2c_lock;

> +	struct virtqueue *vq;

> +};

> +

> +static void virtio_i2c_msg_done(struct virtqueue *vq)

> +{

> +	struct virtio_i2c *vi = vq->vdev->priv;

> +

> +	complete(&vi->completion);

> +}

> +

> +static int virtio_i2c_add_msg(struct virtqueue *vq,

> +			      struct virtio_i2c_msg *vmsg,

> +			      struct i2c_msg *msg)

> +{

> +	struct scatterlist *sgs[3], hdr, bout, bin, status;

> +	int outcnt = 0, incnt = 0;

> +

> +	if (!msg->len)

> +		return -EINVAL;

> +

> +	vmsg->hdr.addr = cpu_to_le16(msg->addr);

> +	vmsg->hdr.flags = cpu_to_le16(msg->flags);

> +	vmsg->hdr.len = cpu_to_le16(msg->len);

> +

> +	vmsg->buf = kzalloc(msg->len, GFP_KERNEL);

> +	if (!vmsg->buf)

> +		return -ENOMEM;

> +

> +	sg_init_one(&hdr, &vmsg->hdr, sizeof(struct virtio_i2c_hdr));

> +	sgs[outcnt++] = &hdr;

> +	if (msg->flags & I2C_M_RD) {

> +		sg_init_one(&bin, vmsg->buf, msg->len);

> +		sgs[outcnt + incnt++] = &bin;

> +	} else {

> +		memcpy(vmsg->buf, msg->buf, msg->len);

> +		sg_init_one(&bout, vmsg->buf, msg->len);

> +		sgs[outcnt++] = &bout;

> +	}

> +	sg_init_one(&status, &vmsg->status, sizeof(vmsg->status));

> +	sgs[outcnt + incnt++] = &status;

> +

> +	return virtqueue_add_sgs(vq, sgs, outcnt, incnt, vmsg, GFP_KERNEL);

> +}

> +

> +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_msg *vmsg;

> +	unsigned long time_left;

> +	int len, i, ret = 0;

> +

> +	mutex_lock(&vi->i2c_lock);

> +	vmsg = &vi->vmsg;

> +	vmsg->buf = NULL;

> +

> +	for (i = 0; i < num; i++) {

> +		ret = virtio_i2c_add_msg(vq, vmsg, &msgs[i]);

> +		if (ret) {

> +			dev_err(&adap->dev, "failed to add msg[%d] to virtqueue.\n", i);

> +			break;

> +		}

> +

> +		virtqueue_kick(vq);

> +

> +		time_left = wait_for_completion_timeout(&vi->completion, adap->timeout);

> +		if (!time_left) {

> +			dev_err(&adap->dev, "msg[%d]: addr=0x%x timeout.\n", i, msgs[i].addr);

> +			break;

> +		}

> +

> +		vmsg = (struct virtio_i2c_msg *)virtqueue_get_buf(vq, &len);

> +		/* vmsg should point to the same address with &vi->vmsg */

> +		if (!(vmsg && vmsg == &vi->vmsg)) {

> +			dev_err(&adap->dev, "msg[%d]: addr=0x%x virtqueue error.\n",

> +				i, msgs[i].addr);

> +			break;

> +		}

> +

> +		if (vmsg->status != VIRTIO_I2C_MSG_OK) {

> +			dev_err(&adap->dev, "msg[%d]: addr=0x%x error=%d.\n",

> +				i, msgs[i].addr, vmsg->status);

> +			break;

> +		}

> +

> +		if ((msgs[i].flags & I2C_M_RD) && msgs[i].len)

> +			memcpy(msgs[i].buf, vmsg->buf, msgs[i].len);

> +

> +		kfree(vmsg->buf);

> +		vmsg->buf = NULL;

> +

> +		reinit_completion(&vi->completion);

> +	}

> +

> +	mutex_unlock(&vi->i2c_lock);

> +	kfree(vi->vmsg.buf);

> +	vi->vmsg.buf = NULL;

> +	return ((ret < 0) ? ret : i);

> +}

> +

> +static void virtio_i2c_del_vqs(struct virtio_device *vdev)

> +{

> +	vdev->config->reset(vdev);

> +	vdev->config->del_vqs(vdev);

> +}

> +

> +static int virtio_i2c_setup_vqs(struct virtio_i2c *vi)

> +{

> +	struct virtio_device *vdev = vi->vdev;

> +

> +	vi->vq = virtio_find_single_vq(vdev, virtio_i2c_msg_done, "msg");

> +	return PTR_ERR_OR_ZERO(vi->vq);

> +}

> +

> +static u32 virtio_i2c_func(struct i2c_adapter *adap)

> +{

> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;

> +}

> +

> +static struct i2c_algorithm virtio_algorithm = {

> +	.master_xfer = virtio_i2c_xfer,

> +	.functionality = virtio_i2c_func,

> +};

> +

> +static struct i2c_adapter virtio_adapter = {

> +	.owner = THIS_MODULE,

> +	.name = "Virtio I2C Adapter",

> +	.class = I2C_CLASS_DEPRECATED,

> +	.algo = &virtio_algorithm,

> +};

> +

> +static int virtio_i2c_probe(struct virtio_device *vdev)

> +{

> +	struct device *pdev = vdev->dev.parent;

> +	struct virtio_i2c *vi;

> +	int ret;

> +

> +	vi = devm_kzalloc(&vdev->dev, sizeof(*vi), GFP_KERNEL);

> +	if (!vi)

> +		return -ENOMEM;

> +

> +	vdev->priv = vi;

> +	vi->vdev = vdev;

> +

> +	mutex_init(&vi->i2c_lock);

> +	init_completion(&vi->completion);

> +

> +	ret = virtio_i2c_setup_vqs(vi);

> +	if (ret)

> +		return ret;

> +

> +	vi->adap = virtio_adapter;

> +	i2c_set_adapdata(&vi->adap, vi);

> +	vi->adap.dev.parent = &vdev->dev;

> +	/* Setup ACPI node for slave devices which will be probed through ACPI */

> +	ACPI_COMPANION_SET(&vi->adap.dev, ACPI_COMPANION(pdev));

> +	vi->adap.timeout = HZ / 10;

> +

> +	ret = i2c_add_adapter(&vi->adap);

> +	if (ret) {

> +		virtio_i2c_del_vqs(vdev);

> +		dev_err(&vdev->dev, "failed to add virtio-i2c adapter.\n");

> +	}

> +

> +	return ret;

> +}

> +

> +static void virtio_i2c_remove(struct virtio_device *vdev)

> +{

> +	struct virtio_i2c *vi = vdev->priv;

> +

> +	i2c_del_adapter(&vi->adap);

> +	virtio_i2c_del_vqs(vdev);

> +}

> +

> +static struct virtio_device_id id_table[] = {

> +	{ VIRTIO_ID_I2C_ADPTER, VIRTIO_DEV_ANY_ID },

> +	{}

> +};

> +MODULE_DEVICE_TABLE(virtio, id_table);

> +

> +static int __maybe_unused virtio_i2c_freeze(struct virtio_device *vdev)

> +{

> +	virtio_i2c_del_vqs(vdev);

> +	return 0;

> +}

> +

> +static int __maybe_unused virtio_i2c_restore(struct virtio_device *vdev)

> +{

> +	return virtio_i2c_setup_vqs(vdev->priv);

> +}

> +

> +static struct virtio_driver virtio_i2c_driver = {

> +	.id_table	= id_table,

> +	.probe		= virtio_i2c_probe,

> +	.remove		= virtio_i2c_remove,

> +	.driver	= {

> +		.name	= "i2c_virtio",

> +	},

> +#ifdef CONFIG_PM_SLEEP

> +	.freeze = virtio_i2c_freeze,

> +	.restore = virtio_i2c_restore,

> +#endif

> +};

> +module_virtio_driver(virtio_i2c_driver);

> +

> +MODULE_DESCRIPTION("Virtio i2c bus driver");

> +MODULE_LICENSE("GPL");

> diff --git a/include/uapi/linux/virtio_i2c.h b/include/uapi/linux/virtio_i2c.h

> new file mode 100644

> index 0000000..7413e45

> --- /dev/null

> +++ b/include/uapi/linux/virtio_i2c.h

> @@ -0,0 +1,31 @@

> +/* SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause */

> +/*

> + * Definitions for virtio I2C Adpter

> + *

> + * Copyright (c) 2020 Intel Corporation. All rights reserved.

> + */

> +

> +#ifndef _UAPI_LINUX_VIRTIO_I2C_H

> +#define _UAPI_LINUX_VIRTIO_I2C_H

> +

> +#include <linux/types.h>

> +#include <linux/virtio_ids.h>

> +#include <linux/virtio_config.h>

> +

> +/**

> + * struct virtio_i2c_hdr - the virtio I2C message header structure

> + * @addr: i2c_msg addr, the slave address

> + * @flags: i2c_msg flags

> + * @len: i2c_msg len

> + */

> +struct virtio_i2c_hdr {

> +	__le16 addr;

> +	__le16 flags;

> +	__le16 len;

> +};

> +

> +/* The final status written by the device */

> +#define VIRTIO_I2C_MSG_OK	0

> +#define VIRTIO_I2C_MSG_ERR	1

> +

> +#endif /* _UAPI_LINUX_VIRTIO_I2C_H */

> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h

> index b052355..398ef2d 100644

> --- a/include/uapi/linux/virtio_ids.h

> +++ b/include/uapi/linux/virtio_ids.h

> @@ -48,5 +48,6 @@

>  #define VIRTIO_ID_FS           26 /* virtio filesystem */

>  #define VIRTIO_ID_PMEM         27 /* virtio pmem */

>  #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */

> +#define VIRTIO_ID_I2C_ADPTER   34 /* virtio i2c adpter */

>  

>  #endif /* _LINUX_VIRTIO_IDS_H */

> -- 

> 2.7.4
Jie Deng Nov. 26, 2020, 2 a.m. UTC | #3
On 2020/11/25 17:35, Michael S. Tsirkin wrote:

> On Mon, Oct 12, 2020 at 09:55:55AM +0800, Jie Deng wrote:

>> 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.

>>

>> This driver communicates with the backend driver through a

>> virtio I2C message structure which includes following parts:

>>

>> - Header: i2c_msg addr, flags, len.

>> - Data buffer: the pointer to the I2C msg data.

>> - Status: the processing result from the backend.

>>

>> People may implement different backend drivers to emulate

>> different controllers according to their needs. A backend

>> example can be found in the device model of the open source

>> project ACRN. For more information, please refer to

>> https://projectacrn.org.

>>

>> The virtio device ID 34 is used for this I2C adpter since IDs

>> before 34 have been reserved by other virtio devices.

>>

>> Co-developed-by: Conghui Chen <conghui.chen@intel.com>

>> Signed-off-by: Conghui Chen <conghui.chen@intel.com>

>> Signed-off-by: Jie Deng <jie.deng@intel.com>

>> Reviewed-by: Shuo Liu <shuo.a.liu@intel.com>

>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> I assume this will be updated once the specification is acked

> by the virtio tc. Holding off on this one for now since

> we know there will be host/guest ABI changes.


Yeah. I will update this driver once the specification is acked by the 
virtio tc.

Thanks.
Viresh Kumar Feb. 25, 2021, 7:21 a.m. UTC | #4
On 12-10-20, 09:55, Jie Deng wrote:
> 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.

> 

> This driver communicates with the backend driver through a

> virtio I2C message structure which includes following parts:

> 

> - Header: i2c_msg addr, flags, len.

> - Data buffer: the pointer to the I2C msg data.

> - Status: the processing result from the backend.

> 

> People may implement different backend drivers to emulate

> different controllers according to their needs. A backend

> example can be found in the device model of the open source

> project ACRN. For more information, please refer to

> https://projectacrn.org.


> diff --git a/include/uapi/linux/virtio_i2c.h b/include/uapi/linux/virtio_i2c.h

> new file mode 100644

> index 0000000..7413e45

> --- /dev/null

> +++ b/include/uapi/linux/virtio_i2c.h

> @@ -0,0 +1,31 @@

> +/* SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause */

> +/*

> + * Definitions for virtio I2C Adpter

> + *

> + * Copyright (c) 2020 Intel Corporation. All rights reserved.

> + */

> +

> +#ifndef _UAPI_LINUX_VIRTIO_I2C_H

> +#define _UAPI_LINUX_VIRTIO_I2C_H

> +

> +#include <linux/types.h>

> +#include <linux/virtio_ids.h>

> +#include <linux/virtio_config.h>

> +

> +/**

> + * struct virtio_i2c_hdr - the virtio I2C message header structure

> + * @addr: i2c_msg addr, the slave address

> + * @flags: i2c_msg flags

> + * @len: i2c_msg len

> + */

> +struct virtio_i2c_hdr {

> +	__le16 addr;

> +	__le16 flags;

> +	__le16 len;

> +};


Hi Jie,

I am a bit confused about the header and the format in which data is being
processed here. When I look at the specification present here:

https://lists.oasis-open.org/archives/virtio-comment/202009/msg00021.html

it talks about 

struct virtio_i2c_out_hdr {
        le16 addr;
        le16 padding;
        le32 flags;
};
struct virtio_i2c_in_hdr {
        u8 status;
};

struct virtio_i2c_req {
        struct virtio_i2c_out_hdr out_hdr;
        u8 write_buf [];
        u8 read_buf [];
        struct virtio_i2c_in_hdr in_hdr;
};

while what we have above is completely different. What am I missing ?

-- 
viresh
Jie Deng Feb. 26, 2021, 2:46 a.m. UTC | #5
On 2021/2/25 15:21, Viresh Kumar wrote:
> On 12-10-20, 09:55, Jie Deng wrote:

>> 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.

>>

>> This driver communicates with the backend driver through a

>> virtio I2C message structure which includes following parts:

>>

>> - Header: i2c_msg addr, flags, len.

>> - Data buffer: the pointer to the I2C msg data.

>> - Status: the processing result from the backend.

>>

>> People may implement different backend drivers to emulate

>> different controllers according to their needs. A backend

>> example can be found in the device model of the open source

>> project ACRN. For more information, please refer to

>> https://projectacrn.org.

>> diff --git a/include/uapi/linux/virtio_i2c.h b/include/uapi/linux/virtio_i2c.h

>> new file mode 100644

>> index 0000000..7413e45

>> --- /dev/null

>> +++ b/include/uapi/linux/virtio_i2c.h

>> @@ -0,0 +1,31 @@

>> +/* SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause */

>> +/*

>> + * Definitions for virtio I2C Adpter

>> + *

>> + * Copyright (c) 2020 Intel Corporation. All rights reserved.

>> + */

>> +

>> +#ifndef _UAPI_LINUX_VIRTIO_I2C_H

>> +#define _UAPI_LINUX_VIRTIO_I2C_H

>> +

>> +#include <linux/types.h>

>> +#include <linux/virtio_ids.h>

>> +#include <linux/virtio_config.h>

>> +

>> +/**

>> + * struct virtio_i2c_hdr - the virtio I2C message header structure

>> + * @addr: i2c_msg addr, the slave address

>> + * @flags: i2c_msg flags

>> + * @len: i2c_msg len

>> + */

>> +struct virtio_i2c_hdr {

>> +	__le16 addr;

>> +	__le16 flags;

>> +	__le16 len;

>> +};

> Hi Jie,

>

> I am a bit confused about the header and the format in which data is being

> processed here. When I look at the specification present here:

>

> https://lists.oasis-open.org/archives/virtio-comment/202009/msg00021.html

>

> it talks about

>

> struct virtio_i2c_out_hdr {

>          le16 addr;

>          le16 padding;

>          le32 flags;

> };

> struct virtio_i2c_in_hdr {

>          u8 status;

> };

>

> struct virtio_i2c_req {

>          struct virtio_i2c_out_hdr out_hdr;

>          u8 write_buf [];

>          u8 read_buf [];

>          struct virtio_i2c_in_hdr in_hdr;

> };

>

> while what we have above is completely different. What am I missing ?


This v4 was the old version before the specification was acked by the 
virtio tc.

Following is the latest specification.

https://raw.githubusercontent.com/oasis-tcs/virtio-spec/master/virtio-i2c.tex

I will send the v5 since the host/guest ABI changes.

Thanks.
Viresh Kumar Feb. 26, 2021, 4:21 a.m. UTC | #6
On 26-02-21, 10:46, Jie Deng wrote:
> This v4 was the old version before the specification was acked by the virtio

> tc.

> 

> Following is the latest specification.

> 

> https://raw.githubusercontent.com/oasis-tcs/virtio-spec/master/virtio-i2c.tex

> 

> I will send the v5 since the host/guest ABI changes.


Okay, now it makes some sense :)

I am interested in this stuff, if possible please keep me Cc'd for following
versions, thanks.

-- 
viresh
Jie Deng Feb. 26, 2021, 6:36 a.m. UTC | #7
On 2021/2/26 12:21, Viresh Kumar wrote:
> On 26-02-21, 10:46, Jie Deng wrote:

>> This v4 was the old version before the specification was acked by the virtio

>> tc.

>>

>> Following is the latest specification.

>>

>> https://raw.githubusercontent.com/oasis-tcs/virtio-spec/master/virtio-i2c.tex

>>

>> I will send the v5 since the host/guest ABI changes.

> Okay, now it makes some sense :)

>

> I am interested in this stuff, if possible please keep me Cc'd for following

> versions, thanks.

Sure. I will add you to the Cc list.
diff mbox series

Patch

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 293e7a0..f2f6543 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"
+	depends on 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 19aff0e..821acfa 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -6,6 +6,9 @@ 
 # ACPI drivers
 obj-$(CONFIG_I2C_SCMI)		+= i2c-scmi.o
 
+# VIRTIO I2C host controller driver
+obj-$(CONFIG_I2C_VIRTIO)	+= i2c-virtio.o
+
 # PC SMBus host controller drivers
 obj-$(CONFIG_I2C_ALI1535)	+= i2c-ali1535.o
 obj-$(CONFIG_I2C_ALI1563)	+= i2c-ali1563.o
diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
new file mode 100644
index 0000000..36d8c68
--- /dev/null
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -0,0 +1,256 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtio I2C Bus Driver
+ *
+ * Copyright (c) 2020 Intel Corporation. All rights reserved.
+ */
+
+#include <linux/acpi.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/io.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/wait.h>
+
+#include <linux/virtio.h>
+#include <linux/virtio_i2c.h>
+
+/**
+ * struct virtio_i2c_msg - the virtio I2C message structure
+ * @hdr: the virtio I2C message header
+ * @buf: virtio I2C message data buffer
+ * @status: the processing result from the backend
+ */
+struct virtio_i2c_msg {
+	struct virtio_i2c_hdr hdr;
+	u8 *buf;
+	u8 status;
+};
+
+/**
+ * struct virtio_i2c - virtio I2C data
+ * @vdev: virtio device for this controller
+ * @completion: completion of virtio I2C message
+ * @vmsg: the virtio I2C message for communication
+ * @adap: I2C adapter for this controller
+ * @i2c_lock: lock for virtqueue processing
+ * @vq: the virtio virtqueue for communication
+ */
+struct virtio_i2c {
+	struct virtio_device *vdev;
+	struct completion completion;
+	struct virtio_i2c_msg vmsg;
+	struct i2c_adapter adap;
+	struct mutex i2c_lock;
+	struct virtqueue *vq;
+};
+
+static void virtio_i2c_msg_done(struct virtqueue *vq)
+{
+	struct virtio_i2c *vi = vq->vdev->priv;
+
+	complete(&vi->completion);
+}
+
+static int virtio_i2c_add_msg(struct virtqueue *vq,
+			      struct virtio_i2c_msg *vmsg,
+			      struct i2c_msg *msg)
+{
+	struct scatterlist *sgs[3], hdr, bout, bin, status;
+	int outcnt = 0, incnt = 0;
+
+	if (!msg->len)
+		return -EINVAL;
+
+	vmsg->hdr.addr = cpu_to_le16(msg->addr);
+	vmsg->hdr.flags = cpu_to_le16(msg->flags);
+	vmsg->hdr.len = cpu_to_le16(msg->len);
+
+	vmsg->buf = kzalloc(msg->len, GFP_KERNEL);
+	if (!vmsg->buf)
+		return -ENOMEM;
+
+	sg_init_one(&hdr, &vmsg->hdr, sizeof(struct virtio_i2c_hdr));
+	sgs[outcnt++] = &hdr;
+	if (msg->flags & I2C_M_RD) {
+		sg_init_one(&bin, vmsg->buf, msg->len);
+		sgs[outcnt + incnt++] = &bin;
+	} else {
+		memcpy(vmsg->buf, msg->buf, msg->len);
+		sg_init_one(&bout, vmsg->buf, msg->len);
+		sgs[outcnt++] = &bout;
+	}
+	sg_init_one(&status, &vmsg->status, sizeof(vmsg->status));
+	sgs[outcnt + incnt++] = &status;
+
+	return virtqueue_add_sgs(vq, sgs, outcnt, incnt, vmsg, GFP_KERNEL);
+}
+
+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_msg *vmsg;
+	unsigned long time_left;
+	int len, i, ret = 0;
+
+	mutex_lock(&vi->i2c_lock);
+	vmsg = &vi->vmsg;
+	vmsg->buf = NULL;
+
+	for (i = 0; i < num; i++) {
+		ret = virtio_i2c_add_msg(vq, vmsg, &msgs[i]);
+		if (ret) {
+			dev_err(&adap->dev, "failed to add msg[%d] to virtqueue.\n", i);
+			break;
+		}
+
+		virtqueue_kick(vq);
+
+		time_left = wait_for_completion_timeout(&vi->completion, adap->timeout);
+		if (!time_left) {
+			dev_err(&adap->dev, "msg[%d]: addr=0x%x timeout.\n", i, msgs[i].addr);
+			break;
+		}
+
+		vmsg = (struct virtio_i2c_msg *)virtqueue_get_buf(vq, &len);
+		/* vmsg should point to the same address with &vi->vmsg */
+		if (!(vmsg && vmsg == &vi->vmsg)) {
+			dev_err(&adap->dev, "msg[%d]: addr=0x%x virtqueue error.\n",
+				i, msgs[i].addr);
+			break;
+		}
+
+		if (vmsg->status != VIRTIO_I2C_MSG_OK) {
+			dev_err(&adap->dev, "msg[%d]: addr=0x%x error=%d.\n",
+				i, msgs[i].addr, vmsg->status);
+			break;
+		}
+
+		if ((msgs[i].flags & I2C_M_RD) && msgs[i].len)
+			memcpy(msgs[i].buf, vmsg->buf, msgs[i].len);
+
+		kfree(vmsg->buf);
+		vmsg->buf = NULL;
+
+		reinit_completion(&vi->completion);
+	}
+
+	mutex_unlock(&vi->i2c_lock);
+	kfree(vi->vmsg.buf);
+	vi->vmsg.buf = NULL;
+	return ((ret < 0) ? ret : i);
+}
+
+static void virtio_i2c_del_vqs(struct virtio_device *vdev)
+{
+	vdev->config->reset(vdev);
+	vdev->config->del_vqs(vdev);
+}
+
+static int virtio_i2c_setup_vqs(struct virtio_i2c *vi)
+{
+	struct virtio_device *vdev = vi->vdev;
+
+	vi->vq = virtio_find_single_vq(vdev, virtio_i2c_msg_done, "msg");
+	return PTR_ERR_OR_ZERO(vi->vq);
+}
+
+static u32 virtio_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm virtio_algorithm = {
+	.master_xfer = virtio_i2c_xfer,
+	.functionality = virtio_i2c_func,
+};
+
+static struct i2c_adapter virtio_adapter = {
+	.owner = THIS_MODULE,
+	.name = "Virtio I2C Adapter",
+	.class = I2C_CLASS_DEPRECATED,
+	.algo = &virtio_algorithm,
+};
+
+static int virtio_i2c_probe(struct virtio_device *vdev)
+{
+	struct device *pdev = vdev->dev.parent;
+	struct virtio_i2c *vi;
+	int ret;
+
+	vi = devm_kzalloc(&vdev->dev, sizeof(*vi), GFP_KERNEL);
+	if (!vi)
+		return -ENOMEM;
+
+	vdev->priv = vi;
+	vi->vdev = vdev;
+
+	mutex_init(&vi->i2c_lock);
+	init_completion(&vi->completion);
+
+	ret = virtio_i2c_setup_vqs(vi);
+	if (ret)
+		return ret;
+
+	vi->adap = virtio_adapter;
+	i2c_set_adapdata(&vi->adap, vi);
+	vi->adap.dev.parent = &vdev->dev;
+	/* Setup ACPI node for slave devices which will be probed through ACPI */
+	ACPI_COMPANION_SET(&vi->adap.dev, ACPI_COMPANION(pdev));
+	vi->adap.timeout = HZ / 10;
+
+	ret = i2c_add_adapter(&vi->adap);
+	if (ret) {
+		virtio_i2c_del_vqs(vdev);
+		dev_err(&vdev->dev, "failed to add virtio-i2c adapter.\n");
+	}
+
+	return ret;
+}
+
+static void virtio_i2c_remove(struct virtio_device *vdev)
+{
+	struct virtio_i2c *vi = vdev->priv;
+
+	i2c_del_adapter(&vi->adap);
+	virtio_i2c_del_vqs(vdev);
+}
+
+static struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_I2C_ADPTER, VIRTIO_DEV_ANY_ID },
+	{}
+};
+MODULE_DEVICE_TABLE(virtio, id_table);
+
+static int __maybe_unused virtio_i2c_freeze(struct virtio_device *vdev)
+{
+	virtio_i2c_del_vqs(vdev);
+	return 0;
+}
+
+static int __maybe_unused virtio_i2c_restore(struct virtio_device *vdev)
+{
+	return virtio_i2c_setup_vqs(vdev->priv);
+}
+
+static struct virtio_driver virtio_i2c_driver = {
+	.id_table	= id_table,
+	.probe		= virtio_i2c_probe,
+	.remove		= virtio_i2c_remove,
+	.driver	= {
+		.name	= "i2c_virtio",
+	},
+#ifdef CONFIG_PM_SLEEP
+	.freeze = virtio_i2c_freeze,
+	.restore = virtio_i2c_restore,
+#endif
+};
+module_virtio_driver(virtio_i2c_driver);
+
+MODULE_DESCRIPTION("Virtio i2c bus driver");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/virtio_i2c.h b/include/uapi/linux/virtio_i2c.h
new file mode 100644
index 0000000..7413e45
--- /dev/null
+++ b/include/uapi/linux/virtio_i2c.h
@@ -0,0 +1,31 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause */
+/*
+ * Definitions for virtio I2C Adpter
+ *
+ * Copyright (c) 2020 Intel Corporation. All rights reserved.
+ */
+
+#ifndef _UAPI_LINUX_VIRTIO_I2C_H
+#define _UAPI_LINUX_VIRTIO_I2C_H
+
+#include <linux/types.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_config.h>
+
+/**
+ * struct virtio_i2c_hdr - the virtio I2C message header structure
+ * @addr: i2c_msg addr, the slave address
+ * @flags: i2c_msg flags
+ * @len: i2c_msg len
+ */
+struct virtio_i2c_hdr {
+	__le16 addr;
+	__le16 flags;
+	__le16 len;
+};
+
+/* The final status written by the device */
+#define VIRTIO_I2C_MSG_OK	0
+#define VIRTIO_I2C_MSG_ERR	1
+
+#endif /* _UAPI_LINUX_VIRTIO_I2C_H */
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index b052355..398ef2d 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -48,5 +48,6 @@ 
 #define VIRTIO_ID_FS           26 /* virtio filesystem */
 #define VIRTIO_ID_PMEM         27 /* virtio pmem */
 #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
+#define VIRTIO_ID_I2C_ADPTER   34 /* virtio i2c adpter */
 
 #endif /* _LINUX_VIRTIO_IDS_H */