Hi David,

This driver allows to write I/O drivers in user-space and feed the input
into the HID subsystem. It operates on the same level as USB-HID and
Bluetooth-HID (HIDP). It does not provide support to write special HID
device drivers but rather provides support for user-space I/O devices to
feed their data into the kernel HID subsystem. The HID subsystem then
loads the HID device drivers for the device and provides input-devices
based on the user-space HID I/O device.

<snip>

+#define UHID_NAME	"uhid"
+#define UHID_BUFSIZE	32
+
+struct uhid_device {
+	struct mutex devlock;
+	bool running;
+	struct device *parent;
+
+	__u8 *rd_data;
+	uint rd_size;
+
+	struct hid_device *hid;
+	struct uhid_event input_buf;
+
+	wait_queue_head_t waitq;
+	spinlock_t qlock;
+	struct uhid_event assemble;
+	__u8 head;
+	__u8 tail;
+	struct uhid_event outq[UHID_BUFSIZE];
+};

The kernel has a ringbuffer structure. Would it make sense to use that
one?

Or would using just a SKB queue be better?

+static void uhid_queue(struct uhid_device *uhid, const struct uhid_event *ev)
+{
+	__u8 newhead;
+
+	newhead = (uhid->head + 1) % UHID_BUFSIZE;
+
+	if (newhead != uhid->tail) {
+		memcpy(&uhid->outq[uhid->head], ev, sizeof(struct uhid_event));
+		uhid->head = newhead;
+		wake_up_interruptible(&uhid->waitq);
+	} else {
+		pr_warn("Output queue is full\n");
+	}
+}
+
+static int uhid_hid_start(struct hid_device *hid)
+{
+	struct uhid_device *uhid = hid->driver_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&uhid->qlock, flags);
+	memset(&uhid->assemble, 0, sizeof(uhid->assemble));
+	uhid->assemble.type = UHID_START;
+	uhid_queue(uhid, &uhid->assemble);
+	spin_unlock_irqrestore(&uhid->qlock, flags);
+
+	return 0;
+}
+
+static void uhid_hid_stop(struct hid_device *hid)
+{
+	struct uhid_device *uhid = hid->driver_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&uhid->qlock, flags);
+	memset(&uhid->assemble, 0, sizeof(uhid->assemble));
+	uhid->assemble.type = UHID_STOP;
+	uhid_queue(uhid, &uhid->assemble);
+	spin_unlock_irqrestore(&uhid->qlock, flags);
+
+	hid->claimed = 0;
+}
+
+static int uhid_hid_open(struct hid_device *hid)
+{
+	struct uhid_device *uhid = hid->driver_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&uhid->qlock, flags);
+	memset(&uhid->assemble, 0, sizeof(uhid->assemble));
+	uhid->assemble.type = UHID_OPEN;
+	uhid_queue(uhid, &uhid->assemble);
+	spin_unlock_irqrestore(&uhid->qlock, flags);
+
+	return 0;
+}
+
+static void uhid_hid_close(struct hid_device *hid)
+{
+	struct uhid_device *uhid = hid->driver_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&uhid->qlock, flags);
+	memset(&uhid->assemble, 0, sizeof(uhid->assemble));
+	uhid->assemble.type = UHID_CLOSE;
+	uhid_queue(uhid, &uhid->assemble);
+	spin_unlock_irqrestore(&uhid->qlock, flags);
+}
+
+static int uhid_hid_power(struct hid_device *hid, int level)
+{
+	/* TODO: Handle PM-hints. This isn't mandatory so we simply return 0
+	 * here.
+	 */
+
+	return 0;
+}
+
+static int uhid_hid_input(struct input_dev *input, unsigned int type,
+				unsigned int code, int value)
+{
+	struct hid_device *hid = input_get_drvdata(input);
+	struct uhid_device *uhid = hid->driver_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&uhid->qlock, flags);
+	memset(&uhid->assemble, 0, sizeof(uhid->assemble));
+
+	uhid->assemble.type = UHID_OUTPUT_EV;
+	uhid->assemble.u.output_ev.type = type;
+	uhid->assemble.u.output_ev.code = code;
+	uhid->assemble.u.output_ev.value = value;
+
+	uhid_queue(uhid, &uhid->assemble);
+	spin_unlock_irqrestore(&uhid->qlock, flags);
+
+	return 0;
+}
+
+static int uhid_hid_parse(struct hid_device *hid)
+{
+	struct uhid_device *uhid = hid->driver_data;
+
+	return hid_parse_report(hid, uhid->rd_data, uhid->rd_size);
+}
+
+static int uhid_hid_get_raw(struct hid_device *hid, unsigned char rnum,
+				__u8 *buf, size_t count, unsigned char rtype)
+{
+	/* TODO: we currently do not support this request. If we want this we
+	 * would need some kind of stream-locking but it isn't needed by the
+	 * main drivers, anyway.
+	 */
+
+	return -EOPNOTSUPP;
+}
+
+static int uhid_hid_output_raw(struct hid_device *hid, __u8 *buf, size_t count,
+				unsigned char report_type)
+{
+	struct uhid_device *uhid = hid->driver_data;
+	__u8 rtype;
+	unsigned long flags;
+
+	switch (report_type) {
+		case HID_FEATURE_REPORT:
+			rtype = UHID_FEATURE_REPORT;
+			break;
+		case HID_OUTPUT_REPORT:
+			rtype = UHID_OUTPUT_REPORT;
+			break;
+		default:
+			return -EINVAL;
+	}
+
+	if (count < 1 || count > UHID_DATA_MAX)
+		return -EINVAL;
+
+	spin_lock_irqsave(&uhid->qlock, flags);
+	memset(&uhid->assemble, 0, sizeof(uhid->assemble));
+
+	uhid->assemble.type = UHID_OUTPUT;
+	uhid->assemble.u.output.size = count;
+	uhid->assemble.u.output.rtype = rtype;
+	memcpy(uhid->assemble.u.output.data, buf, count);
+
+	uhid_queue(uhid, &uhid->assemble);
+	spin_unlock_irqrestore(&uhid->qlock, flags);
+
+	return count;
+}
+
+static struct hid_ll_driver uhid_hid_driver = {
+	.start = uhid_hid_start,
+	.stop = uhid_hid_stop,
+	.open = uhid_hid_open,
+	.close = uhid_hid_close,
+	.power = uhid_hid_power,
+	.hidinput_input_event = uhid_hid_input,
+	.parse = uhid_hid_parse,
+};
+
+static int uhid_dev_create(struct uhid_device *uhid,
+				const struct uhid_event *ev)
+{
+	struct hid_device *hid;
+	int ret;
+
+	ret = mutex_lock_interruptible(&uhid->devlock);
+	if (ret)
+		return ret;
+
+	if (uhid->running) {
+		ret = -EALREADY;
+		goto unlock;
+	}
+
+	uhid->rd_size = ev->u.create.rd_size;
+	uhid->rd_data = kzalloc(uhid->rd_size, GFP_KERNEL);
+	if (!uhid->rd_data) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	if (copy_from_user(uhid->rd_data, ev->u.create.rd_data,
+				uhid->rd_size)) {
+		ret = -EFAULT;
+		goto err_free;
+	}
+
+	hid = hid_allocate_device();
+	if (IS_ERR(hid)) {
+		ret = PTR_ERR(hid);
+		goto err_free;
+	}
+
+	strncpy(hid->name, ev->u.create.name, 128);
+	hid->name[127] = 0;
+	hid->ll_driver = &uhid_hid_driver;
+	hid->hid_get_raw_report = uhid_hid_get_raw;
+	hid->hid_output_raw_report = uhid_hid_output_raw;
+	hid->bus = ev->u.create.bus;
+	hid->vendor = ev->u.create.vendor;
+	hid->product = ev->u.create.product;
+	hid->version = ev->u.create.version;
+	hid->country = ev->u.create.country;
+	hid->phys[0] = 0;
+	hid->uniq[0] = 0;
+	hid->driver_data = uhid;
+	hid->dev.parent = uhid->parent;

Here we have to make sure that we have all required information provided
by userspace. Anything else that HID might require to work better. Or
that BR/EDR or LE provides additionally over USB.

+	uhid->hid = hid;
+	uhid->running = true;
+
+	ret = hid_add_device(hid);
+	if (ret) {
+		pr_err("Cannot register HID device\n");
+		goto err_hid;
+	}
+
+	mutex_unlock(&uhid->devlock);
+
+	return 0;
+
+err_hid:
+	hid_destroy_device(hid);
+	uhid->hid = NULL;
+	uhid->running = false;
+err_free:
+	kfree(uhid->rd_data);
+unlock:
+	mutex_unlock(&uhid->devlock);
+	return ret;
+}
+
+static int uhid_dev_destroy(struct uhid_device *uhid)
+{
+	int ret;
+
+	ret = mutex_lock_interruptible(&uhid->devlock);
+	if (ret)
+		return ret;
+
+	if (!uhid->running) {
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	hid_destroy_device(uhid->hid);
+	kfree(uhid->rd_data);
+	uhid->running = false;
+
+unlock:
+	mutex_unlock(&uhid->devlock);
+	return ret;
+}
+
+static int uhid_dev_input(struct uhid_device *uhid, struct uhid_event *ev)
+{
+	int ret;
+
+	ret = mutex_lock_interruptible(&uhid->devlock);
+	if (ret)
+		return ret;
+
+	if (!uhid->running) {
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data,
+				ev->u.input.size, 0);
+
+unlock:
+	mutex_unlock(&uhid->devlock);
+	return ret;
+}
+
+static int uhid_char_open(struct inode *inode, struct file *file)
+{
+	struct uhid_device *uhid;
+
+	uhid = kzalloc(sizeof(*uhid), GFP_KERNEL);
+	if (!uhid)
+		return -ENOMEM;
+
+	mutex_init(&uhid->devlock);
+	spin_lock_init(&uhid->qlock);
+	init_waitqueue_head(&uhid->waitq);
+	uhid->running = false;
+	uhid->parent = NULL;
+
+	file->private_data = uhid;
+	nonseekable_open(inode, file);
+
+	return 0;

return nonseekable_open().

+}
+
+static int uhid_char_release(struct inode *inode, struct file *file)
+{
+	struct uhid_device *uhid = file->private_data;
+
+	uhid_dev_destroy(uhid);
+	kfree(uhid);
+
+	return 0;
+}
+
+static ssize_t uhid_char_read(struct file *file, char __user *buffer,
+				size_t count, loff_t *ppos)
+{
+	struct uhid_device *uhid = file->private_data;
+	int ret;
+	unsigned long flags;
+	size_t len;
+
+	/* they need at least the "type" member of uhid_event */
+	if (count < sizeof(__u32))
+		return -EINVAL;
+
+try_again:
+	if (file->f_flags & O_NONBLOCK) {
+		if (uhid->head == uhid->tail)
+			return -EAGAIN;
+	} else {
+		ret = wait_event_interruptible(uhid->waitq,
+						uhid->head != uhid->tail);
+		if (ret)
+			return ret;
+	}
+
+	ret = mutex_lock_interruptible(&uhid->devlock);
+	if (ret)
+		return ret;
+
+	if (uhid->head == uhid->tail) {
+		mutex_unlock(&uhid->devlock);
+		goto try_again;
+	} else {
+		len = min(count, sizeof(*uhid->outq));
+		if (copy_to_user(buffer, &uhid->outq[uhid->tail], len)) {
+			ret = -EFAULT;
+		} else {
+			spin_lock_irqsave(&uhid->qlock, flags);
+			uhid->tail = (uhid->tail + 1) % UHID_BUFSIZE;
+			spin_unlock_irqrestore(&uhid->qlock, flags);
+		}
+	}
+
+	mutex_unlock(&uhid->devlock);
+	return ret ? ret : len;
+}
+
+static ssize_t uhid_char_write(struct file *file, const char __user *buffer,
+				size_t count, loff_t *ppos)
+{
+	struct uhid_device *uhid = file->private_data;
+	int ret;
+	size_t len;
+
+	/* we need at least the "type" member of uhid_event */
+	if (count < sizeof(__u32))
+		return -EINVAL;
+
+	memset(&uhid->input_buf, 0, sizeof(uhid->input_buf));
+	len = min(count, sizeof(uhid->input_buf));
+	if (copy_from_user(&uhid->input_buf, buffer, len))
+		return -EFAULT;
+
+	switch (uhid->input_buf.type) {
+		case UHID_CREATE:
+			ret = uhid_dev_create(uhid, &uhid->input_buf);
+			break;
+		case UHID_DESTROY:
+			ret = uhid_dev_destroy(uhid);
+			break;
+		case UHID_INPUT:
+			ret = uhid_dev_input(uhid, &uhid->input_buf);
+			break;
+		default:
+			ret = -EOPNOTSUPP;

switch and case should be on the same indentation.

Can we add an UHID_UNSPEC or UHID_INVALID here for the 0 value.

+	UHID_CREATE,
+	UHID_DESTROY,
+	UHID_START,
+	UHID_STOP,
+	UHID_OPEN,
+	UHID_CLOSE,
+	UHID_OUTPUT,
+	UHID_OUTPUT_EV,
+	UHID_INPUT,
+};
+
+struct uhid_create_req {
+	__u8 name[128];
+	__u8 __user *rd_data;
+	__u16 rd_size;
+
+	__u16 bus;
+	__u32 vendor;
+	__u32 product;
+	__u32 version;
+	__u32 country;
+} __attribute__((packed));

__packed please and all others.

+
+#define UHID_DATA_MAX 4096
+
+enum uhid_report_type {
+	UHID_FEATURE_REPORT,
+	UHID_OUTPUT_REPORT,
+};
+
+struct uhid_input_req {
+	__u8 data[UHID_DATA_MAX];
+	__u16 size;
+} __attribute__((packed));
+
+struct uhid_output_req {
+	__u8 data[UHID_DATA_MAX];
+	__u16 size;
+	__u8 rtype;
+} __attribute__((packed));
+
+struct uhid_output_ev_req {
+	__u16 type;
+	__u16 code;
+	__s32 value;
+} __attribute__((packed));
+
+struct uhid_event {
+	__u32 type;
+
+	union {
+		struct uhid_create_req create;
+		struct uhid_input_req input;
+		struct uhid_output_req output;
+		struct uhid_output_ev_req output_ev;
+	} u;
+} __attribute__((packed));

What about adding another __u32 len here? Just so we can do some extra
length checks if needed.

Regards

Marcel