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

[Ernest Esene <eroken1@gmail.com>]

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

On Tue, May 07, 2019 at 07:33:09PM +0200, Markus Armbruster wrote:
> Ernest Esene <eroken1@gmail.com> writes:
> 
> > 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>
> 
> Could you explain briefly how passing through a host's I2C device can be
> useful?
QEMU supports emulation of I2C devices in software but currently can't
passthrough to real I2C devices. This feature is needed by developers
using QEMU for writing and testing software for I2C devices.

> 
> Any particular reason not to use GPLv2+?
No, I used the wrong script. I'll update the licence.

> > +
> > +        if (addr > CHR_I2C_ADDR_7BIT_MAX) {
> > +            /*
> > +             * TODO: check if adapter support 10-bit addr
> > +             * I2C_FUNC_10BIT_ADDR
> > +             */
> 
> What's the impact of not having done this TODO?
Not all I2C adapters supports 10-bit address.
> Should it be mentioned in the commit message?
I have handled it already.

> > +        return;
> > +    }
> > +    qemu_set_block(fd);
> 
> Sure we want *blocking* I/O?  No other character device does.
No, it is a mistake.
> 
> > +    qemu_chr_open_fd(chr, fd, fd);
> > +    addr = (void *) (long) i2c->address;
> 
> 
> Why not make option "addr" QEMU_OPT_NUMBER and use
> qemu_opt_get_number()?
I never knew QEMU_OPT_NUMBER can handle inputs such: "0x08 and 8",
appropriately.
> 
> Missing: documentation update for qemu-options.hx.
I don't know much about this format (.hx), I'll be happy to have any
useful documentation on this.

Thank you.
-Ernest Esene

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