OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: Re: [PATCH] Update virtio input device specification


On Fri, Mar 24, 2017 at 11:17:59AM +0100, Ladi Prosek wrote:
> * added VIRTIO_INPUT_CFG_ID_DEVIDS / virtio_input_devids
> * added normative statements and moved them all to
>   \devicenormative and \drivernormative sections
> * minor tweaks
> 
> Signed-off-by: Ladi Prosek <lprosek@redhat.com>

Thanks for taking this up!

I think you should start sending non-incremental
patches eventually. Soe comments below.


> ---
> 
> This patch is based on Gerd's 
>   https://www.kraxel.org/cgit/virtio-spec/log/?h=virtio-input
> 
> It is addressing feedback given in
>   https://lists.oasis-open.org/archives/virtio-dev/201605/msg00003.html
> and adding some more content.
> 
> Thanks!
> Ladi
> 
> 
>  conformance.tex  | 18 ++++++++++++
>  virtio-input.tex | 87 ++++++++++++++++++++++++++++++++++++++++++++------------
>  2 files changed, 87 insertions(+), 18 deletions(-)
> 
> diff --git a/conformance.tex b/conformance.tex
> index 7b7df32..2486e9a 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -145,6 +145,15 @@ An SCSI host driver MUST conform to the following normative statements:
>  \item \ref{drivernormative:Device Types / SCSI Host Device / Device Operation / Device Operation: eventq}
>  \end{itemize}
>  
> +\subsection{Input Driver Conformance}\label{sec:Conformance / Driver Conformance / Input Driver Conformance}
> +
> +An input driver MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{drivernormative:Device Types / Input Device / Device Initialization}
> +\item \ref{drivernormative:Device Types / Input Device / Device Operation}
> +\end{itemize}
> +
>  \section{Device Conformance}\label{sec:Conformance / Device Conformance}
>  
>  A device MUST conform to the following normative statements:
> @@ -265,6 +274,15 @@ An SCSI host device MUST conform to the following normative statements:
>  \item \ref{devicenormative:Device Types / SCSI Host Device / Device Operation / Device Operation: eventq}
>  \end{itemize}
>  
> +\subsection{Input Device Conformance}\label{sec:Conformance / Device Conformance / Input Device Conformance}
> +
> +An input device MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{devicenormative:Device Types / Input Device / Device Initialization}
> +\item \ref{devicenormative:Device Types / Input Device / Device Operation}
> +\end{itemize}
> +
>  \section{Legacy Interface: Transitional Device and
>  Transitional Driver Conformance}\label{sec:Conformance / Legacy
>  Interface: Transitional Device and 
> diff --git a/virtio-input.tex b/virtio-input.tex
> index 9151880..eb3e154 100644
> --- a/virtio-input.tex
> +++ b/virtio-input.tex
> @@ -1,7 +1,8 @@
>  \section{Input Device}\label{sec:Device Types / Input Device}
>  
>  The virtio input device can be used to create virtual human interface
> -devices such as keyboards, mice and tablets.  It basically sends linux
> +devices such as keyboards, mice and tablets. An instance of the virtio
> +device represents one such input device. It sends Linux
>  input layer events over virtio.
>  See \href{https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/input.h}{include/uapi/linux/input.h}

which parts of the above are of interest?

>  and \href{https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/input-event-codes.h}{include/uapi/linux/input-event-codes.h}

One thing missing is how should driver handle events
it does not recognize.

Also, is there a chance device will be interested in knowing
which events does driver recognize?


> @@ -32,6 +33,7 @@ enum virtio_input_config_select {
>  	VIRTIO_INPUT_CFG_UNSET      = 0x00,
>  	VIRTIO_INPUT_CFG_ID_NAME    = 0x01,
>  	VIRTIO_INPUT_CFG_ID_SERIAL  = 0x02,
> +	VIRTIO_INPUT_CFG_ID_DEVIDS  = 0x03,
>  	VIRTIO_INPUT_CFG_PROP_BITS  = 0x10,
>  	VIRTIO_INPUT_CFG_EV_BITS    = 0x11,
>  	VIRTIO_INPUT_CFG_ABS_INFO   = 0x12,
> @@ -44,6 +46,13 @@ struct virtio_input_absinfo {
>  	le32  flat;
>  };
>  
> +struct virtio_input_devids {
> +	le16  bustype;
> +	le16  vendor;
> +	le16  product;
> +	le16  version;
> +};
> +
>  struct virtio_input_config {
>  	u8    select;
>  	u8    subsel;
> @@ -53,44 +62,53 @@ struct virtio_input_config {
>  		char string[128];
>  		u8   bitmap[128];
>  		struct virtio_input_absinfo abs;
> +		struct virtio_input_devids ids;
>  	} u;
>  };
>  \end{lstlisting}
>  
> -To query a specific piece of information the driver MUST set
> -\field{select} and \field{subsel} accordingly, then check \field{size}
> -to see and how much information is available.  \field{size} can be
> +To query a specific piece of information the driver sets
> +\field{select} and \field{subsel} accordingly, then checks \field{size}
> +to see how much information is available.  \field{size} can be
>  zero if no information is available.  Strings do not include a
> -terminating NUL byte.
> +NUL terminator.
>  
>  \begin{description}
>  
>  \item[VIRTIO_INPUT_CFG_ID_NAME]
> -\field{subsel} is not used and MUST be zero.
> +\field{subsel} is zero.
>  Returns the name of the device, in \field{u.string}.
>  
> -Same as EVIOCGNAME ioctl for linux evdev devices.
> +Same as EVIOCGNAME ioctl for Linux evdev devices.
>  
>  \item[VIRTIO_INPUT_CFG_ID_SERIAL]
> -\field{subsel} is not used and MUST be zero.
> +\field{subsel} is zero.
>  Returns the serial number of the device, in \field{u.string}.
>  
> +\item[VIRTIO_INPUT_CFG_ID_DEVIDS]
> +\field{subsel} is zero.
> +Returns ID information of the device, in \field{u.ids}.
> +
> +Same as EVIOCGID ioctl for Linux evdev devices.
> +

Above line confuses more than it clarifies IMHO.

Looks like that ioctl returns
 42 struct input_id {
 43         __u16 bustype;
 44         __u16 vendor;
 45         __u16 product;
 46         __u16 version;
 47 };

Hos is this the same? Maybe "similar to"?


>  \item[VIRTIO_INPUT_CFG_PROP_BITS]
> -\field{subsel} is not used and MUST be zero.
> +\field{subsel} is zero.
>  Returns input properties (INPUT_PROP_*) of the device, in \field{u.bitmap}.
>  
> +Same as EVIOCGPROP ioctl for Linux evdev devices.
> +

Above line confuses more than it clarifies IMHO.

Referred to headers contain no information about this ioctl.

>  \item[VIRTIO_INPUT_CFG_EV_BITS]
>  \field{subsel} specifies the event type (EV_*).  If \field{size} is
> -non-zero the event type is supported and a bitmap the of supported
> +non-zero the event type is supported and a bitmap of supported
>  event codes is returned in \field{u.bitmap}.
>  
> -Same as EVIOCGBIT ioctl.
> +Same as EVIOCGBIT ioctl for Linux evdev devices.
>  
>  \item[VIRTIO_INPUT_CFG_ABS_INFO]
>  \field{subsel} specifies the absolute axes (ABS_*).
> -Informations about the axis will be returned in \field{u.abs}.
> +Information about the axis will be returned in \field{u.abs}.
>  
> -Same as EVIOCGABS ioctl.
> +Same as EVIOCGABS ioctl for Linux evdev devices.
>  
>  \end{description}
>  
> @@ -101,19 +119,33 @@ Same as EVIOCGABS ioctl.
>  \item The eventq is populated with receive buffers.
>  \end{enumerate}
>  
> +\drivernormative{\subsubsection}{Device Initialization}{Device Types / Input Device / Device Initialization}
> +A driver MUST set both \field{select} and \field{subsel} when querying
> +  device configuration, in any order.
> +
> +A driver MUST NOT write to configuration fields other than \field{select}
> +  and \field{subsel}.
> +
> +A driver SHOULD check the \field{size} field before accessing the
> +  configuration information.
> +
> +\devicenormative{\subsubsection}{Device Initialization}{Device Types / Input Device / Device Initialization}
> +A device MUST set the \field{size} field to zero if it doesn't support a
> +  given \field{select} and \field{subsel} combination.
> +
>  \subsection{Device Operation}\label{sec:Device Types / Input Device / Device Operation}
>  
>  \begin{enumerate}
>  \item Input events such as press and release events for keys and
> -  buttons and motion events are sent from the device to the driver
> -  using the eventq.
> -\item Status feedback such as keyboard led updates are sent from the
> +  buttons, and motion events for pointing devices are sent from
> +  the device to the driver using the eventq.
> +\item Status feedback such as keyboard LED updates are sent from the
>    driver to the device using the statusq.
>  \item Both queues use the same virtio_input_event struct.
>    \field{type}, \field{code} and \field{value} are filled according to
> -  the linux input layer (evdev) interface, except that the fields are
> +  the Linux input layer (evdev) interface, except that the fields are
>    in little endian byte order whereas the evdev ioctl interface uses
> -  native endian.
> +  native endian-ness.
>  \end{enumerate}
>  
>  \begin{lstlisting}
> @@ -123,3 +155,22 @@ struct virtio_input_event {
>  	le32 value;
>  };
>  \end{lstlisting}
> +
> +\drivernormative{\subsubsection}{Device Operation}{Device Types / Input Device / Device Operation}
> +
> +A driver SHOULD keep the eventq populated with buffers. These buffers
> +  MUST be device-writable and MUST be at least the size of
> +  struct virtio_input_event.
> +
> +Buffers placed into the statusq by a driver MUST be at least the size
> +  of struct virtio_input_event.
> +
> +\devicenormative{\subsubsection}{Device Operation}{Device Types / Input Device / Device Operation}
> +
> +A device MAY drop input events if the eventq does not have enough
> +  available buffers. It SHOULD NOT drop individual input events if
> +  they are part of a sequence forming one input device update. For
> +  example, a pointing device update typically consists of several
> +  input events, one for each axis, and a terminating EV_SYN event.
> +  A device SHOULD either buffer or drop the entire sequence.
> +
> -- 
> 2.7.4


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]