Stefan Hajnoczi
2021-Mar-02 09:51 UTC
[PATCH v5] i2c: virtio: add a virtio i2c frontend driver
On Tue, Mar 02, 2021 at 10:42:06AM +0800, Jie Deng wrote:> > On 2021/3/1 23:19, Arnd Bergmann wrote: > > On Mon, Mar 1, 2021 at 7:41 AM Jie Deng <jie.deng at intel.com> wrote: > > > > > --- /dev/null > > > +++ b/include/uapi/linux/virtio_i2c.h > > > @@ -0,0 +1,56 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-or-later WITH Linux-syscall-note */The uapi VIRTIO header files are BSD licensed so they can be easily used by other projects (including other operating systems and hypervisors that don't use Linux). Please see the license headers in <linux/virtio_net.h> or <linux/virtio_blk.h>.> > > +/* > > > + * Definitions for virtio I2C Adpter > > > + * > > > + * Copyright (c) 2021 Intel Corporation. All rights reserved. > > > + */ > > > + > > > +#ifndef _UAPI_LINUX_VIRTIO_I2C_H > > > +#define _UAPI_LINUX_VIRTIO_I2C_H > > Why is this a uapi header? Can't this all be moved into the driver > > itself?Linux VIRTIO drivers provide a uapi header with structs and constants that describe the device interface. This allows other software like QEMU, other operating systems, etc to reuse these headers instead of redefining everything. These files should contain: 1. Device-specific feature bits (VIRTIO_<device>_F_<feature>) 2. VIRTIO Configuration Space layout (struct virtio_<device>_config) 3. Virtqueue request layout (struct virtio_<device>_<req/resp>) For examples, see <linux/virtio_net.h> and <linux/virtio_blk.h>.> > > +/** > > > + * struct virtio_i2c_req - the virtio I2C request structure > > > + * @out_hdr: the OUT header of the virtio I2C message > > > + * @write_buf: contains one I2C segment being written to the device > > > + * @read_buf: contains one I2C segment being read from the device > > > + * @in_hdr: the IN header of the virtio I2C message > > > + */ > > > +struct virtio_i2c_req { > > > + struct virtio_i2c_out_hdr out_hdr; > > > + u8 *write_buf; > > > + u8 *read_buf; > > > + struct virtio_i2c_in_hdr in_hdr; > > > +}; > > In particular, this structure looks like it is only ever usable between > > the transfer functions in the driver itself, it is shared with neither > > user space nor the virtio host side.I agree. This struct is not part of the device interface. It's part of the Linux driver implementation. This belongs inside the driver code and not in include/uapi/ where public headers are located. Stefan -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20210302/f801cc42/attachment.sig>
Arnd Bergmann
2021-Mar-02 10:54 UTC
[PATCH v5] i2c: virtio: add a virtio i2c frontend driver
On Tue, Mar 2, 2021 at 10:51 AM Stefan Hajnoczi <stefanha at redhat.com> wrote:> On Tue, Mar 02, 2021 at 10:42:06AM +0800, Jie Deng wrote: > > > > +/* > > > > + * Definitions for virtio I2C Adpter > > > > + * > > > > + * Copyright (c) 2021 Intel Corporation. All rights reserved. > > > > + */ > > > > + > > > > +#ifndef _UAPI_LINUX_VIRTIO_I2C_H > > > > +#define _UAPI_LINUX_VIRTIO_I2C_H > > > Why is this a uapi header? Can't this all be moved into the driver > > > itself? > > Linux VIRTIO drivers provide a uapi header with structs and constants > that describe the device interface. This allows other software like > QEMU, other operating systems, etc to reuse these headers instead of > redefining everything. > > These files should contain: > 1. Device-specific feature bits (VIRTIO_<device>_F_<feature>) > 2. VIRTIO Configuration Space layout (struct virtio_<device>_config) > 3. Virtqueue request layout (struct virtio_<device>_<req/resp>) > > For examples, see <linux/virtio_net.h> and <linux/virtio_blk.h>.Ok, makes sense. So it's not strictly uapi but just helpful for virtio applications to reference these. I suppose it is slightly harder when building qemu on other operating systems though, how does it get these headers on BSD or Windows? Arnd