[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ff4357f3-93f1-cd86-a1da-01dcb144e52b@infradead.org>
Date: Mon, 7 Dec 2020 15:03:23 -0800
From: Randy Dunlap <rdunlap@...radead.org>
To: "Enrico Weigelt, metux IT consult" <info@...ux.net>,
linux-kernel@...r.kernel.org
Cc: corbet@....net, linus.walleij@...aro.org,
bgolaszewski@...libre.com, mst@...hat.com, jasowang@...hat.com,
linux-doc@...r.kernel.org, linux-gpio@...r.kernel.org,
virtualization@...ts.linux-foundation.org,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v3] drivers: gpio: add virtio-gpio guest driver
On 12/7/20 12:48 PM, Enrico Weigelt, metux IT consult wrote:
> Introducing new GPIO driver for virtual GPIO devices via virtio.
Introduce
>
> The driver allows routing GPIO control into VM guests, eg. brigding
> virtual gpios to specific host gpios, or attaching simulators for
> automatic application testing.
>
These ...
> Changes v3:
> * spec: fixed type names
> * spec: replace "host"/"guest" by "device"/"cpu"
> * spec: change terminology from "events" to "messages"
> * driver: fixed missing can_sleep flag
> * driver: select VIRTIO instead of depends on
> * driver: drop references to qemu in Kconfig
> * driver: fixed incomplete error handling and possible deadlock
> in case of sending buf failed
> * driver: dropped unneeded WARN_ON
> * driver: fixed retval of virtio_gpio_xmit()
> * driver: dynamically allocate virtio buffers
> * driver: added locking on gpio operations
> * driver: added irq_chip functions
>
> Changes v2:
> * uapi: fixed header license
> * driver: sorted include's
> * driver: fixed formatting
> * driver: fixed unneeded devm allocation - plain kzalloc/kfree is enough
> * driver: fixed missing devm_kzalloc fail check
> * driver: use devm_kcalloc() for array allocation
> * spec: added virtio-gpio protocol specification
... Vx change descriptions go after the following "---" line.
I usually put them before the diffstat, with one blank line
separating them.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@...ux.net>
> ---
> Documentation/gpio/virtio-gpio.rst | 176 +++++++++++++++++
> MAINTAINERS | 6 +
> drivers/gpio/Kconfig | 7 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-virtio.c | 381 +++++++++++++++++++++++++++++++++++++
> include/uapi/linux/virtio_gpio.h | 39 ++++
> include/uapi/linux/virtio_ids.h | 1 +
> 7 files changed, 611 insertions(+)
> create mode 100644 Documentation/gpio/virtio-gpio.rst
> create mode 100644 drivers/gpio/gpio-virtio.c
> create mode 100644 include/uapi/linux/virtio_gpio.h
>
> diff --git a/Documentation/gpio/virtio-gpio.rst b/Documentation/gpio/virtio-gpio.rst
> new file mode 100644
> index 000000000000..e7bf01ec1ce7
> --- /dev/null
> +++ b/Documentation/gpio/virtio-gpio.rst
> @@ -0,0 +1,176 @@
> +"""""""""""""""""
> +Virtio-GPIO protocol specification
> +"""""""""""""""""
> +...........
> +Specification for virtio-based virtiual GPIO devices
virtual
although it seems redundant.
> +...........
> +
> ++------------
> ++Version_ 1.0
> ++------------
> +
> +===================
> +General
> +===================
> +
> +The virtio-gpio protocol provides access to general purpose IO devices
> +to virtual machine guests. These virtualized GPIOs could be either provided
in (?)
> +by some simulator (eg. virtual HIL), routed to some external device or
(e.g.
> +routed to real GPIOs on the host (eg. virtualized embedded applications).
(e.g.
> +
> +Instead of simulating some existing real GPIO chip within an VMM, this
within a
and why not just "VM"?
> +protocol provides an hardware independent interface between host and guest
a hardware
> +that solely relies on an active virtio connection (no matter which transport
> +actually used), no other buses or additional platform driver logic required.
> +
> +===================
> +Protocol layout
> +===================
> +
> +----------------------
> +Configuration space
> +----------------------
> +
> ++--------+----------+-------------------------------+
> +| Offset | Type | Description |
> ++========+==========+===============================+
> +| 0x00 | u8 | version |
> ++--------+----------+-------------------------------+
> +| 0x02 | u16 | number of GPIO lines |
> ++--------+----------+-------------------------------+
> +| 0x04 | u32 | size of gpio name block |
> ++--------+----------+-------------------------------+
> +| 0x20 | char[32] | device name (0-terminated) |
> ++--------+----------+-------------------------------+
> +| 0x40 | char[] | line names block |
> ++--------+----------+-------------------------------+
> +
> +- for version field currently only value 1 supported.
> +- the line names block holds a stream of zero-terminated strings,
> + holding the individual line names.
> +- unspecified fields are reserved for future use and should be zero.
> +
> +------------------------
> +Virtqueues and messages:
> +------------------------
> +
> +- Queue #0: transmission from device to cpu
CPU
> +- Queue #1: transmission from cpu to device
CPU
> +
> +The queues transport messages of the struct virtio_gpio_msg:
> +
> +Message format:
> +---------------
> +
> ++--------+----------+---------------+
> +| Offset | Type | Description |
> ++========+==========+===============+
> +| 0x00 | uint16 | event type |
> ++--------+----------+---------------+
> +| 0x02 | uint16 | line id |
> ++--------+----------+---------------+
> +| 0x04 | uint32 | value |
> ++--------+----------+---------------+
> +
> +Message types:
> +--------------
Do the Message types go into the "event type" field above?
Please clarify.
> +
> ++-------+----------------------------------------+-----------------------------+
> +| Code | Symbol | |
> ++=======+========================================+=============================+
> +| 0x01 | VIRTIO_GPIO_MSG_GUEST_REQUEST | request gpio line |
> ++-------+---------------------------------------+-----------------------------+
> +| 0x02 | VIRTIO_GPIO_MSG_GUEST_DIRECTION_INPUT | set direction to input |
> ++-------+---------------------------------------+-----------------------------+
> +| 0x03 | VIRTIO_GPIO_MSG_GUEST_DIRECTION_OUTPUT | set direction to output |
> ++-------+---------------------------------------+-----------------------------+
> +| 0x04 | VIRTIO_GPIO_MSG_GUEST_GET_DIRECTION | read current direction |
> ++-------+---------------------------------------+-----------------------------+
> +| 0x05 | VIRTIO_GPIO_MSG_GUEST_GET_VALUE | read current level |
> ++-------+---------------------------------------+-----------------------------+
> +| 0x06 | VIRTIO_GPIO_MSG_GUEST_SET_VALUE | set current (out) level |
> ++-------+---------------------------------------+-----------------------------+
> +| 0x11 | VIRTIO_GPIO_MSG_HOST_LEVEL | state changed (host->guest) |
> ++-------+----------------+-----------------------+-----------------------------+
> +
> +----------------------
> +Data flow:
> +----------------------
> +
> +- all operations, except ``VIRTIO_GPIO_MSG_HOST_LEVEL``, are guest-initiated
> +- host replies ``VIRTIO_GPIO_MSG_HOST_LEVEL`` OR'ed to the ``type`` field
> +- ``VIRTIO_GPIO_MSG_HOST_LEVEL`` is only sent asynchronically from host to guest
asynchronously
> +- in replies, a negative ``value`` field denotes an unix-style errno code
a Unix-style
> +- valid direction values are:> + * 0 = output
> + * 1 = input
> +- valid line state values are:
> + * 0 = inactive
> + * 1 = active
> +
> +VIRTIO_GPIO_MSG_GUEST_REQUEST
> +----------------------------
> +
> +- notify the host that given line# is going to be used
> +- request:
> + * ``line`` field: line number
> + * ``value`` field: unused
> +- reply:
> + * ``value`` field: errno code (0 = success)
> +
> +VIRTIO_GPIO_MSG_GUEST_DIRECTION_INPUT
> +------------------------------------
> +
> +- set line line direction to input
> +- request:
> + * ``line`` field: line number
> + * ``value`` field: unused
> +- reply: value field holds errno
> + * ``value`` field: errno code (0 = success)
> +
> +VIRTIO_GPIO_MSG_GUEST_DIRECTION_OUTPUT
> +-------------------------------------
> +
> +- set line direction to output and given line state
> +- request:
> + * ``line`` field: line number
> + * ``value`` field: output state (0=inactive, 1=active)
> +- reply:
> + * ``value`` field: holds errno
> +
> +VIRTIO_GPIO_MSG_GUEST_GET_DIRECTION
> +----------------------------------
> +
> +- retrieve line direction
> +- request:
> + * ``line`` field: line number
> + * ``value`` field: unused
> +- reply:
> + * ``value`` field: direction (0=output, 1=input) or errno code
> +
> +VIRTIO_GPIO_MSG_GUEST_GET_VALUE
> +------------------------------
> +
> +- retrieve line state value
> +- request:
> + * ``line`` field: line number
> + * ``value`` field: unused
> +- reply:
> + * ``value`` field: line state (0=inactive, 1=active) or errno code
> +
> +VIRTIO_GPIO_MSG_GUEST_SET_VALUE
> +------------------------------
> +
> +- set line state value (output only)
> +- request:
> + * ``line`` field: line number
> + * ``value`` field: line state (0=inactive, 1=active)
> +- reply:
> + * ``value`` field: new line state or errno code
> +
> +VIRTIO_GPIO_MSG_HOST_LEVEL
> +-------------------------
> +
> +- async notification from host to gues: line state changed
guest:
> +- ``line`` field: line number
> +- ``value`` field: new line state (0=inactive, 1=active)
--
~Randy
Powered by blists - more mailing lists