Re: [virtio-comment] [PATCH] virtio-i2c: add the device specification

[Jie Deng <jie.deng@intel.com>]

On 2020/10/21 20:08, Michael S. Tsirkin wrote:

> On Tue, Sep 22, 2020 at 10:49:02AM +0800, Jie Deng wrote:
>
> +
> +The first bit of \field{flags} indicates whether it is a read or write request.
> +It mesns a read request if the first bit of \field{flags} is set, otherwise
>
> means

I will fix this typo. Thank you.

> > +it is a write request. The rest bits of \field{flags} are reserved.
> Do we really need this flg? write request is just a big write followed by 1
> byte read, read request is write read 1 byte read.
The "addr", "flags", "len" and "buf" are all from Linux kernel structure 
"struct i2c_msg".
This design may help the backend driver to extract "struct i2c_msg" from
"struct virtio_i2c_req" easily.

The "flags" has many bits been defined in I2C. It is no mere an 
identification for
read/write. Although, currently, we only used the first bit, we may use 
other bits
for feature extension in the future. So I think it's necessary to keep it.

Thanks.


> > +The \field{len} of the request indicates the length of the I2C message.
> > +
> > +The \field{buf} of the request contains the I2C message. If the first bit of
> > +\field{flags} is '1', the \field{buf} is written by the device and it contains
> > +the message read from the device. If the first bit of \field{flags} is '0', the
> > +\field{buf} is written by the driver and it contains the message written to the
> > +the device.
> > +
> > +The final \field{status} byte is written by the device: either VIRTIO_I2C_MSG_OK
> > +for success or VIRTIO_I2C_MSG_ERR for error.
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_I2C_MSG_OK     0
> > +#define VIRTIO_I2C_MSG_ERR    1
> > +\end{lstlisting}
> > +
> > +\drivernormative{\subsubsection}{Device Operation}{Device Types / I2C Adapter Device / Device Operation}
> > +
> > +A driver MUST set \field{addr}, \field{flags} and \field{len} before sending
> > +the message.
> > +
> > +A driver MUST place the I2C message into \field{buf} if the first bit of
> > +\field{flags} is '0'.
> > +
> > +A driver MUST NOT use the message if the final \field{status}
>
> What does "use the message" mean? What message?
I mean the "addr", "flags" , "len " and "buf " in "virtio_i2c_req" here.
I will describe it more precisely in v2.

Thanks.

> > return from the
> > +device is VIRTIO_I2C_MSG_ERR.
> returned?

I will fix this typo. Thank you.


> > +
> > +\devicenormative{\subsubsection}{Device Operation}{Device Types / I2C Adapter Device / Device Operation}
> > +
> > +A device MUST NOT change the value of \field{addr}, \field{flags} and \field{len}.
>
>
> > +
> > +A device MUST place the message into \field{buf} if the first bit of \field{flags}
> > +is '1'.
>
> again, what the message?
Here is the I2C message. I will describe it more precisely in v2.

Thanks.