On 2021/3/12 14:10, Viresh Kumar wrote:
I saw your email about wrong version being sent, I already wrote some
reviews. Sending them anyway for FWIW :)
On 12-03-21, 21:33, Jie Deng wrote:
+struct virtio_i2c {
+ struct virtio_device *vdev;
+ struct completion completion;
+ struct i2c_adapter adap;
+ struct mutex lock;
As I said in the previous version (Yes, we were both waiting for
Wolfram to answer that), this lock shouldn't be required at all.
And since none of us have a use-case at hand where we will have a
problem without this lock, we should really skip it. We can always
come back and add it if we find an issue somewhere. Until then, better
to keep it simple.
The problem is you can't guarantee that adap->algo->master_xfer
is only called from i2c_transfer. Any function who holds the adapter can
call
adap->algo->master_xfer directly. I prefer to avoid potential issues
rather than
find a issue then fix.
+
+static struct i2c_adapter virtio_adapter = {
+ .owner = THIS_MODULE,
+ .name = "Virtio I2C Adapter",
+ .class = I2C_CLASS_DEPRECATED,
What happened to this discussion ?
https://lore.kernel.org/lkml/20210305072903.wtw645rukmqr5hx5@vireshk-i7/
My understanding is that new driver sets this to warn users that the
adapter doesn't support classes anymore.
I'm not sure if Wolfram can explain it more clear for you.
+ .algo = &virtio_algorithm,
+
+ return ret;
+
+ vi->adap = virtio_adapter;
This is strange, why are you allocating memory for adapter twice ?
Once for virtio_adapter and once for vi->adap ? Either fill the fields
directly for v->adap here and remove virtio_adapter or make vi->adap a
pointer.
Will make vi->adap a pointer. Thanks.
