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

I will be replying here instead of replying to each and every msg :)

On 28-06-21, 16:58, Wolfram Sang wrote:

quoted

You can fine Viresh's vhost-user implementation at
https://lore.kernel.org/qemu-devel/cover.1617278395.git.viresh.kumar@linaro.org/t/#m3b5044bad9769b170f505e63bd081eb27cef8db2 (local)

It looks OK so far; yet, it is not complete. But it might be bearable
in the end.

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.

quoted

As you say, it does get a bit clumsy, but I think there is also a good argument
to be made that the clumsiness is based on the host Linux user interface
more than the on the requirements of the physical interface,
and that should not have to be reflected in the virtio specification.

Makes sense to me.

quoted

Right, this one has come up before as well: the preliminary result
was to assume that this probably won't be needed, but would be easy
enough to add later if necessary.

If adding support incrementally works for such an interface, this makes
sense as well.

Yes, we don't support few of SMBUS transaction (the block ones) as you
specified.

So, where are we?

The virtio specification is already merged and here is the latest
version [3].

As I understand, this v10 does not support I2C transactions (or
I2C_RDWR as you said).

I am not sure why you say I2C_RDWR isn't supported. The spec and Linux
driver (+ my Rust/qemu backend), they all support I2C_RDWR as well as
SMBUS. To clarify on an earlier point, every virtio transfer may
contain one or more struct i2c_msg instances, all processed together
(as expected).

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.

But you want to support all clients. So, this doesn't match, or?

-- 
viresh

[1] https://github.com/rust-vmm/vhost-device/pull/1
[2] https://github.com/rust-vmm/vhost-device/blob/5aa22c92faac84ab07b6b15a214513556e8b1d01/src/i2c/src/i2c.rs
[3] https://github.com/oasis-tcs/virtio-spec/blob/master/virtio-i2c.tex
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization