Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver

[Viresh Kumar]

On 25-03-21, 13:09, Jie Deng wrote:
> 
> On 2021/3/24 15:33, Viresh Kumar wrote:
> > +
> > +/* Definitions from virtio-i2c specifications */
> > +#define VHOST_USER_I2C_MAX_QUEUES       1
> > +
> > +/* Status */
> > +#define VIRTIO_I2C_MSG_OK               0
> > +#define VIRTIO_I2C_MSG_ERR              1
> > +
> > +/* The bit 0 of the @virtio_i2c_out_hdr.@flags, used to group the requests 
> > */
> > +#define VIRTIO_I2C_FLAGS_FAIL_NEXT      0x00000001
> > +
> > +/**
> > + * struct virtio_i2c_out_hdr - the virtio I2C message OUT header
> > + * @addr: the controlled device's address
> > + * @padding: used to pad to full dword
> > + * @flags: used for feature extensibility
> > + */
> > +struct virtio_i2c_out_hdr {
> > +    uint16_t addr;
> > +    uint16_t padding;
> > +    uint32_t flags;
> > +} __attribute__((packed));
> 
> 
> __le16,  __le32 ?

Maybe, but I didn't do them because of this:

docs/devel/style.rst:

"Don't use Linux kernel internal types like u32, __u32 or __le32."
 
> > +
> > +/**
> > + * struct virtio_i2c_in_hdr - the virtio I2C message IN header
> > + * @status: the processing result from the backend
> > + */
> > +struct virtio_i2c_in_hdr {
> > +    uint8_t status;
> > +} __attribute__((packed));
> > +
> 
> I understand these definitions can be removed once the frontend driver is
> merged by the Linux ?

Yes, we would be required to somehow include the uapi header that
kernel is adding and then this won't be required.
 
> > +/* vhost-user-i2c definitions */
> > +
> > +#ifndef container_of
> > +#define container_of(ptr, type, member) ({                      \
> > +        const typeof(((type *) 0)->member) *__mptr = (ptr);     \
> > +        (type *) ((char *) __mptr - offsetof(type, member));})
> > +#endif
> 
> 
> This seems to be a general interface.  I see there is a definition in
> qemu/compiler.h.
> 
> Can we reuse it ?

Damn. My bad (maybe not). I picked this part from the RPMB patchset
that Alex sent and didn't bother looking for it.

Though on the other hand, we are looking to make this file independent
of qemu so it can be used by other hypervisors without any (or much)
modifications, and maybe so it was done so.

Alex ?

-- 
viresh