Re: [PATCH] staging: unisys: add visorhid driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 25, 2015 at 09:58:51AM -0400, Benjamin Romer wrote:
> From: Erik Arfvidson <erik.arfvidson@xxxxxxxxxx>
> 
> This driver provides mouse and keyboard input to Unisys s-Par
> Partition Desktop application. This device is created by the
> visorbus device.
> 
> Signed-off-by: Erik Arfvidson <erik.arfvidson@xxxxxxxxxx>
> Signed-off-by: Benjamin Romer <benjamin.romer@xxxxxxxxxx>
> ---
>  drivers/staging/unisys/Kconfig                     |   1 +
>  drivers/staging/unisys/Makefile                    |   1 +
>  drivers/staging/unisys/visorhid/Kconfig            |  10 +
>  drivers/staging/unisys/visorhid/Makefile           |   7 +
>  drivers/staging/unisys/visorhid/keyboardchannel.h  |  32 +
>  drivers/staging/unisys/visorhid/mousechannel.h     |  32 +
>  drivers/staging/unisys/visorhid/ultrainputreport.h |  82 +++
>  drivers/staging/unisys/visorhid/visorhid.c         | 798 +++++++++++++++++++++
>  8 files changed, 963 insertions(+)
>  create mode 100644 drivers/staging/unisys/visorhid/Kconfig
>  create mode 100644 drivers/staging/unisys/visorhid/Makefile
>  create mode 100644 drivers/staging/unisys/visorhid/keyboardchannel.h
>  create mode 100644 drivers/staging/unisys/visorhid/mousechannel.h
>  create mode 100644 drivers/staging/unisys/visorhid/ultrainputreport.h
>  create mode 100644 drivers/staging/unisys/visorhid/visorhid.c
> 
> diff --git a/drivers/staging/unisys/Kconfig b/drivers/staging/unisys/Kconfig
> index 778f9d0..bdc8ba8 100644
> --- a/drivers/staging/unisys/Kconfig
> +++ b/drivers/staging/unisys/Kconfig
> @@ -13,5 +13,6 @@ if UNISYSSPAR
>  
>  source "drivers/staging/unisys/visorbus/Kconfig"
>  source "drivers/staging/unisys/visornic/Kconfig"
> +source "drivers/staging/unisys/visorhid/Kconfig"
>  
>  endif # UNISYSSPAR
> diff --git a/drivers/staging/unisys/Makefile b/drivers/staging/unisys/Makefile
> index a515ebc..d071094 100644
> --- a/drivers/staging/unisys/Makefile
> +++ b/drivers/staging/unisys/Makefile
> @@ -3,3 +3,4 @@
>  #
>  obj-$(CONFIG_UNISYS_VISORBUS)		+= visorbus/
>  obj-$(CONFIG_UNISYS_VISORNIC)		+= visornic/
> +obj-$(CONFIG_UNISYS_VISORHID)          += visorhid/
> diff --git a/drivers/staging/unisys/visorhid/Kconfig b/drivers/staging/unisys/visorhid/Kconfig
> new file mode 100644
> index 0000000..3b83e2c
> --- /dev/null
> +++ b/drivers/staging/unisys/visorhid/Kconfig
> @@ -0,0 +1,10 @@
> +#
> +# Unisys visorhid configuration
> +#
> +
> +config UNISYS_VISORHID
> +	tristate "Unisys visorhid driver"
> +	depends on UNISYSSPAR && UNISYS_VISORBUS && FB
> +	---help---
> +	If you say Y here, you will enable the Unisys visorhid driver.
> +
> diff --git a/drivers/staging/unisys/visorhid/Makefile b/drivers/staging/unisys/visorhid/Makefile
> new file mode 100644
> index 0000000..e457bd1
> --- /dev/null
> +++ b/drivers/staging/unisys/visorhid/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# Makefile for Unisys visorhid
> +#
> +
> +obj-$(CONFIG_UNISYS_VISORHID)	+= visorhid.o
> +
> +ccflags-y += -Idrivers/staging/unisys/include
> diff --git a/drivers/staging/unisys/visorhid/keyboardchannel.h b/drivers/staging/unisys/visorhid/keyboardchannel.h
> new file mode 100644
> index 0000000..0722c60
> --- /dev/null
> +++ b/drivers/staging/unisys/visorhid/keyboardchannel.h
> @@ -0,0 +1,32 @@
> +/* Copyright (C) 2010 - 2013 UNISYS CORPORATION
> + * All rights reserved.

2013?

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.

I have to ask, do you really mean "any later version"?

> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT.  See the GNU General Public License for more
> + * details.
> + */
> +
> +#ifndef __KEYBOARDCHANNEL_H__
> +#define __KEYBOARDCHANNEL_H__
> +
> +#include <linux/kernel.h>
> +#include <linux/uuid.h>
> +
> +#include "channel.h"
> +#include "ultrainputreport.h"
> +
> +/* {C73416D0-B0B8-44af-B304-9D2AE99F1B3D} */
> +#define SPAR_KEYBOARD_CHANNEL_PROTOCOL_UUID				\
> +	UUID_LE(0xc73416d0, 0xb0b8, 0x44af,				\
> +		0xb3, 0x4, 0x9d, 0x2a, 0xe9, 0x9f, 0x1b, 0x3d)
> +#define SPAR_KEYBOARD_CHANNEL_PROTOCOL_VERSIONID 1
> +#define KEYBOARD_MAXINPUTREPORTS 50
> +
> +#endif
> diff --git a/drivers/staging/unisys/visorhid/mousechannel.h b/drivers/staging/unisys/visorhid/mousechannel.h
> new file mode 100644
> index 0000000..ac6883b
> --- /dev/null
> +++ b/drivers/staging/unisys/visorhid/mousechannel.h
> @@ -0,0 +1,32 @@
> +/* Copyright (C) 2010 - 2013 UNISYS CORPORATION
> + * All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT.  See the GNU General Public License for more
> + * details.
> + */
> +
> +#ifndef __MOUSECHANNEL_H__
> +#define __MOUSECHANNEL_H__
> +
> +#include <linux/kernel.h>
> +#include <linux/uuid.h>
> +
> +#include "channel.h"
> +#include "ultrainputreport.h"
> +
> +/* {ADDF07D4-94A9-46e2-81C3-61ABCDBDBD87} */
> +#define SPAR_MOUSE_CHANNEL_PROTOCOL_UUID  \
> +	UUID_LE(0xaddf07d4, 0x94a9, 0x46e2, \
> +		0x81, 0xc3, 0x61, 0xab, 0xcd, 0xbd, 0xbd, 0x87)
> +#define SPAR_MOUSE_CHANNEL_PROTOCOL_VERSIONID 1
> +#define MOUSE_MAXINPUTREPORTS 50
> +
> +#endif
> diff --git a/drivers/staging/unisys/visorhid/ultrainputreport.h b/drivers/staging/unisys/visorhid/ultrainputreport.h
> new file mode 100644
> index 0000000..4cd75cc
> --- /dev/null
> +++ b/drivers/staging/unisys/visorhid/ultrainputreport.h
> @@ -0,0 +1,82 @@
> +/* Copyright (C) 2010 - 2013 UNISYS CORPORATION
> + * All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT.  See the GNU General Public License for more
> + * details.
> + */
> +
> +#ifndef __ULTRAINPUTREPORT_H__
> +#define __ULTRAINPUTREPORT_H__
> +
> +#include <linux/types.h>
> +
> +#include "ultrainputreport.h"
> +
> +/* Identifies mouse and keyboard activity which is specified by the firmware to
> + *  the host using the cmsimpleinput protocol.  @ingroup coretypes
> + */
> +enum ultra_inputaction {
> +	inputaction_none = 0,
> +	inputaction_xy_motion = 1,	/*< only motion; arg1=x, arg2=y */
> +	inputaction_mouse_button_down = 2, /*< arg1: 1=left, 2=center, 3=right,
> +					    4, *  5 */
> +	inputaction_mouse_button_up = 3, /*< arg1: 1=left, 2=center, 3=right,
> +					  4,*  5 */

Very odd comment style, it's not readable at all.  I think you ran this
through some script...


> +	inputaction_mouse_button_click = 4, /*< arg1: 1=left, 2=center, 3=right,
> +					 *  4, 5 */
> +	inputaction_mouse_button_dclick = 5, /*< arg1: 1=left, 2=center,
> +					 3=right, *  4, 5 */
> +	inputaction_wheel_rotate_away = 6, /*< arg1: wheel rotation away from
> +					  *  user */
> +	inputaction_wheel_rotate_toward = 7, /*< arg1: wheel rotation toward
> +					    *  user */
> +	inputaction_set_max_xy = 8,	/*< set screen maxXY; arg1=x, arg2=y */
> +	inputaction_key_down = 64,	/*< arg1: scancode, as follows:
> +					 * If arg1 <= 0xff, it's a 1-byte
> +					 * scancode and arg1 is that scancode.
> +					 * If arg1 > 0xff, it's a 2-byte
> +					 * scanecode, with the 1st byte in the
> +					 * low 8 bits, and the 2nd byte in the
> +					 * high 8 bits.  E.g., the right ALT key
> +					 * would appear as x'38e0'.
> +					 */
> +	inputaction_key_up = 65,	/*< arg1: scancode (in same format as
> +					 * inputaction_keyDown)
> +					 */
> +	inputaction_set_locking_key_state = 66,
> +					/*< arg1: scancode (in same format
> +					 *         as inputaction_keyDown);
> +					 *         MUST refer to one of the
> +					 *         locking keys, like capslock,
> +					 *         numlock, or scrolllock
> +					 *   arg2: 1 iff locking key should be
> +					 *         in the LOCKED position
> +					 *         (e.g., light is ON)
> +					 */
> +	inputaction_key_down_up = 67,	/*< arg1: scancode (in same format
> +					 *         as inputaction_keyDown)
> +					 */
> +	inputaction_last
> +};
> +
> +struct ultra_inputactivity {
> +	u16 action;
> +	u16 arg1;
> +	u16 arg2;
> +	u16 arg3;
> +} __packed;
> +
> +struct ultra_inputreport {
> +	u64 seq_no;
> +	struct ultra_inputactivity activity;
> +} __packed;
> +
> +#endif
> diff --git a/drivers/staging/unisys/visorhid/visorhid.c b/drivers/staging/unisys/visorhid/visorhid.c
> new file mode 100644
> index 0000000..7384bef
> --- /dev/null
> +++ b/drivers/staging/unisys/visorhid/visorhid.c
> @@ -0,0 +1,798 @@
> +/* visorhid.c
> + *
> + * Copyright (C) 2011 - 2014 UNISYS CORPORATION
> + * All rights reserved.

2014?

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT.  See the GNU General Public License for more
> + * details.
> + */
> +
> +/* This driver lives in a generic guest Linux partition, and registers to
> + * receive keyboard and mouse channels from the visorbus driver.  It reads
> + * inputs from such channels, and delivers it to the Linux OS in the
> + * standard way the Linux expects for input drivers.
> + */
> +
> +#include <linux/buffer_head.h>
> +#include <linux/fb.h>
> +#include <linux/fs.h>
> +#include <linux/input.h>
> +#include <linux/serio.h>
> +#include <linux/uaccess.h>
> +
> +#include <asm/segment.h>
> +
> +#include "keyboardchannel.h"
> +#include "mousechannel.h"
> +#include "version.h"
> +#include "visorbus.h"
> +
> +#define MAXDEVICES     16384

That's a lot of devices, do you really mean it?

> +#define PIXELS_ACROSS_DEFAULT 800
> +#define PIXELS_DOWN_DEFAULT   600
> +#define DEV_NAME_SIZE 99

No tabs for indenting?

> +#define SYSFS_VIRTUALSIZE "/sys/class/graphics/fb0/virtual_size"

I really hate to even look as to why you have a full sysfs path in a
#define in a kernel file.  Luckily I don't see it used anywhere, please
remove it.

> +
> +static spinlock_t devnopool_lock;
> +static void *dev_no_pool; /* < pool to grab device numbers from */

What is up with the '<' character in comments?

> +static const uuid_le spar_keyboard_channel_protocol_uuid =
> +	SPAR_KEYBOARD_CHANNEL_PROTOCOL_UUID;
> +static const uuid_le spar_mouse_channel_protocol_uuid =
> +	SPAR_MOUSE_CHANNEL_PROTOCOL_UUID;
> +static int visorhid_probe(struct visor_device *dev);
> +static void visorhid_remove(struct visor_device *dev);
> +static void visorhid_channel_interrupt(struct visor_device *dev);
> +static int visorhid_pause(struct visor_device *dev,
> +			  visorbus_state_complete_func complete_func);
> +static int visorhid_resume(struct visor_device *dev,
> +			   visorbus_state_complete_func complete_func);
> +static struct input_dev *register_client_keyboard(void);
> +static struct input_dev *register_client_mouse(void);
> +static struct input_dev *register_client_wheel(void);
> +static void unregister_client_input(struct input_dev *visorinput_dev);
> +
> +/* GUIDS for all channel types supported by this driver. */
> +static struct visor_channeltype_descriptor visorhid_channel_types[] = {
> +	{
> +		.guid = SPAR_KEYBOARD_CHANNEL_PROTOCOL_UUID,
> +		.name = "keyboard"
> +	},
> +	{
> +		.guid = SPAR_MOUSE_CHANNEL_PROTOCOL_UUID,
> +		.name = "mouse"
> +	},
> +	{
> +		.guid = NULL_UUID_LE,
> +		.name = NULL
> +	}
> +};
> +
> +/** This is used to tell the visor bus driver which types of visor devices
> + *  we support, and what functions to call when a visor device that we support
> + *  is attached or removed.
> + */
> +static struct visor_driver visorhid_driver = {
> +	.name = "visorhid",
> +	.version = VERSION,

Why do we care about "versions"?

> +	.vertag = NULL,
> +	.owner = THIS_MODULE,
> +	.channel_types = visorhid_channel_types,
> +	.probe = visorhid_probe,
> +	.remove = visorhid_remove,
> +	.channel_interrupt = visorhid_channel_interrupt,
> +	.pause = visorhid_pause,
> +	.resume = visorhid_resume,
> +};
> +
> +/*  This is the private data that we store for each device.
> + *  A pointer to this struct is kept in each "struct device"

Why isn't this embedded in a struct device?

> + */
> +struct visorhid_devdata {
> +	int devno;
> +	struct visor_device *dev;
> +	/** lock for dev */
> +	struct rw_semaphore lock_visor_dev;
> +	char name[DEV_NAME_SIZE];

Why duplicate the name, why can't you use the struct device?

> +	struct list_head list_all;   /* < link within list_all_devices list */
> +	struct kref kref;

You don't use this reference at all, why even have it?

> +	struct input_dev *visorinput_dev;
> +	struct input_dev *visorinput_dev2;
> +	bool supported_client_device;
> +	bool paused;
> +};
> +
> + /* List of all visorhid_devdata structs,
> +  * linked via the list_all member
> +  */
> +static LIST_HEAD(list_all_devices);
> +static DEFINE_SPINLOCK(lock_all_devices);
> +
> +/* Borrowed from drivers/input/keyboard/atakbd.c */
> +/* This maps 1-byte scancodes to keycodes. */
> +static unsigned char visorkbd_keycode[256] = {	/* American layout */
> +	[0] = KEY_GRAVE,
> +	[1] = KEY_ESC,
> +	[2] = KEY_1,
> +	[3] = KEY_2,
> +	[4] = KEY_3,
> +	[5] = KEY_4,
> +	[6] = KEY_5,
> +	[7] = KEY_6,
> +	[8] = KEY_7,
> +	[9] = KEY_8,
> +	[10] = KEY_9,
> +	[11] = KEY_0,
> +	[12] = KEY_MINUS,
> +	[13] = KEY_EQUAL,
> +	[14] = KEY_BACKSPACE,
> +	[15] = KEY_TAB,
> +	[16] = KEY_Q,
> +	[17] = KEY_W,
> +	[18] = KEY_E,
> +	[19] = KEY_R,
> +	[20] = KEY_T,
> +	[21] = KEY_Y,
> +	[22] = KEY_U,
> +	[23] = KEY_I,
> +	[24] = KEY_O,
> +	[25] = KEY_P,
> +	[26] = KEY_LEFTBRACE,
> +	[27] = KEY_RIGHTBRACE,
> +	[28] = KEY_ENTER,
> +	[29] = KEY_LEFTCTRL,
> +	[30] = KEY_A,
> +	[31] = KEY_S,
> +	[32] = KEY_D,
> +	[33] = KEY_F,
> +	[34] = KEY_G,
> +	[35] = KEY_H,
> +	[36] = KEY_J,
> +	[37] = KEY_K,
> +	[38] = KEY_L,
> +	[39] = KEY_SEMICOLON,
> +	[40] = KEY_APOSTROPHE,
> +	[41] = KEY_GRAVE,	/* FIXME, '#' */
> +	[42] = KEY_LEFTSHIFT,
> +	[43] = KEY_BACKSLASH,	/* FIXME, '~' */
> +	[44] = KEY_Z,
> +	[45] = KEY_X,
> +	[46] = KEY_C,
> +	[47] = KEY_V,
> +	[48] = KEY_B,
> +	[49] = KEY_N,
> +	[50] = KEY_M,
> +	[51] = KEY_COMMA,
> +	[52] = KEY_DOT,
> +	[53] = KEY_SLASH,
> +	[54] = KEY_RIGHTSHIFT,
> +	[55] = KEY_KPASTERISK,
> +	[56] = KEY_LEFTALT,
> +	[57] = KEY_SPACE,
> +	[58] = KEY_CAPSLOCK,
> +	[59] = KEY_F1,
> +	[60] = KEY_F2,
> +	[61] = KEY_F3,
> +	[62] = KEY_F4,
> +	[63] = KEY_F5,
> +	[64] = KEY_F6,
> +	[65] = KEY_F7,
> +	[66] = KEY_F8,
> +	[67] = KEY_F9,
> +	[68] = KEY_F10,
> +	[69] = KEY_NUMLOCK,
> +	[70] = KEY_SCROLLLOCK,
> +	[71] = KEY_KP7,
> +	[72] = KEY_KP8,
> +	[73] = KEY_KP9,
> +	[74] = KEY_KPMINUS,
> +	[75] = KEY_KP4,
> +	[76] = KEY_KP5,
> +	[77] = KEY_KP6,
> +	[78] = KEY_KPPLUS,
> +	[79] = KEY_KP1,
> +	[80] = KEY_KP2,
> +	[81] = KEY_KP3,
> +	[82] = KEY_KP0,
> +	[83] = KEY_KPDOT,
> +	[87] = KEY_F11,
> +	[88] = KEY_F12,
> +	[90] = KEY_KPLEFTPAREN,
> +	[91] = KEY_KPRIGHTPAREN,
> +	[92] = KEY_KPASTERISK,	/* FIXME */
> +	[93] = KEY_KPASTERISK,
> +	[94] = KEY_KPPLUS,
> +	[95] = KEY_HELP,
> +	[96] = KEY_KPENTER,
> +	[97] = KEY_RIGHTCTRL,
> +	[98] = KEY_KPSLASH,
> +	[99] = KEY_KPLEFTPAREN,
> +	[100] = KEY_KPRIGHTPAREN,
> +	[101] = KEY_KPSLASH,
> +	[102] = KEY_HOME,
> +	[103] = KEY_UP,
> +	[104] = KEY_PAGEUP,
> +	[105] = KEY_LEFT,
> +	[106] = KEY_RIGHT,
> +	[107] = KEY_END,
> +	[108] = KEY_DOWN,
> +	[109] = KEY_PAGEDOWN,
> +	[110] = KEY_INSERT,
> +	[111] = KEY_DELETE,
> +	[112] = KEY_MACRO,
> +	[113] = KEY_MUTE
> +};
> +
> +/* This maps the <xx> in extended scancodes of the form "0xE0 <xx>" into
> + * keycodes.
> + */
> +static unsigned char ext_keycode[256] = {
> +	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,	/* 0x00 */
> +	0, 0, 0, 0, 0, 0, 0, 0,	/* 0x10 */
> +	0, 0, 0, 0, KEY_KPENTER, KEY_RIGHTCTRL, 0, 0,	/* 0x18 */
> +	0, 0, 0, 0, 0, 0, 0, 0,	/* 0x20 */
> +	KEY_RIGHTALT, 0, 0, 0, 0, 0, 0, 0,	/* 0x28 */
> +	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,	/* 0x30 */
> +	0, 0, 0, 0, 0, 0, 0, KEY_HOME,	/* 0x40 */
> +	KEY_UP, KEY_PAGEUP, 0, KEY_LEFT, 0, KEY_RIGHT, 0, KEY_END, /* 0x48 */
> +	KEY_DOWN, KEY_PAGEDOWN, KEY_INSERT, KEY_DELETE, 0, 0, 0, 0, /* 0x50 */
> +	0, 0, 0, 0, 0, 0, 0, 0,	/* 0x58 */
> +	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,	/* 0x60 */
> +	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,	/* 0x70 */
> +};
> +
> +static struct visorhid_devdata *
> +devdata_create(struct visor_device *dev)
> +{
> +	struct visorhid_devdata *devdata = NULL;
> +	int devno = -1;
> +	uuid_le guid;
> +
> +	guid = visorchannel_get_uuid(dev->visorchannel);
> +	devdata = kzalloc(sizeof(*devdata),
> +			  GFP_KERNEL | __GFP_NORETRY);
> +	if (!devdata)
> +		goto cleanups;
> +
> +	spin_lock(&devnopool_lock);
> +	devno = find_first_zero_bit(dev_no_pool, MAXDEVICES);
> +	spin_unlock(&devnopool_lock);

Why not use an idr or ida structure?  Much simpler, easier, and correct.

> +	if (devno == MAXDEVICES)
> +		goto cleanups_set_bit;
> +	set_bit(devno, dev_no_pool);
> +	devdata->devno = devno;
> +	devdata->dev = dev;
> +	strncpy(devdata->name, dev_name(&dev->device), sizeof(devdata->name));

Again, why is this needed?

> +
> +	/* This is an input device in a client guest partition,
> +	 * so we need to create whatever gizmos are necessary to
> +	 * deliver our inputs to the guest OS.
> +	 */
> +	if (!uuid_le_cmp(guid, spar_keyboard_channel_protocol_uuid)) {
> +		devdata->visorinput_dev = register_client_keyboard();
> +		if (!devdata->visorinput_dev)
> +			goto cleanups_register;
> +		devdata->supported_client_device = true;
> +	} else if (!uuid_le_cmp(guid, spar_mouse_channel_protocol_uuid)) {
> +		devdata->visorinput_dev = register_client_mouse();
> +		if (!devdata->visorinput_dev)
> +			goto cleanups_register2;
> +		devdata->visorinput_dev2 = register_client_wheel();
> +		if (!devdata->visorinput_dev2)
> +			goto cleanups_register2;
> +		devdata->supported_client_device = true;
> +	}
> +
> +	init_rwsem(&devdata->lock_visor_dev);
> +	kref_init(&devdata->kref);
> +
> +	spin_lock(&lock_all_devices);
> +	list_add_tail(&devdata->list_all, &list_all_devices);
> +	spin_unlock(&lock_all_devices);
> +	return devdata;
> +
> +cleanups_register2:
> +	unregister_client_input(devdata->visorinput_dev);
> +	devdata->visorinput_dev = NULL;
> +cleanups_register:
> +	clear_bit(devno, dev_no_pool);
> +cleanups_set_bit:
> +	kfree(devdata);
> +cleanups:
> +	return NULL;
> +}
> +
> +static void
> +devdata_release(struct kref *mykref)
> +{
> +	struct visorhid_devdata *devdata =
> +	    container_of(mykref, struct visorhid_devdata, kref);
> +	clear_bit(devdata->devno, dev_no_pool);
> +	spin_lock(&lock_all_devices);
> +	list_del(&devdata->list_all);
> +	spin_unlock(&lock_all_devices);
> +}
> +
> +static int
> +visorhid_probe(struct visor_device *dev)
> +{
> +	int rc = 0;
> +	struct visorhid_devdata *devdata = NULL;
> +	uuid_le guid;
> +
> +	devdata = devdata_create(dev);
> +	if (!devdata) {
> +		rc = -1;
> +		goto cleanups;
> +	}
> +	dev_set_drvdata(&dev->device, devdata);
> +	guid = visorchannel_get_uuid(dev->visorchannel);
> +	if (uuid_le_cmp(guid, spar_mouse_channel_protocol_uuid) &&
> +	    uuid_le_cmp(guid, spar_keyboard_channel_protocol_uuid)) {
> +		rc = -1;
> +		goto cleanups;
> +	}
> +
> +	if (devdata->supported_client_device)
> +		visorbus_enable_channel_interrupts(dev);
> +
> +cleanups:
> +	if (rc < 0) {
> +		if (devdata)
> +			kref_put(&devdata->kref, devdata_release);
> +	}
> +	return rc;
> +}
> +
> +static void
> +host_side_disappeared(struct visorhid_devdata *devdata)
> +{
> +	down_write(&devdata->lock_visor_dev);
> +	sprintf(devdata->name, "<dev#%d-history>", devdata->devno);
> +	devdata->dev = NULL;	/* indicate device destroyed */
> +	up_write(&devdata->lock_visor_dev);
> +}
> +
> +static void
> +visorhid_remove(struct visor_device *dev)
> +{
> +	struct visorhid_devdata *devdata = dev_get_drvdata(&dev->device);
> +
> +	if (!devdata)
> +		return;
> +
> +	dev_set_drvdata(&dev->device, NULL);
> +	host_side_disappeared(devdata);
> +	unregister_client_input(devdata->visorinput_dev);
> +	devdata->visorinput_dev = NULL;
> +	unregister_client_input(devdata->visorinput_dev2);
> +	devdata->visorinput_dev2 = NULL;
> +	kref_put(&devdata->kref, devdata_release);
> +}
> +
> +static void
> +visorhid_cleanup_guts(void)
> +{
> +	visorbus_unregister_visor_driver(&visorhid_driver);
> +		kfree(dev_no_pool);
> +		dev_no_pool = NULL;
> +}
> +
> +static void
> +unregister_client_input(struct input_dev *visorinput_dev)
> +{
> +	if (visorinput_dev)
> +		input_unregister_device(visorinput_dev);
> +}
> +
> +/* register_client_keyboard() initializes and returns a Linux gizmo that we
> + * can use to deliver keyboard inputs to Linux.  We of course do this when
> + * we see keyboard inputs coming in on a keyboard channel.
> + */
> +static struct input_dev *
> +register_client_keyboard(void)
> +{
> +	int i, error;
> +	struct input_dev *visorinput_dev = NULL;
> +
> +	visorinput_dev = input_allocate_device();
> +	if (!visorinput_dev)
> +		return NULL;
> +
> +	visorinput_dev->name = "visor Keyboard";
> +	visorinput_dev->phys = "visorkbd/input0";

Shouldn't that be "visorkbd!input0"?

> +	visorinput_dev->id.bustype = BUS_HOST;
> +	visorinput_dev->id.vendor = 0x0001;
> +	visorinput_dev->id.product = 0x0001;
> +	visorinput_dev->id.version = 0x0100;
> +
> +	visorinput_dev->evbit[0] = BIT_MASK(EV_KEY) |
> +				   BIT_MASK(EV_REP) |
> +				   BIT_MASK(EV_LED);
> +	visorinput_dev->ledbit[0] = BIT_MASK(LED_CAPSL) |
> +				    BIT_MASK(LED_SCROLLL) |
> +				    BIT_MASK(LED_NUML);
> +	visorinput_dev->keycode = visorkbd_keycode;
> +	visorinput_dev->keycodesize = sizeof(unsigned char);
> +	visorinput_dev->keycodemax = ARRAY_SIZE(visorkbd_keycode);
> +
> +	for (i = 1; i < ARRAY_SIZE(visorkbd_keycode); i++)
> +		set_bit(visorkbd_keycode[i], visorinput_dev->keybit);
> +
> +	error = input_register_device(visorinput_dev);
> +	if (error) {
> +		input_free_device(visorinput_dev);
> +		return NULL;
> +	}
> +	return visorinput_dev;
> +}
> +
> +static struct input_dev *
> +register_client_mouse(void)
> +{
> +	int error;
> +	struct input_dev *visorinput_dev = NULL;
> +	int xres, yres;
> +	struct fb_info *fb0;
> +
> +	visorinput_dev = input_allocate_device();
> +	if (!visorinput_dev)
> +		return NULL;
> +
> +	visorinput_dev->name = "visor Mouse";
> +	visorinput_dev->phys = "visormou/input0";

/me hands you a some extra "me" characters...

Again, s/\//\!/g

> +	visorinput_dev->id.bustype = BUS_HOST;
> +	visorinput_dev->id.vendor = 0x0001;
> +	visorinput_dev->id.product = 0x0001;
> +	visorinput_dev->id.version = 0x0100;
> +
> +	visorinput_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +	set_bit(BTN_LEFT, visorinput_dev->keybit);
> +	set_bit(BTN_RIGHT, visorinput_dev->keybit);
> +	set_bit(BTN_MIDDLE, visorinput_dev->keybit);
> +
> +	if (registered_fb[0]) {
> +		fb0 = registered_fb[0];
> +		xres = fb0->var.xres_virtual;
> +		yres = fb0->var.yres_virtual;
> +	} else {
> +		xres = PIXELS_ACROSS_DEFAULT;
> +		yres = PIXELS_DOWN_DEFAULT;
> +	}
> +	input_set_abs_params(visorinput_dev, ABS_X, 0, xres, 0, 0);
> +	input_set_abs_params(visorinput_dev, ABS_Y, 0, yres, 0, 0);
> +
> +	error = input_register_device(visorinput_dev);
> +	if (error) {
> +		input_free_device(visorinput_dev);
> +		return NULL;
> +	}
> +
> +	/* Sending top-left and bottom-right positions is ABSOLUTELY
> +	 * REQUIRED if we want X to move the mouse to the exact points
> +	 * we tell it.  I have NO IDEA why.
> +	 */
> +	input_report_abs(visorinput_dev, ABS_X, 0);
> +	input_report_abs(visorinput_dev, ABS_Y, 0);
> +	input_sync(visorinput_dev);
> +	input_report_abs(visorinput_dev, ABS_X, xres - 1);
> +	input_report_abs(visorinput_dev, ABS_Y, yres - 1);
> +	input_sync(visorinput_dev);
> +
> +	return visorinput_dev;
> +}
> +
> +static struct input_dev *
> +register_client_wheel(void)
> +{
> +	int error;
> +	struct input_dev *visorinput_dev = NULL;
> +
> +	visorinput_dev = input_allocate_device();
> +	if (!visorinput_dev)
> +		return NULL;
> +
> +	visorinput_dev->name = "visor Wheel";
> +	visorinput_dev->phys = "visorwhl/input0";

Same '!' issue.

> +	visorinput_dev->id.bustype = BUS_HOST;
> +	visorinput_dev->id.vendor = 0x0001;
> +	visorinput_dev->id.product = 0x0001;
> +	visorinput_dev->id.version = 0x0100;
> +
> +	/* We need to lie a little to prevent the evdev driver "Don't
> +	 * know how to use device" error.  (evdev erroneously thinks
> +	 * that a device without an X and Y axis is useless.)
> +	 */
> +	visorinput_dev->evbit[0] = BIT_MASK(EV_REL) |
> +			/*lie */   BIT_MASK(EV_KEY);
> +	visorinput_dev->relbit[0] = BIT_MASK(REL_WHEEL) |
> +			/*lie */    BIT_MASK(REL_X) |
> +			/*lie */    BIT_MASK(REL_Y);
> +	set_bit(BTN_LEFT, visorinput_dev->keybit);	/*lie */
> +	set_bit(BTN_RIGHT, visorinput_dev->keybit);	/*lie */
> +	set_bit(BTN_MIDDLE, visorinput_dev->keybit);	/*lie */
> +
> +	error = input_register_device(visorinput_dev);
> +	if (error) {
> +		input_free_device(visorinput_dev);
> +		return NULL;
> +	}
> +	return visorinput_dev;
> +}
> +
> +static void
> +do_key(struct input_dev *inpt, int keycode, int down)
> +{
> +	input_report_key(inpt, keycode, down);
> +}
> +
> +/* Make it so the current locking state of the locking key indicated by
> + * <keycode> is as indicated by <desired_state> (1=locked, 0=unlocked).
> + */
> +static void
> +handle_locking_key(struct input_dev *visorinput_dev,
> +		   int keycode, int desired_state)
> +{
> +	int led;
> +	char *sled;
> +
> +	switch (keycode) {
> +	case KEY_CAPSLOCK:
> +		led = LED_CAPSL;
> +		sled = "CAP";
> +		break;
> +	case KEY_SCROLLLOCK:
> +		led = LED_SCROLLL;
> +		sled = "SCR";
> +		break;
> +	case KEY_NUMLOCK:
> +		led = LED_NUML;
> +		sled = "NUM";
> +		break;
> +	default:
> +		led = -1;
> +		break;
> +	}
> +	if (led >= 0) {
> +		int old_state = (test_bit(led, visorinput_dev->led) != 0);
> +
> +		if (old_state != desired_state) {
> +			do_key(visorinput_dev, keycode, 1);
> +			do_key(visorinput_dev, keycode, 0);
> +			input_sync(visorinput_dev);
> +			__change_bit(led, visorinput_dev->led);
> +		}
> +	}
> +}
> +
> +/* <scancode> is either a 1-byte scancode, or an extended 16-bit scancode
> + * with 0xE0 in the low byte and the extended scancode value in the next
> + * higher byte.
> + */
> +static int
> +scancode_to_keycode(int scancode)
> +{
> +	int keycode;
> +
> +	if (scancode > 0xff)
> +		keycode = ext_keycode[(scancode >> 8) & 0xff];
> +	else
> +		keycode = visorkbd_keycode[scancode];
> +	return keycode;
> +}
> +
> +static int
> +calc_button(int x)
> +{
> +	switch (x) {
> +	case 1:
> +		return BTN_LEFT;
> +	case 2:
> +		return BTN_MIDDLE;
> +	case 3:
> +		return BTN_RIGHT;
> +	default:
> +		return -1;
> +	}
> +}
> +
> +/* This is used only when this driver is active as an input driver in the
> + * client guest partition.  It is called periodically so we can obtain inputs
> + * from the channel, and deliver them to the guest OS.
> + */
> +static void
> +visorhid_channel_interrupt(struct visor_device *dev)
> +{
> +	struct ultra_inputreport r;
> +	int scancode, keycode;
> +	struct input_dev *visorinput_dev;
> +	struct input_dev *visorinput_dev2;
> +	int xmotion, ymotion, zmotion, button;
> +	int i;
> +
> +	struct visorhid_devdata *devdata = dev_get_drvdata(&dev->device);
> +
> +	if (!devdata)
> +		return;
> +
> +	down_write(&devdata->lock_visor_dev);
> +	if (devdata->paused) /* don't touch device/channel when paused */
> +		goto out_locked;
> +
> +	visorinput_dev = devdata->visorinput_dev;
> +	if (!visorinput_dev)
> +		goto out_locked;
> +
> +	visorinput_dev2 = devdata->visorinput_dev2;
> +	while (visorchannel_signalremove(dev->visorchannel, 0, &r)) {
> +		scancode = r.activity.arg1;
> +		keycode = scancode_to_keycode(scancode);
> +		switch (r.activity.action) {
> +		case inputaction_key_down:
> +			do_key(visorinput_dev, keycode, 1);
> +			input_sync(visorinput_dev);
> +			break;
> +		case inputaction_key_up:
> +			do_key(visorinput_dev, keycode, 0);
> +			input_sync(visorinput_dev);
> +			break;
> +		case inputaction_key_down_up:
> +			do_key(visorinput_dev, keycode, 1);
> +			do_key(visorinput_dev, keycode, 0);
> +			input_sync(visorinput_dev);
> +			break;
> +		case inputaction_set_locking_key_state:
> +			handle_locking_key(visorinput_dev, keycode,
> +					   r.activity.arg2);
> +			break;
> +		case inputaction_xy_motion:
> +			xmotion = r.activity.arg1;
> +			ymotion = r.activity.arg2;
> +			input_report_abs(visorinput_dev, ABS_X, xmotion);
> +			input_report_abs(visorinput_dev, ABS_Y, ymotion);
> +			input_sync(visorinput_dev);
> +			break;
> +		case inputaction_mouse_button_down:
> +			button = calc_button(r.activity.arg1);
> +			if (button < 0)
> +				break;
> +			input_report_key(visorinput_dev, button, 1);
> +			input_sync(visorinput_dev);
> +			break;
> +		case inputaction_mouse_button_up:
> +			button = calc_button(r.activity.arg1);
> +			if (button < 0)
> +				break;
> +			input_report_key(visorinput_dev, button, 0);
> +			input_sync(visorinput_dev);
> +			break;
> +		case inputaction_mouse_button_click:
> +			button = calc_button(r.activity.arg1);
> +			if (button < 0)
> +				break;
> +			input_report_key(visorinput_dev, button, 1);
> +
> +			input_sync(visorinput_dev);
> +			input_report_key(visorinput_dev, button, 0);
> +			input_sync(visorinput_dev);
> +			break;
> +		case inputaction_mouse_button_dclick:
> +			button = calc_button(r.activity.arg1);
> +			if (button < 0)
> +				break;
> +			for (i = 0; i < 2; i++) {
> +				input_report_key(visorinput_dev, button, 1);
> +				input_sync(visorinput_dev);
> +				input_report_key(visorinput_dev, button, 0);
> +				input_sync(visorinput_dev);
> +			}
> +			break;
> +		case inputaction_wheel_rotate_away:
> +			if (!visorinput_dev2)
> +				goto out_locked;
> +			zmotion = r.activity.arg1;
> +			input_report_rel(visorinput_dev2, REL_WHEEL, 1);
> +			input_sync(visorinput_dev2);
> +			break;
> +		case inputaction_wheel_rotate_toward:
> +			if (!visorinput_dev2)
> +				goto out_locked;
> +			zmotion = r.activity.arg1;
> +			input_report_rel(visorinput_dev2, REL_WHEEL, -1);
> +			input_sync(visorinput_dev2);
> +			break;
> +		}
> +	}
> +out_locked:
> +	up_write(&devdata->lock_visor_dev);
> +}
> +
> +static int
> +visorhid_pause(struct visor_device *dev,
> +	       visorbus_state_complete_func complete_func)
> +{
> +	int rc;
> +	struct visorhid_devdata *devdata = dev_get_drvdata(&dev->device);
> +
> +	if (!devdata) {
> +		rc = -ENODEV;
> +		goto out;
> +	}
> +
> +	down_write(&devdata->lock_visor_dev);
> +	if (devdata->paused) {
> +		rc = -EBUSY;
> +		goto out_locked;
> +	}
> +	devdata->paused = true;
> +	complete_func(dev, 0);
> +	rc = 0;
> +out_locked:
> +	up_write(&devdata->lock_visor_dev);
> +out:
> +	return rc;
> +}
> +
> +static int
> +visorhid_resume(struct visor_device *dev,
> +		visorbus_state_complete_func complete_func)
> +{
> +	int rc;
> +	struct visorhid_devdata *devdata = dev_get_drvdata(&dev->device);
> +
> +	if (!devdata) {
> +		rc = -ENODEV;
> +		goto out;
> +	}
> +	down_write(&devdata->lock_visor_dev);
> +	if (!devdata->paused) {
> +		rc = -EBUSY;
> +		goto out_locked;
> +	}
> +	devdata->paused = false;
> +	complete_func(dev, 0);
> +	rc = 0;
> +out_locked:
> +	up_write(&devdata->lock_visor_dev);
> +out:
> +	return rc;
> +}
> +
> +static int
> +visorhid_init(void)
> +{
> +	int rc = 0;
> +
> +	spin_lock_init(&devnopool_lock);
> +	dev_no_pool = kzalloc(BITS_TO_LONGS(MAXDEVICES), GFP_KERNEL);

Again, use an idr/ida structure instead please.

> +	if (!dev_no_pool) {
> +		rc = -ENOMEM;
> +		goto cleanups;
> +	}
> +	visorbus_register_visor_driver(&visorhid_driver);

No error checking?

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux