Re: [PATCH v14] i2c: virtio: add a virtio i2c frontend driver

Wolfram Sang
2021-07-22T15:35:35Z

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

Hi,

so only minor stuff left from my side.

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

The sorting is not good. I think your entry should go to the bottom of
the Kconfig file.

> 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

This comment can go, I'd say.

> +obj-$(CONFIG_I2C_VIRTIO) += i2c-virtio.o
> +
> ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG

> + /*
> + * We don't support 0 length messages and so masked out
> + * I2C_FUNC_SMBUS_QUICK in virtio_i2c_func().
> + */
> + if (!msgs[i].len)
> + break;

I recommend using struct i2c_adapter_quirks with I2C_AQ_NO_ZERO_LEN. But
let's wait first if zero length are possible or not.

Also, checkpatch:

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

So, is one of you interested in maintaining this driver?

All the best,

Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]