From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: virtio-comment-return-1475-cohuck=redhat.com@lists.oasis-open.org Sender: List-Post: List-Help: List-Unsubscribe: List-Subscribe: From: Jie Deng References: <5d3d8a27893734be954ebbe560f638fbafb3429c.1603778046.git.jie.deng@intel.com> <20201027081406-mutt-send-email-mst@kernel.org> Message-ID: <752795c8-868a-add0-30c0-9d34e542140c@intel.com> Date: Wed, 28 Oct 2020 15:54:39 +0800 MIME-Version: 1.0 In-Reply-To: <20201027081406-mutt-send-email-mst@kernel.org> Subject: [virtio-comment] Re: [PATCH v3] virtio-i2c: add the device specification Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable To: "Michael S. Tsirkin" Cc: virtio-comment@lists.oasis-open.org, virtio-dev@lists.oasis-open.org List-ID: On 2020/10/27 20:20, Michael S. Tsirkin wrote: > On Tue, Oct 27, 2020 at 02:00:24PM +0800, Jie Deng wrote: >> virtio-i2c is a virtual I2C adapter device. It provides a way >> to =EF=AC=82exibly communicate with the I2C slave devices from the guest= . >> >> This patch adds the specification for this device. >> >> Signed-off-by: Jie Deng >> --- >> conformance.tex | 28 ++++++++++++++--- >> content.tex | 1 + >> virtio-i2c.tex | 94 +++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++ >> 3 files changed, 119 insertions(+), 4 deletions(-) >> create mode 100644 virtio-i2c.tex >> >> +The driver queues requests to the virtqueue, and they are used by the >> +device. The request is the representation of one segment of an I2C >> +transaction. Each request is of form: >> + >> +\begin{lstlisting} >> +struct virtio_i2c_req { >> + le16 addr; >> + le16 flags; >> + le16 len; >> + u8 buf[]; >> + u8 status; >> +}; >> +\end{lstlisting} >> + >> +The \field{addr} is the address of the I2C slave device. >> + >> +The first bit of \field{flags} indicates whether it is a read or write = request. >> +It means a read request if the first bit of \field{flags} is set, other= wise >> +it is a write request. The rest bits of \field{flags} are reserved. >> + >> +The \field{len} is the number of data bytes in the \field{buf} being re= ad from or >> +written to the I2C slave address. >> + >> +The \field{buf} of the request contains one segment of an I2C transacti= on. >> +If the first bit of \field{flags} is '1', the \field{buf} is written by= the >> +device and it contains one segment of an I2C transaction being read fro= m the >> +device. > Let's give a name to the flag then? READ I guess? > I guess this means it's an exact reverse of the write-only/read-only > flag in the descriptor? > I still think it's both a potential source of errors and a waste > to have this bit in the device struct when we have a generic one. > > How about adding some motivation to explain why it's done > like this? Hmm... It seems the description here is a bit unsatisfactory. I don't=20 mean to use this flag to play the role of that flag of the descriptor. I just want to=20 encapsulate all the i2c_msg fields into the request for I2C use. The flag in the descriptor is defined from=20 virtio perspective while the flag in this request is defined from I2C perspective. It may be necessary to reverse the cause and effect: It seems better to say "If it is a write request (write descriptor),=20 then the first bit of the flag in the request should be set to 0." than "the first bit of the flag in the request is 0, then it is a write=20 request (write descriptor)". I will try to add more description to make it looks better. Thanks. >> If the first bit of \field{flags} is '0', the \field{buf} is written >> +by the driver and it contains one segment of an I2C transaction being w= ritten >> +to the the device. > one the? > Right. Thanks for your careful review. > more detail on how are multi-segment transactions formed? > don't you need flags to start/stop? > Currently, it is designed to simply transparently transmit the "i2c_msg messages" from the frontend OS kernel to the backend. No need to tag the start/stop segment. Thanks. > >> +The final \field{status} byte is written by the device: either VIRTIO_I= 2C_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} > what if one segment in a transaction fails? The driver shall return the number of segments successfully processed. Thanks. >> +\drivernormative{\subsubsection}{Device Operation}{Device Types / I2C A= dapter Device / Device Operation} >> + >> +A driver MUST set \field{addr}, \field{flags} and \field{len} before se= nding >> +the request. >> + >> +A driver MUST place one segment of an I2C transaction into \field{buf} = if the >> +first bit of \field{flags} is '0'. >> + >> +A driver MUST NOT use \field{addr}, \field{flags}, \field{len} and \fie= ld{buf} >> +if the final \field{status} returned from the device is VIRTIO_I2C_MSG_= ERR. >> + >> +A driver MUST queue the requests in order if multiple requests are goin= g to >> +be sent at a time. >> + >> +\devicenormative{\subsubsection}{Device Operation}{Device Types / I2C A= dapter Device / Device Operation} >> + >> +A device MUST NOT change the value of \field{addr}, \field{flags} and \= field{len}. >> + >> +A device MUST place one segment of an I2C transaction into \field{buf} = if the first >> +bit of \field{flags} is '1'. >> + >> +A device MUST guarantee the requests being handled in order if multiple= requests >> +are received. >> --=20 >> 2.7.4 This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lis= ts Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/