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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 22 Jul 2021 17:35:29 +0200
From:   Wolfram Sang <wsa@...nel.org>
To:     Jie Deng <jie.deng@...el.com>
Cc:     linux-i2c@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org, mst@...hat.com, arnd@...db.de,
        jasowang@...hat.com, andriy.shevchenko@...ux.intel.com,
        yu1.wang@...el.com, shuo.a.liu@...el.com, conghui.chen@...el.com,
        viresh.kumar@...aro.org, stefanha@...hat.com,
        gregkh@...uxfoundation.org
Subject: Re: [PATCH v14] i2c: virtio: add a virtio i2c frontend driver

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


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)