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

2021-07-22 Thread Viresh Kumar
On 23-07-21, 07:58, Viresh Kumar wrote: > Would need a spec update, which I am going to send. > > We would also need another update to spec to make the Quick thing > working. Lemme do it separately and we merge the latest version of the > driver for linux-next until then. > > I checked the code

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

2021-07-22 Thread Viresh Kumar
Hi Wolfram, On 22-07-21, 17:15, Wolfram Sang wrote: > Nope, I think you misinterpreted that. SMBUS_QUICK will not send any > byte. After the address phase (with the RW bit as data), a STOP will > immediately follow. len = 0 will ensure that. > > msgbuf0[0] is set to 'command' because every mode

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

2021-07-05 Thread Jie Deng
On 2021/7/5 20:18, Viresh Kumar wrote: On 29-06-21, 12:43, Wolfram Sang wrote: From the spec: The case when ``length of \field{write_buf}''=0, and at the same time, ``length of \field{read_buf}''=0 doesn't make any sense. I mentioned this in my first reply and to my understanding I did not

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

2021-07-05 Thread Viresh Kumar
On 29-06-21, 12:43, Wolfram Sang wrote: > > > From the spec: > > > > The case when ``length of \field{write_buf}''=0, and at the same time, > > ``length of \field{read_buf}''=0 doesn't make any sense. > > > > I mentioned this in my first reply and to my understanding I did not get > > a reply

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

2021-06-30 Thread Viresh Kumar
On 30-06-21, 16:38, 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

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

2021-06-30 Thread Arnd Bergmann
On Wed, Jun 30, 2021 at 10:09 AM Andy Shevchenko wrote: > > On Wed, Jun 30, 2021 at 09:55:49AM +0200, Arnd Bergmann wrote: > > On Wed, Jun 30, 2021 at 9:51 AM Jie Deng wrote: > > ... > > > On a related note, we are apparently still missing the bit in the virtio bus > > layer that fills in the

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

2021-06-30 Thread Andy Shevchenko
On Wed, Jun 30, 2021 at 09:55:49AM +0200, Arnd Bergmann wrote: > On Wed, Jun 30, 2021 at 9:51 AM Jie Deng wrote: ... > On a related note, we are apparently still missing the bit in the virtio bus > layer that fills in the dev->of_node pointer of the virtio device. Without > this, it is not

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

2021-06-30 Thread Arnd Bergmann
On Wed, Jun 30, 2021 at 9:51 AM Jie Deng wrote: > On 2021/6/30 15:32, Wolfram Sang wrote: > + snprintf(vi->adap.name, sizeof(vi->adap.name), "Virtio I2C Adapter"); > >>> Is there something to add so you can distinguish multiple instances? > >>> Most people want that. > >> > >> I find the

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

2021-06-30 Thread Jie Deng
On 2021/6/30 15:32, Wolfram Sang wrote: + snprintf(vi->adap.name, sizeof(vi->adap.name), "Virtio I2C Adapter"); Is there something to add so you can distinguish multiple instances? Most people want that. I find the I2C core will set a device name "i2c-%d" for this purpose, right? I

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

2021-06-30 Thread Jie Deng
On 2021/6/29 18:07, Wolfram Sang wrote: Hi, so some minor comments left: + if (!msgs[i].len) + break; I hope this can extended in the future to allow zero-length messages. If this is impossible we need to set an adapter quirk instead. Yes, we can

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

2021-06-29 Thread Viresh Kumar
On 29-06-21, 13:11, Wolfram Sang wrote: > > > The case when ``length of \field{write_buf}''=0, and at the same time, > > ``length of \field{read_buf}''=0 is called not-a-read-write request > > and result for such a request is I2C device specific. > > Obviously, I don't know much about the specs

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

2021-06-29 Thread Viresh Kumar
On 29-06-21, 12:43, Wolfram Sang wrote: > > > From the spec: > > > > The case when ``length of \field{write_buf}''=0, and at the same time, > > ``length of \field{read_buf}''=0 doesn't make any sense. > > > > I mentioned this in my first reply and to my understanding I did not get > > a reply

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

2021-06-29 Thread Viresh Kumar
On 29-06-21, 12:23, Wolfram Sang wrote: > > > > You are not emulating I2C_FUNC_SMBUS_QUICK, so you need to mask it out. > > > > What is it that we need to have to emulate it ? I did use it in my > > qemu and rust backends, not sure if this was ever sent by device I > > used for testing SMBUS

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

2021-06-29 Thread Viresh Kumar
On 29-06-21, 12:07, Wolfram Sang wrote: > > +static u32 virtio_i2c_func(struct i2c_adapter *adap) > > +{ > > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; > > You are not emulating I2C_FUNC_SMBUS_QUICK, so you need to mask it out. What is it that we need to have to emulate it ? I did use it in

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

2021-06-29 Thread Viresh Kumar
On 29-06-21, 10:30, Wolfram Sang wrote: > > >     3. It seems the I2C core takes care of locking already, so is it safy to > > remove "struct mutex lock in struct virtio_i2c"? > > Looks to me like the mutex is only to serialize calls to > virtio_i2c_xfer(). Then, it can go. The core does

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

2021-06-29 Thread Viresh Kumar
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

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

2021-06-28 Thread Viresh Kumar
I will be replying here instead of replying to each and every msg :) On 28-06-21, 16:58, Wolfram Sang wrote: > > > You can fine Viresh's vhost-user implementation at > > https://lore.kernel.org/qemu-devel/cover.1617278395.git.viresh.ku...@linaro.org/t/#m3b5044bad9769b170f505e63bd081eb27cef8db2 >

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

2021-06-28 Thread Jie Deng
On 2021/6/28 22:58, Wolfram Sang wrote: If adding support incrementally works for such an interface, this makes sense as well. So, where are we? As I understand, this v10 does not support I2C transactions (or I2C_RDWR as you said). But you want to support all clients. So, this doesn't match,

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

2021-06-28 Thread Jie Deng
On 2021/6/28 17:01, Arnd Bergmann wrote: On Mon, Jun 28, 2021 at 10:39 AM Wolfram Sang wrote: sorry for the long delay. I am not familiar with VFIO, so I had to dive into the topic a little first. I am still not seeing through it completely, so I have very high-level questions first. You

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

2021-06-28 Thread Arnd Bergmann
On Mon, Jun 28, 2021 at 12:13 PM Wolfram Sang wrote: > > > > Ok, that's what I thought. There is one corner case that I've struggled > > with though: Suppose the host has an SMBus-only driver, and the > > proposed driver exposes this as an I2C device to the guest, which > > makes it available to

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

2021-06-28 Thread Arnd Bergmann
On Mon, Jun 28, 2021 at 11:25 AM Wolfram Sang wrote: > > As far as I understand me (please clarify), implementing only the smbus > > subset would mean that we cannot communicate with all client devices, > > while implementing both would add more complexity than the lower-level > > protocol. > >

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

2021-06-28 Thread Arnd Bergmann
On Mon, Jun 28, 2021 at 10:39 AM Wolfram Sang wrote: > > sorry for the long delay. I am not familiar with VFIO, so I had to dive > into the topic a little first. I am still not seeing through it > completely, so I have very high-level questions first. You probably know this already, but just in

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

2021-05-27 Thread Jie Deng
On 2021/4/15 16:18, Wolfram Sang wrote: On Thu, Apr 15, 2021 at 04:15:07PM +0800, Jie Deng wrote: On 2021/4/15 15:28, Wolfram Sang wrote: Now that we were able to catch you, I will use the opportunity to clarify the doubts I had. - struct mutex lock in struct virtio_i2c, I don't think this

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

2021-05-11 Thread Jie Deng
On 2021/4/15 15:21, Wolfram Sang wrote: I didn't forget this. It is a very small change. I'm not sure if the maintainer Wolfram has any comments so that I can address them together in one version. Noted. I'll have a look in the next days. Hi Wolfram, Kindly reminder. Hope this patch hasn't

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

2021-04-15 Thread Jie Deng
On 2021/4/15 16:18, Wolfram Sang wrote: On Thu, Apr 15, 2021 at 04:15:07PM +0800, Jie Deng wrote: On 2021/4/15 15:28, Wolfram Sang wrote: Now that we were able to catch you, I will use the opportunity to clarify the doubts I had. - struct mutex lock in struct virtio_i2c, I don't think this

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

2021-04-15 Thread Jie Deng
On 2021/4/15 15:28, Wolfram Sang wrote: Now that we were able to catch you, I will use the opportunity to clarify the doubts I had. - struct mutex lock in struct virtio_i2c, I don't think this is required since the core takes care of locking in absence of this. This is likely correct.

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

2021-04-15 Thread Jie Deng
On 2021/4/15 14:45, Viresh Kumar wrote: On 23-03-21, 10:27, Arnd Bergmann wrote: I usually recommend the use of __maybe_unused for the suspend/resume callbacks for drivers that use SIMPLE_DEV_PM_OPS() or similar helpers that hide the exact conditions under which the functions get called. In

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

2021-04-15 Thread Jie Deng
On 2021/4/14 11:52, Viresh Kumar wrote: Is i2c/for-next the right tree to merge it ? It should be. Thanks Viresh. Hi Wolfram, Do you have any comments for this patch ? Your opinion will be important to improve this patch since you are the maintainer of I2C. Thanks, Jie

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

2021-04-15 Thread Jie Deng
On 2021/4/15 11:51, Jason Wang wrote: +    for (i = 0; i < nr; i++) { +    /* Detach the ith request from the vq */ +    req = virtqueue_get_buf(vq, ); + +    /* + * Condition (req && req == [i]) should always meet since + * we have total nr requests in the vq.

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

2021-04-14 Thread Jason Wang
在 2021/3/23 下午10:19, Jie Deng 写道: Add an I2C bus driver for virtio para-virtualization. The controller can be emulated by the backend driver in any device model software by following the virtio protocol. The device specification can be found on

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

2021-04-13 Thread Jie Deng
Hi maintainers, What's the status of this patch ? Is i2c/for-next the right tree to merge it ? Thanks, Jie On 2021/3/23 22:19, Jie Deng wrote: Add an I2C bus driver for virtio para-virtualization. The controller can be emulated by the backend driver in any device model software by

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

2021-03-24 Thread Jie Deng
On 2021/3/24 14:09, Viresh Kumar wrote: On 24-03-21, 14:05, Jie Deng wrote: Or, now that I think about it a bit more, another thing we can do here is see if virtqueue_get_buf() returns NULL, if it does then we should keep expecting more messages as it may be early interrupt. What do you say ?

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

2021-03-24 Thread Jie Deng
On 2021/3/24 12:20, Viresh Kumar wrote: On 23-03-21, 22:19, Jie Deng wrote: +static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) +{ + struct virtio_i2c *vi = i2c_get_adapdata(adap); + struct virtqueue *vq = vi->vq; + struct virtio_i2c_req

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

2021-03-23 Thread Jie Deng
On 2021/3/24 11:52, Viresh Kumar wrote: On 24-03-21, 08:53, Jie Deng wrote: On 2021/3/23 17:38, Viresh Kumar wrote: On 23-03-21, 14:31, Viresh Kumar wrote: On 23-03-21, 22:19, Jie Deng wrote: +static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) +{ +

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

2021-03-23 Thread Jie Deng
On 2021/3/23 17:27, Arnd Bergmann wrote: On Tue, Mar 23, 2021 at 9:33 AM Jie Deng wrote: On 2021/3/23 15:27, Viresh Kumar wrote: On 23-03-21, 22:19, Jie Deng wrote: +static int __maybe_unused virtio_i2c_freeze(struct virtio_device *vdev) +{ +virtio_i2c_del_vqs(vdev); +return 0; +}

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

2021-03-23 Thread Jie Deng
On 2021/3/23 17:38, Viresh Kumar wrote: On 23-03-21, 14:31, Viresh Kumar wrote: On 23-03-21, 22:19, Jie Deng wrote: +static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) +{ + struct virtio_i2c *vi = i2c_get_adapdata(adap); + struct virtqueue *vq =

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

2021-03-23 Thread Arnd Bergmann
On Tue, Mar 23, 2021 at 9:33 AM Jie Deng wrote: > > On 2021/3/23 15:27, Viresh Kumar wrote: > > > On 23-03-21, 22:19, Jie Deng wrote: > >> +static int __maybe_unused virtio_i2c_freeze(struct virtio_device *vdev) > >> +{ > >> +virtio_i2c_del_vqs(vdev); > >> +return 0; > >> +} > >> + > >>

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

2021-03-23 Thread Jie Deng
On 2021/3/23 15:27, Viresh Kumar wrote: On 23-03-21, 22:19, Jie Deng wrote: +static int __maybe_unused virtio_i2c_freeze(struct virtio_device *vdev) +{ + virtio_i2c_del_vqs(vdev); + return 0; +} + +static int __maybe_unused virtio_i2c_restore(struct virtio_device *vdev) +{ +

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

2021-03-23 Thread Jie Deng
Add an I2C bus driver for virtio para-virtualization. The controller can be emulated by the backend driver in any device model software by following the virtio protocol. The device specification can be found on https://lists.oasis-open.org/archives/virtio-comment/202101/msg8.html. By