Re: [PATCH v10] i2c: virtio: add a virtio i2c frontend driver

From: Viresh Kumar <viresh.kumar@linaro.org>
To: "Wolfram Sang" <wsa@kernel.org>, "Arnd Bergmann" <arnd@arndb.de>,
	"Jie Deng" <jie.deng@intel.com>,
	"Linux I2C" <linux-i2c@vger.kernel.org>,
	virtualization@lists.linux-foundation.org,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	conghui.chen@intel.com, kblaiech@mellanox.com,
	jarkko.nikula@linux.intel.com,
	"Sergey Semin" <Sergey.Semin@baikalelectronics.ru>,
	"Mike Rapoport" <rppt@kernel.org>,
	loic.poulain@linaro.org, "Tali Perry" <tali.perry1@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Bjorn Andersson" <bjorn.andersson@linaro.org>,
	yu1.wang@intel.com, shuo.a.liu@intel.com,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [PATCH v10] i2c: virtio: add a virtio i2c frontend driver
Date: Tue, 29 Jun 2021 14:22:13 +0530	[thread overview]
Message-ID: <20210629085213.7a7eqcgkmtk5y7nh@vireshk-i7> (raw)
In-Reply-To: <YNrZVho/98qgJS9N@kunai>

On 29-06-21, 10:27, Wolfram Sang wrote:
> > While we are at it, this has been replaced by a Rust counterpart [1]
> > (as that makes it hypervisor agnostic, which is the goal of my work
> > here) and I need someone with I2C knowledge to help review it. It
> > should be okay even if you don't understand Rust a lot, just review
> > this file[2] which is where most of i2c specific stuff lies.
> 
> Can't promise I can do this before my holidays, but I will try.

Thanks.

> > I am not sure why you say I2C_RDWR isn't supported. The spec and Linux
> 
> This is how I interpreted Arnd's response. I said mulitple times that I
> might be missing something so I double check.
> 
> > SMBUS. To clarify on an earlier point, every virtio transfer may
> > contain one or more struct i2c_msg instances, all processed together
> > (as expected).
> 
> That was the information missing for me so far becasue...
> 
> > If you see virtio_i2c_send_reqs() in this patch, you will see that it
> > converts a stream of i2c_req messages to their virtio counterparts and
> > send them together, consider it a single transaction.
> 
> ... when I checked virtio_i2c_send_reqs(), I also saw
> virtqueue_add_sgs() but I had no idea if this will end up as REP_START
> on the physical bus or not. But it definately should.

Just think of virtqueue_add_sgs() as something that setups the
structures for transfer. The actual stuff at the other end (host)
happens only after virtqueue_kick() is called at the guest (this
notifies the host that data is present now), in response the backend
running at host will re-create the struct i2c_msg and issue:

    struct i2c_rdwr_ioctl_data data;
    data.nmsgs = count;
    data.msgs = msgs;

    return ioctl(adapter->fd, I2C_RDWR, &data);

So we will end up recreating the exact situation as when
virtio_i2c_xfer() is called.

-- 
viresh