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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.








[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux