Re: [PATCH v4] Add virtio-input driver.

2015-03-26 Thread Michael S. Tsirkin
On Tue, Mar 24, 2015 at 01:08:46PM +0100, Gerd Hoffmann wrote:
> virtio-input is basically evdev-events-over-virtio, so this driver isn't
> much more than reading configuration from config space and forwarding
> incoming events to the linux input layer.
> 
> Signed-off-by: Gerd Hoffmann 

Looks good overallm though I think I still see a couple of minor issues.

> ---
>  MAINTAINERS   |   6 +
>  drivers/virtio/Kconfig|  10 ++
>  drivers/virtio/Makefile   |   1 +
>  drivers/virtio/virtio_input.c | 363 
> ++
>  include/uapi/linux/Kbuild |   1 +
>  include/uapi/linux/virtio_ids.h   |   1 +
>  include/uapi/linux/virtio_input.h |  76 
>  7 files changed, 458 insertions(+)
>  create mode 100644 drivers/virtio/virtio_input.c
>  create mode 100644 include/uapi/linux/virtio_input.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 358eb01..6f233dd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10442,6 +10442,12 @@ S:   Maintained
>  F:   drivers/vhost/
>  F:   include/uapi/linux/vhost.h
>  
> +VIRTIO INPUT DRIVER
> +M:   Gerd Hoffmann 
> +S:   Maintained
> +F:   drivers/virtio/virtio_input.c
> +F:   include/uapi/linux/virtio_input.h
> +
>  VIA RHINE NETWORK DRIVER
>  M:   Roger Luethi 
>  S:   Maintained
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index b546da5..cab9f3f 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -48,6 +48,16 @@ config VIRTIO_BALLOON
>  
>If unsure, say M.
>  
> +config VIRTIO_INPUT
> + tristate "Virtio input driver"
> + depends on VIRTIO
> + depends on INPUT
> + ---help---
> +  This driver supports virtio input devices such as
> +  keyboards, mice and tablets.
> +
> +  If unsure, say M.
> +
>   config VIRTIO_MMIO
>   tristate "Platform bus driver for memory mapped virtio devices"
>   depends on HAS_IOMEM
> diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> index d85565b..41e30e3 100644
> --- a/drivers/virtio/Makefile
> +++ b/drivers/virtio/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
>  virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
>  virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
>  obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
> +obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
> diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
> new file mode 100644
> index 000..fc99a15
> --- /dev/null
> +++ b/drivers/virtio/virtio_input.c
> @@ -0,0 +1,363 @@
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +struct virtio_input {
> + struct virtio_device   *vdev;
> + struct input_dev   *idev;
> + char   name[64];
> + char   serial[64];
> + char   phys[64];
> + struct virtqueue   *evt, *sts;
> + struct virtio_input_event  evts[64];
> + spinlock_t lock;
> + bool   ready;
> +};
> +
> +static void virtinput_queue_evtbuf(struct virtio_input *vi,
> +struct virtio_input_event *evtbuf)
> +{
> + struct scatterlist sg[1];
> +
> + sg_init_one(sg, evtbuf, sizeof(*evtbuf));
> + virtqueue_add_inbuf(vi->evt, sg, 1, evtbuf, GFP_ATOMIC);
> +}
> +
> +static void virtinput_recv_events(struct virtqueue *vq)
> +{
> + struct virtio_input *vi = vq->vdev->priv;
> + struct virtio_input_event *event;
> + unsigned long flags;
> + unsigned int len;
> +
> + spin_lock_irqsave(&vi->lock, flags);
> + if (vi->ready) {
> + while ((event = virtqueue_get_buf(vi->evt, &len)) != NULL) {
> + input_event(vi->idev,
> + le16_to_cpu(event->type),
> + le16_to_cpu(event->code),
> + le32_to_cpu(event->value));
> + virtinput_queue_evtbuf(vi, event);
> + }
> + virtqueue_kick(vq);
> + }
> + spin_unlock_irqrestore(&vi->lock, flags);
> +}
> +
> +static int virtinput_send_status(struct virtio_input *vi,
> +  u16 type, u16 code, s32 value)
> +{
> + struct virtio_input_event *stsbuf;
> + struct scatterlist sg[1];
> + unsigned long flags;
> + int rc;
> +
> + stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
> + if (!stsbuf)
> + return -ENOMEM;
> +
> + stsbuf->type  = cpu_to_le16(type);
> + stsbuf->code  = cpu_to_le16(code);
> + stsbuf->value = cpu_to_le32(value);
> + sg_init_one(sg, stsbuf, sizeof(*stsbuf));
> +
> + spin_lock_irqsave(&vi->lock, flags);
> + if (vi->ready) {
> + rc = virtqueue_add_outbuf(vi->sts, sg, 1, stsbuf, GFP_ATOMIC);
> + virtqueue_kick(vi->sts);
> + } else {
> + rc = -ENODEV;
> + }
> + spin_unlock_irqrestore(&vi

Re: [PATCH v4] Add virtio-input driver.

2015-03-25 Thread Rusty Russell
Vojtech Pavlik  writes:
> On Wed, Mar 25, 2015 at 01:51:43PM +1030, Rusty Russell wrote:
>> Imagine a future virtio standard which incorporates this.  And a Windows
>> or FreeBSD implementation of the device and or driver.  How ugly would
>> they be?
>
> A windows translation layer is fairly easy, people porting software from
> Windows to Linux told me numerous times that adapting isn't hard. I also
> believe that at least one of the BSD's has a compatible implementation
> these days based on the fact that I was asked to allow copying the
> headers in the past.

Thanks Vojtech, that answers my questions.

I figure we apply this and see where it leads...

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] Add virtio-input driver.

2015-03-24 Thread Vojtech Pavlik
On Wed, Mar 25, 2015 at 01:51:43PM +1030, Rusty Russell wrote:

> Gerd Hoffmann  writes:
> > virtio-input is basically evdev-events-over-virtio, so this driver isn't
> > much more than reading configuration from config space and forwarding
> > incoming events to the linux input layer.
> >
> > Signed-off-by: Gerd Hoffmann 
> 
> Is the input layer sane?  I've never dealt with it, so I don't know.

I'm rather biased having designed it, but I'd say it's reasonable. It
certainly has limitations and design mistakes, but none are too bad.
One testimony to that Android has based its own Input API around it.

> What's it used for?

For all human input devices, like keyboards, mice, touchscreens, etc. 

> Imagine a future virtio standard which incorporates this.  And a Windows
> or FreeBSD implementation of the device and or driver.  How ugly would
> they be?

A windows translation layer is fairly easy, people porting software from
Windows to Linux told me numerous times that adapting isn't hard. I also
believe that at least one of the BSD's has a compatible implementation
these days based on the fact that I was asked to allow copying the
headers in the past.

-- 
Vojtech Pavlik
Director SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] Add virtio-input driver.

2015-03-24 Thread Rusty Russell
Gerd Hoffmann  writes:
> virtio-input is basically evdev-events-over-virtio, so this driver isn't
> much more than reading configuration from config space and forwarding
> incoming events to the linux input layer.
>
> Signed-off-by: Gerd Hoffmann 

Is the input layer sane?  I've never dealt with it, so I don't know.

What's it used for?

Imagine a future virtio standard which incorporates this.  And a Windows
or FreeBSD implementation of the device and or driver.  How ugly would
they be?

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4] Add virtio-input driver.

2015-03-24 Thread Gerd Hoffmann
virtio-input is basically evdev-events-over-virtio, so this driver isn't
much more than reading configuration from config space and forwarding
incoming events to the linux input layer.

Signed-off-by: Gerd Hoffmann 
---
 MAINTAINERS   |   6 +
 drivers/virtio/Kconfig|  10 ++
 drivers/virtio/Makefile   |   1 +
 drivers/virtio/virtio_input.c | 363 ++
 include/uapi/linux/Kbuild |   1 +
 include/uapi/linux/virtio_ids.h   |   1 +
 include/uapi/linux/virtio_input.h |  76 
 7 files changed, 458 insertions(+)
 create mode 100644 drivers/virtio/virtio_input.c
 create mode 100644 include/uapi/linux/virtio_input.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 358eb01..6f233dd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10442,6 +10442,12 @@ S: Maintained
 F: drivers/vhost/
 F: include/uapi/linux/vhost.h
 
+VIRTIO INPUT DRIVER
+M: Gerd Hoffmann 
+S: Maintained
+F: drivers/virtio/virtio_input.c
+F: include/uapi/linux/virtio_input.h
+
 VIA RHINE NETWORK DRIVER
 M: Roger Luethi 
 S: Maintained
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index b546da5..cab9f3f 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -48,6 +48,16 @@ config VIRTIO_BALLOON
 
 If unsure, say M.
 
+config VIRTIO_INPUT
+   tristate "Virtio input driver"
+   depends on VIRTIO
+   depends on INPUT
+   ---help---
+This driver supports virtio input devices such as
+keyboards, mice and tablets.
+
+If unsure, say M.
+
  config VIRTIO_MMIO
tristate "Platform bus driver for memory mapped virtio devices"
depends on HAS_IOMEM
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index d85565b..41e30e3 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
 virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
 virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
 obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
+obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
new file mode 100644
index 000..fc99a15
--- /dev/null
+++ b/drivers/virtio/virtio_input.c
@@ -0,0 +1,363 @@
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+struct virtio_input {
+   struct virtio_device   *vdev;
+   struct input_dev   *idev;
+   char   name[64];
+   char   serial[64];
+   char   phys[64];
+   struct virtqueue   *evt, *sts;
+   struct virtio_input_event  evts[64];
+   spinlock_t lock;
+   bool   ready;
+};
+
+static void virtinput_queue_evtbuf(struct virtio_input *vi,
+  struct virtio_input_event *evtbuf)
+{
+   struct scatterlist sg[1];
+
+   sg_init_one(sg, evtbuf, sizeof(*evtbuf));
+   virtqueue_add_inbuf(vi->evt, sg, 1, evtbuf, GFP_ATOMIC);
+}
+
+static void virtinput_recv_events(struct virtqueue *vq)
+{
+   struct virtio_input *vi = vq->vdev->priv;
+   struct virtio_input_event *event;
+   unsigned long flags;
+   unsigned int len;
+
+   spin_lock_irqsave(&vi->lock, flags);
+   if (vi->ready) {
+   while ((event = virtqueue_get_buf(vi->evt, &len)) != NULL) {
+   input_event(vi->idev,
+   le16_to_cpu(event->type),
+   le16_to_cpu(event->code),
+   le32_to_cpu(event->value));
+   virtinput_queue_evtbuf(vi, event);
+   }
+   virtqueue_kick(vq);
+   }
+   spin_unlock_irqrestore(&vi->lock, flags);
+}
+
+static int virtinput_send_status(struct virtio_input *vi,
+u16 type, u16 code, s32 value)
+{
+   struct virtio_input_event *stsbuf;
+   struct scatterlist sg[1];
+   unsigned long flags;
+   int rc;
+
+   stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
+   if (!stsbuf)
+   return -ENOMEM;
+
+   stsbuf->type  = cpu_to_le16(type);
+   stsbuf->code  = cpu_to_le16(code);
+   stsbuf->value = cpu_to_le32(value);
+   sg_init_one(sg, stsbuf, sizeof(*stsbuf));
+
+   spin_lock_irqsave(&vi->lock, flags);
+   if (vi->ready) {
+   rc = virtqueue_add_outbuf(vi->sts, sg, 1, stsbuf, GFP_ATOMIC);
+   virtqueue_kick(vi->sts);
+   } else {
+   rc = -ENODEV;
+   }
+   spin_unlock_irqrestore(&vi->lock, flags);
+
+   if (rc != 0)
+   kfree(stsbuf);
+   return rc;
+}
+
+static void virtinput_recv_status(struct virtqueue *vq)
+{
+   struct virtio_input *vi = vq->vdev->priv;
+   struct virtio_input_event *stsbuf;
+   unsigned long flags;
+   unsigned