[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V2 4/6] tools/vhost-user-i2c: Add backend driver
From: |
Viresh Kumar |
Subject: |
Re: [PATCH V2 4/6] tools/vhost-user-i2c: Add backend driver |
Date: |
Mon, 5 Apr 2021 10:58:44 +0530 |
User-agent: |
NeoMutt/20180716-391-311a52 |
On 02-04-21, 10:55, Jie Deng wrote:
>
> On 2021/4/1 20:12, Viresh Kumar wrote:
> > +
> > +/* vhost-user-i2c definitions */
> > +
> > +#define MAX_I2C_VDEV (1 << 7)
> > +#define MAX_I2C_ADAPTER 16
>
> Generally speaking, 16 is big enough for most cases. But comparing with
> static configuration,
> I think it is better if we can check how many adapters in the host when
> doing initialization and
> use that number as "MAX_I2C_ADAPTER".
Hmm, so this doesn't limit the adapter number in /dev/i2cX (i.e. we can still
process /dev/i2c99), but rather how many adapters we can pass in
--device-list="" parameter. And so I feel 16 is big enough and if someone really
needs more, then they can update the macro here.
> > +static VI2cAdapter *vi2c_create_adapter(int32_t bus, uint16_t
> > client_addr[],
> > + int32_t n_client)
> > +{
> > + VI2cAdapter *adapter;
> > + char path[20];
> > + uint64_t funcs;
> > + int32_t fd, i;
> > +
> > + if (bus < 0) {
> > + return NULL;
> > + }
> > +
> > + adapter = g_malloc0(sizeof(*adapter));
> > + if (!adapter) {
> > + g_printerr("failed to alloc adapter");
> > + return NULL;
> > + }
> > +
> > + snprintf(path, sizeof(path), "/dev/i2c-%d", bus);
> > + path[sizeof(path) - 1] = '\0';
> > +
> > + fd = open(path, O_RDWR);
> > + if (fd < 0) {
> > + g_printerr("virtio_i2c: failed to open %s\n", path);
> > + goto fail;
> > + }
> > +
> > + adapter->fd = fd;
> > + adapter->bus = bus;
> > +
> > + if (ioctl(fd, I2C_FUNCS, &funcs) < 0) {
> > + g_printerr("virtio_i2c: failed to get functionality %s: %d\n",
> > path,
> > + errno);
> > + goto close_fd;
> > + }
> > +
> > + if (funcs & I2C_FUNC_I2C) {
> > + adapter->smbus = false;
> > + } else if (funcs & I2C_FUNC_SMBUS_WORD_DATA) {
>
>
> Only I2C_FUNC_SMBUS_WORD_DATA is checked here. But in addition to it, the
> smbus_xfer
> seems support I2C_FUNC_SMBUS_BYTE, I2C_FUNC_SMBUS_BYTE_DATA. So if an
> adapter only
> support the latter two, it will never go to smbus_xfer.
Yeah, I missed it. Thanks. I am updating it as:
- } else if (funcs & I2C_FUNC_SMBUS_WORD_DATA) {
+ } else if (funcs & (I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BYTE |
+ I2C_FUNC_SMBUS_BYTE_DATA)) {
--
viresh
- [PATCH V2 0/6] virtio: Implement generic vhost-user-i2c backend, Viresh Kumar, 2021/04/01
- [PATCH V2 1/6] !Merge: Update virtio headers from kernel, Viresh Kumar, 2021/04/01
- [PATCH V2 3/6] hw/virtio: add vhost-user-i2c-pci boilerplate, Viresh Kumar, 2021/04/01
- [PATCH V2 2/6] hw/virtio: add boilerplate for vhost-user-i2c device, Viresh Kumar, 2021/04/01
- [PATCH V2 4/6] tools/vhost-user-i2c: Add backend driver, Viresh Kumar, 2021/04/01
- [PATCH V2 5/6] docs: add a man page for vhost-user-i2c, Viresh Kumar, 2021/04/01
- [PATCH V2 6/6] MAINTAINERS: Add entry for virtio-i2c, Viresh Kumar, 2021/04/01