Re: [Qemu-devel] [PATCH] chardev/char-i2c: Implement Linux I2C character device

[Eric Blake <eblake@redhat.com>]

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

On 5/3/19 2:31 PM, Ernest Esene wrote:
> Add support for Linux I2C character device for I2C device passthrough
> For example:
> -chardev linux-i2c,address=0x46,path=/dev/i2c-N,id=i2c-chardev
> 
> Signed-off-by: Ernest Esene <eroken1@gmail.com>
> ---

Just an interface review:

> +++ b/qapi/char.json
> @@ -240,6 +240,21 @@
>    'data': { 'device': 'str' },
>    'base': 'ChardevCommon' }
>  
> +##
> +# @ChardevI2c:
> +#
> +# Configuration info for i2c chardev (only on linux).

Rather than a parenthetical comment, you could use:

> +#
> +# @device: The name of the special file for the device,
> +#          i.e. /dev/i2c-0 on linux
> +# @address: The address of the i2c device on the host.
> +#
> +##

Missing a 'Since: 4.1' line.

> +{ 'struct': 'ChardevI2c',
> +  'data': { 'device': 'str',
> +            'address': 'int16'},
> +  'base': 'ChardevCommon'}

'if': 'defined(CONFIG_LINUX)'

as part of the usage of this struct, so that introspection will only see
the struct where it can be used.

> +
>  ##
>  # @ChardevSocket:
>  #
> @@ -398,6 +413,7 @@
>    'data': { 'file': 'ChardevFile',
>              'serial': 'ChardevHostdev',
>              'parallel': 'ChardevHostdev',
> +            'i2c': 'ChardevI2c',
>              'pipe': 'ChardevHostdev',
>              'socket': 'ChardevSocket',
>              'udp': 'ChardevUdp',
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]