From: "Michael S. Tsirkin" <mst@redhat.com> To: Viresh Kumar <viresh.kumar@linaro.org> Cc: Jie Deng <jie.deng@intel.com>, linux-i2c@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org, wsa@kernel.org, wsa+renesas@sang-engineering.com, arnd@arndb.de, jasowang@redhat.com, andriy.shevchenko@linux.intel.com, yu1.wang@intel.com, shuo.a.liu@intel.com, conghui.chen@intel.com, stefanha@redhat.com, gregkh@linuxfoundation.org, vincent.guittot@linaro.org, alex.bennee@linaro.org Subject: Re: [PATCH v14] i2c: virtio: add a virtio i2c frontend driver Date: Tue, 13 Jul 2021 11:34:31 -0400 [thread overview] Message-ID: <20210713113301-mutt-send-email-mst@kernel.org> (raw) In-Reply-To: <20210709034407.xmglkgzubrztnxsg@vireshk-i7> On Fri, Jul 09, 2021 at 09:14:07AM +0530, Viresh Kumar wrote: > On 09-07-21, 10:25, 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 following the virtio protocol. > > > > The device specification can be found on > > https://lists.oasis-open.org/archives/virtio-comment/202101/msg00008.html. > > > > By following the specification, people may implement different > > backend drivers to emulate different controllers according to > > their needs. > > > > Co-developed-by: Conghui Chen <conghui.chen@intel.com> > > Signed-off-by: Conghui Chen <conghui.chen@intel.com> > > Signed-off-by: Jie Deng <jie.deng@intel.com> > > --- > > Changes v13 -> v14 > > - Put the headers in virtio_i2c.h in alphabetical order. > > - Dropped I2C_FUNC_SMBUS_QUICK support. > > - Dropped few unnecessary variables and checks. > > - Use "num" everywhere instead of num or nr, to be consistent. > > - Added few comments which make the design more clear. > > Thanks a lot for following this up so far :) > > > +static int virtio_i2c_prepare_reqs(struct virtqueue *vq, > > + struct virtio_i2c_req *reqs, > > + struct i2c_msg *msgs, int num) > > +{ > > + struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr; > > + int i; > > + > > + for (i = 0; i < num; i++) { > > + int outcnt = 0, incnt = 0; > > + > > + /* > > + * We don't support 0 length messages and so masked out > > + * I2C_FUNC_SMBUS_QUICK in virtio_i2c_func(). > > + */ > > + if (!msgs[i].len) > > + break; > > + > > + /* > > + * Only 7-bit mode supported for this moment. For the address > > + * format, Please check the Virtio I2C Specification. > > + */ > > + reqs[i].out_hdr.addr = cpu_to_le16(msgs[i].addr << 1); > > + > > + if (i != num - 1) > > + reqs[i].out_hdr.flags = cpu_to_le32(VIRTIO_I2C_FLAGS_FAIL_NEXT); > > + > > + sg_init_one(&out_hdr, &reqs[i].out_hdr, sizeof(reqs[i].out_hdr)); > > + sgs[outcnt++] = &out_hdr; > > + > > + reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1); > > + if (!reqs[i].buf) > > + break; > > + > > + sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len); > > + > > + if (msgs[i].flags & I2C_M_RD) > > + sgs[outcnt + incnt++] = &msg_buf; > > + else > > + sgs[outcnt++] = &msg_buf; > > + > > + sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr)); > > + sgs[outcnt + incnt++] = &in_hdr; > > + > > + if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) { > > + i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false); > > + break; > > + } > > + } > > + > > + return i; > > +} > > Wolfram, in case you wonder why we don't error out early as discussed earlier, > then ... > > > +static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > > + int num) > > +{ > > ... > > > + /* > > + * For the case where count < num, i.e. we weren't able to queue all the > > + * msgs, ideally we should abort right away and return early, but some > > + * of the messages are already sent to the remote I2C controller and the > > + * virtqueue will be left in undefined state in that case. We kick the > > + * remote here to clear the virtqueue, so we can try another set of > > + * messages later on. > > + */ > > ... here is the reasoning for that. > > Please see if you can still get it merged into 5.14-rc1/2. Thanks. > > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> > Tested-by: Viresh Kumar <viresh.kumar@linaro.org> Well a new driver so maybe rc2 is still ok ... Acked-by: Michael S. Tsirkin <mst@redhat.com> > -- > viresh
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com> To: Viresh Kumar <viresh.kumar@linaro.org> Cc: vincent.guittot@linaro.org, arnd@arndb.de, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, wsa@kernel.org, wsa+renesas@sang-engineering.com, linux-i2c@vger.kernel.org, stefanha@redhat.com, shuo.a.liu@intel.com, andriy.shevchenko@linux.intel.com, conghui.chen@intel.com, yu1.wang@intel.com Subject: Re: [PATCH v14] i2c: virtio: add a virtio i2c frontend driver Date: Tue, 13 Jul 2021 11:34:31 -0400 [thread overview] Message-ID: <20210713113301-mutt-send-email-mst@kernel.org> (raw) In-Reply-To: <20210709034407.xmglkgzubrztnxsg@vireshk-i7> On Fri, Jul 09, 2021 at 09:14:07AM +0530, Viresh Kumar wrote: > On 09-07-21, 10:25, 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 following the virtio protocol. > > > > The device specification can be found on > > https://lists.oasis-open.org/archives/virtio-comment/202101/msg00008.html. > > > > By following the specification, people may implement different > > backend drivers to emulate different controllers according to > > their needs. > > > > Co-developed-by: Conghui Chen <conghui.chen@intel.com> > > Signed-off-by: Conghui Chen <conghui.chen@intel.com> > > Signed-off-by: Jie Deng <jie.deng@intel.com> > > --- > > Changes v13 -> v14 > > - Put the headers in virtio_i2c.h in alphabetical order. > > - Dropped I2C_FUNC_SMBUS_QUICK support. > > - Dropped few unnecessary variables and checks. > > - Use "num" everywhere instead of num or nr, to be consistent. > > - Added few comments which make the design more clear. > > Thanks a lot for following this up so far :) > > > +static int virtio_i2c_prepare_reqs(struct virtqueue *vq, > > + struct virtio_i2c_req *reqs, > > + struct i2c_msg *msgs, int num) > > +{ > > + struct scatterlist *sgs[3], out_hdr, msg_buf, in_hdr; > > + int i; > > + > > + for (i = 0; i < num; i++) { > > + int outcnt = 0, incnt = 0; > > + > > + /* > > + * We don't support 0 length messages and so masked out > > + * I2C_FUNC_SMBUS_QUICK in virtio_i2c_func(). > > + */ > > + if (!msgs[i].len) > > + break; > > + > > + /* > > + * Only 7-bit mode supported for this moment. For the address > > + * format, Please check the Virtio I2C Specification. > > + */ > > + reqs[i].out_hdr.addr = cpu_to_le16(msgs[i].addr << 1); > > + > > + if (i != num - 1) > > + reqs[i].out_hdr.flags = cpu_to_le32(VIRTIO_I2C_FLAGS_FAIL_NEXT); > > + > > + sg_init_one(&out_hdr, &reqs[i].out_hdr, sizeof(reqs[i].out_hdr)); > > + sgs[outcnt++] = &out_hdr; > > + > > + reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1); > > + if (!reqs[i].buf) > > + break; > > + > > + sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len); > > + > > + if (msgs[i].flags & I2C_M_RD) > > + sgs[outcnt + incnt++] = &msg_buf; > > + else > > + sgs[outcnt++] = &msg_buf; > > + > > + sg_init_one(&in_hdr, &reqs[i].in_hdr, sizeof(reqs[i].in_hdr)); > > + sgs[outcnt + incnt++] = &in_hdr; > > + > > + if (virtqueue_add_sgs(vq, sgs, outcnt, incnt, &reqs[i], GFP_KERNEL)) { > > + i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], false); > > + break; > > + } > > + } > > + > > + return i; > > +} > > Wolfram, in case you wonder why we don't error out early as discussed earlier, > then ... > > > +static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > > + int num) > > +{ > > ... > > > + /* > > + * For the case where count < num, i.e. we weren't able to queue all the > > + * msgs, ideally we should abort right away and return early, but some > > + * of the messages are already sent to the remote I2C controller and the > > + * virtqueue will be left in undefined state in that case. We kick the > > + * remote here to clear the virtqueue, so we can try another set of > > + * messages later on. > > + */ > > ... here is the reasoning for that. > > Please see if you can still get it merged into 5.14-rc1/2. Thanks. > > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> > Tested-by: Viresh Kumar <viresh.kumar@linaro.org> Well a new driver so maybe rc2 is still ok ... Acked-by: Michael S. Tsirkin <mst@redhat.com> > -- > viresh _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2021-07-13 15:34 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-09 2:25 [PATCH v14] i2c: virtio: add a virtio i2c frontend driver Jie Deng 2021-07-09 2:25 ` Jie Deng 2021-07-09 3:44 ` Viresh Kumar 2021-07-09 3:44 ` Viresh Kumar 2021-07-13 15:34 ` Michael S. Tsirkin [this message] 2021-07-13 15:34 ` Michael S. Tsirkin 2021-07-13 15:38 ` Michael S. Tsirkin 2021-07-13 15:38 ` Michael S. Tsirkin 2021-07-14 2:10 ` Viresh Kumar 2021-07-14 2:10 ` Viresh Kumar 2021-07-14 8:33 ` Jie Deng 2021-07-14 8:33 ` Jie Deng 2021-07-22 5:14 ` Viresh Kumar 2021-07-22 5:14 ` Viresh Kumar 2021-07-22 6:06 ` Greg KH 2021-07-22 6:06 ` Greg KH 2021-07-22 6:11 ` Viresh Kumar 2021-07-22 6:11 ` Viresh Kumar 2021-07-22 15:35 ` Wolfram Sang 2021-07-23 2:21 ` Jie Deng 2021-07-23 2:21 ` Jie Deng 2021-07-23 2:25 ` Viresh Kumar 2021-07-23 2:25 ` Viresh Kumar 2021-09-04 20:01 ` Michael S. Tsirkin 2021-09-04 20:01 ` Michael S. Tsirkin 2021-09-06 4:43 ` Viresh Kumar 2021-09-06 4:43 ` Viresh Kumar 2021-09-06 6:40 ` Wolfram Sang 2021-09-08 2:07 ` Jie Deng 2021-09-08 2:07 ` Jie Deng
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210713113301-mutt-send-email-mst@kernel.org \ --to=mst@redhat.com \ --cc=alex.bennee@linaro.org \ --cc=andriy.shevchenko@linux.intel.com \ --cc=arnd@arndb.de \ --cc=conghui.chen@intel.com \ --cc=gregkh@linuxfoundation.org \ --cc=jasowang@redhat.com \ --cc=jie.deng@intel.com \ --cc=linux-i2c@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=shuo.a.liu@intel.com \ --cc=stefanha@redhat.com \ --cc=vincent.guittot@linaro.org \ --cc=viresh.kumar@linaro.org \ --cc=virtualization@lists.linux-foundation.org \ --cc=wsa+renesas@sang-engineering.com \ --cc=wsa@kernel.org \ --cc=yu1.wang@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.