On 2021/3/5 15:23, Jason Wang wrote:
+ virtqueue_kick(vq);
+
+ time_left = wait_for_completion_timeout(&vi->completion,
adap->timeout);
+ if (!time_left) {
+ dev_err(&adap->dev, "virtio i2c backend timeout.\n");
+ ret = -ETIMEDOUT;
+ goto err_unlock_free;
So if the request is finished after the timerout, all the following
request will fail, is this expected?
This is an expected behavior. If timeout happens, we don't need to
care about the requests whether
really completed by "HW" or not. Just return error and let the i2c
core to decide whether to resend.
So you need at least reinit the completion at least?
Right. Will fix it. Thank you.
+ }
+
+ ret = virtio_i2c_complete_reqs(vq, reqs, msgs, nr);
So consider driver queue N requests, can device raise interrupt if
it completes the first request? If yes, the code break, if not it
need to be clarified in the spec.
The device can raise interrupt when some requests are still not
completed though this is not a good operation.
Then you need forbid this in the spec.
Yeah, but I think we can add some description to explain this clearly
instead of forbid it directly.
In this case, the remaining requests in the vq will be ignored and
the i2c_algorithm. master_xfer will return 1 for
your example. And let the i2c core to decide whether to resend.
Acaultly I remember there's no VIRTIO_I2C_FLAGS_FAIL_NEXT in
previous versions, and after reading the spec I still don't get the
motivation for that (it may complicates both driver and device
actually).
This flag is introduced by Stefan. Please check following link for
the details
https://lists.oasis-open.org/archives/virtio-comment/202012/msg00075.html.
> We just need to make sure that once the driver adds some requests to
the
> virtqueue,
> it should complete them (either success or fail) before adding new
requests.
> I think this
> is a behavior of physical I2C adapter drivers and we can follow.
Is this a driver requirement or device? If it's the driver, the code
have already did something like this. E.g you wait for the completion
of the request and forbid new request via i2c_lock.
Thanks
The driver. VIRTIO_I2C_FLAGS_FAIL_NEXT doesn't help in Linux driver.
But I agree with Stefan that
VIRTIO is not specific to Linux so the specs design should avoid the
limitations of the current
Linux driver behavior.
