[patch 0/2] add actuators support

[patch 0/2] add actuators support

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

I am owner of height adjustable desk and naturally, as it has an USB
interface, I need to controll it from my computer. Started to think
about what would be the best way, I realized that I need to introduce
a new driver class in kernel. The reason is a need to have one API
for all possible kinds of actuator devices (USB, I2C, gpio, etc).

So the first patch introduces generic actuator device class. Please
see the patch description for more info. Basically it allows device
drivers to register actuators providing set of ops. Also it exposes
actuator manipulation possibilities via generic Netlink interface
to userspace.

The second patch introduces driver based on the actuator driver class
for Linak USB2LIN device.

I created a library which wraps-up the generic netlink API so it could
be easily used by applications. The library comes with a utility called
"act" which can be used to manipulate the actuator devices. It has
iproute2 command line and stdout syntax.
Here there are couple of examples:

$ act dev show
usb/1-2/0: driver_name usb2lin type linear units um value 70200 move_state stopped
$ act dev move usb/1-2/0 value 100000
$ act dev stop usb/1-2/0

You can monitor the actuator status changes:

$ act mon
get: usb/1-2/0: driver_name usb2lin type linear units um value 70300 move_state positive
get: usb/1-2/0: driver_name usb2lin type linear units um value 99900 move_state stopped
get: usb/1-2/0: driver_name usb2lin type linear units um value 100300 move_state negative
get: usb/1-2/0: driver_name usb2lin type linear units um value 10100 move_state stopped
get: usb/1-2/0: driver_name usb2lin type linear units um value 10500 move_state positive
get: usb/1-2/0: driver_name usb2lin type linear units um value 99900 move_state stopped

The libactuator is available here:
https://git.kernel.org/pub/scm/linux/kernel/git/jpirko/libactuator.git/

This patchset is based on current net-next git tree.
I apologize, I'm not used to send patches outside netdev subsystem
so please tell me on top of which tree to rebase this if neede and
who to cc. Thanks!

Jiri Pirko (2):
  add support for generic actuator devices
  actuator: introduce Linak USB2LIN cable support

 MAINTAINERS                   |   7 +
 drivers/Kconfig               |   2 +
 drivers/Makefile              |   1 +
 drivers/actuator/Kconfig      |  26 +++
 drivers/actuator/Makefile     |   2 +
 drivers/actuator/actuator.c   | 410 +++++++++++++++++++++++++++++++++++++++
 drivers/actuator/usb2lin.c    | 433 ++++++++++++++++++++++++++++++++++++++++++
 drivers/hid/hid-core.c        |   1 +
 drivers/hid/hid-ids.h         |   3 +
 include/linux/actuator.h      |  43 +++++
 include/uapi/linux/actuator.h |  74 ++++++++
 11 files changed, 1002 insertions(+)
 create mode 100644 drivers/actuator/Kconfig
 create mode 100644 drivers/actuator/Makefile
 create mode 100644 drivers/actuator/actuator.c
 create mode 100644 drivers/actuator/usb2lin.c
 create mode 100644 include/linux/actuator.h
 create mode 100644 include/uapi/linux/actuator.h

-- 
2.9.3

[patch 1/2] add support for generic actuator devices

[Jiri Pirko]

Introduce support for generic actuator devices. Each device is
identified by bus_name, dev_name and index. First two are derived
according to the bus and address of the parent device. For example
"usb/1-2/". The last one is defined by driver itself. That is for cases
where it has multiple actuator devices connected via single parent.

Each driver registers actuators one by one. For each instance, ops are
defined. Currently there are 3 ops supported:
-status - called by core to driver in order to get actual actuator status
-move - called to initiate the movement of an actuator
-stop - called to stop the movement if any

A generic Netlink interface is exposed to userspace. It allows to get
actuator status, dump all available actuator devices, iniatiate move and
stop of an actuator. Also the netlink interface is used to send events
to userspace in case driver notifies core about status change.

libactuator project is there to provide generic Netlink wrapper and
custom util to manipulate actuators:
https://git.kernel.org/pub/scm/linux/kernel/git/jpirko/libactuator.git/

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 MAINTAINERS                   |   7 +
 drivers/Kconfig               |   2 +
 drivers/Makefile              |   1 +
 drivers/actuator/Kconfig      |  15 ++
 drivers/actuator/Makefile     |   1 +
 drivers/actuator/actuator.c   | 410 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/actuator.h      |  43 +++++
 include/uapi/linux/actuator.h |  74 ++++++++
 8 files changed, 553 insertions(+)
 create mode 100644 drivers/actuator/Kconfig
 create mode 100644 drivers/actuator/Makefile
 create mode 100644 drivers/actuator/actuator.c
 create mode 100644 include/linux/actuator.h
 create mode 100644 include/uapi/linux/actuator.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 1347726..029795a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5671,6 +5671,13 @@ W:	https://linuxtv.org
 S:	Maintained
 F:	drivers/media/radio/radio-gemtek*
 
+GENERIC ACTUATOR SUPPORT
+M:	Jiri Pirko <jiri@resnulli.us>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	drivers/actuator/
+F:	include/linux/actuator.h
+
 GENERIC GPIO I2C DRIVER
 M:	Haavard Skinnemoen <hskinnemoen@gmail.com>
 S:	Supported
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 505c676..7617173 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -208,4 +208,6 @@ source "drivers/tee/Kconfig"
 
 source "drivers/mux/Kconfig"
 
+source "drivers/actuator/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index dfdcda0..35e0e55 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -182,3 +182,4 @@ obj-$(CONFIG_FPGA)		+= fpga/
 obj-$(CONFIG_FSI)		+= fsi/
 obj-$(CONFIG_TEE)		+= tee/
 obj-$(CONFIG_MULTIPLEXER)	+= mux/
+obj-$(CONFIG_ACTUATOR)		+= actuator/
diff --git a/drivers/actuator/Kconfig b/drivers/actuator/Kconfig
new file mode 100644
index 0000000..6deac93
--- /dev/null
+++ b/drivers/actuator/Kconfig
@@ -0,0 +1,15 @@
+menuconfig ACTUATOR
+	tristate "Generic actuator support"
+	depends on NET
+	---help---
+	  This provides a support for generic actuator devices. For example
+	  all sorts of linear and rotary actuators.
+
+	  If you want this support, you should say Y here.
+
+	  This support can also be built as a module.  If so, the module
+	  will be called actuator.
+
+if ACTUATOR
+
+endif
diff --git a/drivers/actuator/Makefile b/drivers/actuator/Makefile
new file mode 100644
index 0000000..c45af36
--- /dev/null
+++ b/drivers/actuator/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_ACTUATOR)		+= actuator.o
diff --git a/drivers/actuator/actuator.c b/drivers/actuator/actuator.c
new file mode 100644
index 0000000..ce8a587
--- /dev/null
+++ b/drivers/actuator/actuator.c
@@ -0,0 +1,410 @@
+/*
+ * drivers/actuator/actuator.c - Generic actuator support
+ * Copyright (c) 2016-2017 Jiri Pirko <jiri@resnulli.us>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+#include <net/genetlink.h>
+#include <net/netlink.h>
+#include <uapi/linux/actuator.h>
+#include <linux/actuator.h>
+
+static LIST_HEAD(actuator_list);
+static DEFINE_MUTEX(actuator_mutex);
+
+struct actuator {
+	struct list_head list;
+	struct device *dev;
+	unsigned int index;
+	const struct actuator_ops *aco;
+	unsigned long priv[0];
+};
+
+void *actuator_priv(struct actuator *act)
+{
+	return act->priv;
+}
+EXPORT_SYMBOL_GPL(actuator_priv);
+
+static struct genl_family actuator_nl_family;
+
+enum actuator_multicast_groups {
+	ACTUATOR_MCGRP_CONFIG,
+};
+
+static const struct genl_multicast_group actuator_nl_mcgrps[] = {
+	[ACTUATOR_MCGRP_CONFIG] = { .name = ACTUATOR_GENL_MCGRP_CONFIG_NAME },
+};
+
+static int actuator_nl_put_handle(struct sk_buff *msg, struct actuator *act)
+{
+	if (nla_put_string(msg, ACTUATOR_ATTR_BUS_NAME, act->dev->bus->name))
+		return -EMSGSIZE;
+	if (nla_put_string(msg, ACTUATOR_ATTR_DEV_NAME, dev_name(act->dev)))
+		return -EMSGSIZE;
+	if (nla_put_u32(msg, ACTUATOR_ATTR_INDEX, act->index))
+		return -EMSGSIZE;
+	return 0;
+}
+
+static int actuator_nl_fill(struct sk_buff *msg, struct actuator *act,
+			    enum actuator_command cmd,
+			    struct actuator_status *status, bool fill_status,
+			    u32 portid, u32 seq, int flags)
+{
+	void *hdr;
+
+	if (!status && fill_status) {
+		struct actuator_status tmp_status;
+		int err;
+
+		err = act->aco->status(act, &tmp_status);
+		if (err)
+			return err;
+		status = &tmp_status;
+	}
+
+	hdr = genlmsg_put(msg, portid, seq, &actuator_nl_family, flags, cmd);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (actuator_nl_put_handle(msg, act))
+		goto nla_put_failure;
+	if (nla_put_string(msg, ACTUATOR_ATTR_DRIVER_NAME,
+			   act->aco->driver_name))
+		goto nla_put_failure;
+	if (nla_put_u8(msg, ACTUATOR_ATTR_TYPE, act->aco->type))
+		goto nla_put_failure;
+	if (nla_put_u8(msg, ACTUATOR_ATTR_UNITS, act->aco->units))
+		goto nla_put_failure;
+	if (fill_status) {
+		if (nla_put_u32(msg, ACTUATOR_ATTR_VALUE, status->value))
+			goto nla_put_failure;
+		if (nla_put_u8(msg, ACTUATOR_ATTR_MOVE_STATE,
+			       status->move_state))
+			goto nla_put_failure;
+	}
+
+	genlmsg_end(msg, hdr);
+	return 0;
+
+nla_put_failure:
+	genlmsg_cancel(msg, hdr);
+	return -EMSGSIZE;
+}
+
+static void actuator_notify(struct actuator *act, enum actuator_command cmd,
+			    struct actuator_status *status, bool fill_status)
+{
+	struct sk_buff *msg;
+	int err;
+
+	WARN_ON(cmd != ACTUATOR_CMD_GET &&
+		cmd != ACTUATOR_CMD_NEW &&
+		cmd != ACTUATOR_CMD_DEL);
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg) {
+		dev_err(act->dev, "Failed to allocate actuator notification message\n");
+		return;
+	}
+
+	err = actuator_nl_fill(msg, act, cmd, status, fill_status, 0, 0, 0);
+	if (err) {
+		dev_err(act->dev, "Failed to fill-up actuator notification message\n");
+		nlmsg_free(msg);
+		return;
+	}
+
+	genlmsg_multicast(&actuator_nl_family, msg, 0,
+			  ACTUATOR_MCGRP_CONFIG, GFP_KERNEL);
+}
+
+/**
+ *	actuator_notify_status - Send actuator notifications
+ *
+ *	@act: actuator
+ *	@status: status
+ *
+ *	Called by the driver to notify about async status change.
+ */
+void actuator_notify_status(struct actuator *act,
+			    struct actuator_status *status)
+{
+	actuator_notify(act, ACTUATOR_CMD_GET, status, true);
+}
+EXPORT_SYMBOL_GPL(actuator_notify_status);
+
+static void actuator_notify_new(struct actuator *act)
+{
+	actuator_notify(act, ACTUATOR_CMD_NEW, NULL, true);
+}
+
+static void actuator_notify_del(struct actuator *act)
+{
+	actuator_notify(act, ACTUATOR_CMD_DEL, NULL, false);
+}
+
+static int actuator_nl_cmd_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct actuator *act = info->user_ptr[0];
+	struct sk_buff *msg;
+	int err;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	err = actuator_nl_fill(msg, act, ACTUATOR_CMD_GET, NULL, true,
+			       info->snd_portid, info->snd_seq, 0);
+	if (err) {
+		nlmsg_free(msg);
+		return err;
+	}
+
+	return genlmsg_reply(msg, info);
+}
+
+static int actuator_nl_cmd_get_dumpit(struct sk_buff *msg,
+				      struct netlink_callback *cb)
+{
+	struct actuator *act;
+	int start = cb->args[0];
+	int idx = 0;
+	int err;
+
+	mutex_lock(&actuator_mutex);
+	list_for_each_entry(act, &actuator_list, list) {
+		if (idx < start) {
+			idx++;
+			continue;
+		}
+		err = actuator_nl_fill(msg, act, ACTUATOR_CMD_GET, NULL, true,
+				       NETLINK_CB(cb->skb).portid,
+				       cb->nlh->nlmsg_seq, NLM_F_MULTI);
+		if (err)
+			goto out;
+		idx++;
+	}
+out:
+	mutex_unlock(&actuator_mutex);
+
+	cb->args[0] = idx;
+	return msg->len;
+}
+
+static int actuator_nl_cmd_move_doit(struct sk_buff *skb,
+				     struct genl_info *info)
+{
+	struct actuator *act = info->user_ptr[0];
+	u32 value;
+
+	if (!info->attrs[ACTUATOR_ATTR_VALUE])
+		return -EINVAL;
+
+	value = nla_get_u32(info->attrs[ACTUATOR_ATTR_VALUE]);
+	if (act->aco->move)
+		return act->aco->move(act, value);
+	return -EOPNOTSUPP;
+}
+
+static int actuator_nl_cmd_stop_doit(struct sk_buff *skb,
+				     struct genl_info *info)
+{
+	struct actuator *act = info->user_ptr[0];
+
+	if (act->aco->stop)
+		return act->aco->stop(act);
+	return -EOPNOTSUPP;
+}
+
+static struct actuator *actuator_get_from_attrs(struct net *net,
+						struct nlattr **attrs)
+{
+	struct actuator *act;
+	char *busname;
+	char *devname;
+	u32 index;
+
+	if (!attrs[ACTUATOR_ATTR_BUS_NAME] ||
+	    !attrs[ACTUATOR_ATTR_DEV_NAME] ||
+	    !attrs[ACTUATOR_ATTR_INDEX])
+		return ERR_PTR(-EINVAL);
+
+	busname = nla_data(attrs[ACTUATOR_ATTR_BUS_NAME]);
+	devname = nla_data(attrs[ACTUATOR_ATTR_DEV_NAME]);
+	index = nla_get_u32(attrs[ACTUATOR_ATTR_INDEX]);
+
+	list_for_each_entry(act, &actuator_list, list) {
+		if (strcmp(act->dev->bus->name, busname) == 0 &&
+		    strcmp(dev_name(act->dev), devname) == 0 &&
+		    act->index == index)
+			return act;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+static struct actuator *actuator_get_from_info(struct genl_info *info)
+{
+	return actuator_get_from_attrs(genl_info_net(info), info->attrs);
+}
+
+static int actuator_nl_pre_doit(const struct genl_ops *ops,
+				struct sk_buff *skb, struct genl_info *info)
+{
+	struct actuator *act;
+
+	mutex_lock(&actuator_mutex);
+	act = actuator_get_from_info(info);
+	if (IS_ERR(act)) {
+		mutex_unlock(&actuator_mutex);
+		return PTR_ERR(act);
+	}
+	info->user_ptr[0] = act;
+	return 0;
+}
+
+static void actuator_nl_post_doit(const struct genl_ops *ops,
+				  struct sk_buff *skb, struct genl_info *info)
+{
+	mutex_unlock(&actuator_mutex);
+}
+
+static const struct nla_policy actuator_nl_policy[ACTUATOR_ATTR_MAX + 1] = {
+	[ACTUATOR_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING },
+	[ACTUATOR_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING },
+	[ACTUATOR_ATTR_INDEX] = { .type = NLA_U32 },
+	[ACTUATOR_ATTR_VALUE] = { .type = NLA_U32 },
+};
+
+static const struct genl_ops actuator_nl_ops[] = {
+	{
+		.cmd = ACTUATOR_CMD_GET,
+		.doit = actuator_nl_cmd_get_doit,
+		.dumpit = actuator_nl_cmd_get_dumpit,
+		.policy = actuator_nl_policy,
+		/* can be retrieved by unprivileged users */
+	},
+	{
+		.cmd = ACTUATOR_CMD_MOVE,
+		.doit = actuator_nl_cmd_move_doit,
+		.policy = actuator_nl_policy,
+		.flags = GENL_ADMIN_PERM,
+	},
+	{
+		.cmd = ACTUATOR_CMD_STOP,
+		.doit = actuator_nl_cmd_stop_doit,
+		.policy = actuator_nl_policy,
+		.flags = GENL_ADMIN_PERM,
+	},
+};
+
+static struct genl_family actuator_nl_family __ro_after_init = {
+	.name		= ACTUATOR_GENL_NAME,
+	.version	= ACTUATOR_GENL_VERSION,
+	.maxattr	= ACTUATOR_ATTR_MAX,
+	.pre_doit	= actuator_nl_pre_doit,
+	.post_doit	= actuator_nl_post_doit,
+	.module		= THIS_MODULE,
+	.ops		= actuator_nl_ops,
+	.n_ops		= ARRAY_SIZE(actuator_nl_ops),
+	.mcgrps		= actuator_nl_mcgrps,
+	.n_mcgrps	= ARRAY_SIZE(actuator_nl_mcgrps),
+};
+
+/**
+ *	actuator_alloc - Allocate new actuator instance resources
+ *
+ *	@aco: actuator instance ops
+ *
+ *	Allocate new actuator instance resources.
+ */
+struct actuator *actuator_alloc(const struct actuator_ops *aco)
+{
+	struct actuator *act;
+
+	WARN_ON(!aco->status || !aco->driver_name ||
+		aco->type == ACTUATOR_TYPE_UNSPEC ||
+		aco->units == ACTUATOR_UNITS_UNSPEC);
+	act = kzalloc(sizeof(struct actuator) + aco->priv_size, GFP_KERNEL);
+	if (!act)
+		return NULL;
+	act->aco = aco;
+	return act;
+}
+EXPORT_SYMBOL_GPL(actuator_alloc);
+
+/**
+ *	actuator_free - Free actuator instance resources
+ *
+ *	@act: actuator
+ */
+void actuator_free(struct actuator *act)
+{
+	kfree(act);
+}
+EXPORT_SYMBOL_GPL(actuator_free);
+
+/**
+ *	actuator_register - Register actuator instance
+ *
+ *	@act: actuator
+ *	@dev: parent device
+ *	@index: index within the parent device
+ */
+int actuator_register(struct actuator *act, struct device *dev,
+		      unsigned int index)
+{
+	act->dev = dev;
+	act->index = index;
+	mutex_lock(&actuator_mutex);
+	list_add_tail(&act->list, &actuator_list);
+	mutex_unlock(&actuator_mutex);
+	actuator_notify_new(act);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(actuator_register);
+
+/**
+ *	actuator_unregister - Unregister actuator instance
+ *
+ *	@act: actuator
+ */
+void actuator_unregister(struct actuator *act)
+{
+	mutex_lock(&actuator_mutex);
+	list_del(&act->list);
+	mutex_unlock(&actuator_mutex);
+	actuator_notify_del(act);
+}
+EXPORT_SYMBOL_GPL(actuator_unregister);
+
+static int __init actuator_init(void)
+{
+	return genl_register_family(&actuator_nl_family);
+}
+
+static void __exit actuator_exit(void)
+{
+	genl_unregister_family(&actuator_nl_family);
+}
+
+module_init(actuator_init);
+module_exit(actuator_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Jiri Pirko <jiri@resnulli.us>");
+MODULE_DESCRIPTION("Generic actuator support");
+MODULE_ALIAS_GENL_FAMILY(ACTUATOR_GENL_NAME);
diff --git a/include/linux/actuator.h b/include/linux/actuator.h
new file mode 100644
index 0000000..4086885
--- /dev/null
+++ b/include/linux/actuator.h
@@ -0,0 +1,43 @@
+/*
+ * include/linux/actuator.h - Generic actuator support
+ * Copyright (c) 2016-2017 Jiri Pirko <jiri@resnulli.us>
+ *
+ * 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.
+ */
+
+#ifndef _ACTUATOR_H_
+#define _ACTUATOR_H_
+
+#include <linux/types.h>
+#include <uapi/linux/actuator.h>
+
+struct actuator;
+
+struct actuator_status {
+	u32 value;
+	enum actuator_move_state move_state;
+};
+
+struct actuator_ops {
+	size_t priv_size;
+	const char *driver_name;
+	enum actuator_type type;
+	enum actuator_units units;
+	int (*status)(struct actuator *act, struct actuator_status *status);
+	int (*move)(struct actuator *act, u32 value);
+	int (*stop)(struct actuator *act);
+};
+
+void actuator_notify_status(struct actuator *act,
+			    struct actuator_status *status);
+void *actuator_priv(struct actuator *act);
+struct actuator *actuator_alloc(const struct actuator_ops *aco);
+void actuator_free(struct actuator *act);
+int actuator_register(struct actuator *act, struct device *dev,
+		      unsigned int index);
+void actuator_unregister(struct actuator *act);
+
+#endif
diff --git a/include/uapi/linux/actuator.h b/include/uapi/linux/actuator.h
new file mode 100644
index 0000000..4e2d958
--- /dev/null
+++ b/include/uapi/linux/actuator.h
@@ -0,0 +1,74 @@
+/*
+ * include/uapi/linux/actuator.h - Generic actuator support
+ * Copyright (c) 2017 Jiri Pirko <jiri@resnulli.us>
+ *
+ * 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.
+ */
+
+#ifndef _UAPI_LINUX_ACTUATOR_H_
+#define _UAPI_LINUX_ACTUATOR_H_
+
+#define ACTUATOR_GENL_NAME "actuator"
+#define ACTUATOR_GENL_VERSION 0x1
+#define ACTUATOR_GENL_MCGRP_CONFIG_NAME "config"
+
+enum actuator_command {
+	/* don't change the order or add anything between, this is ABI! */
+	ACTUATOR_CMD_UNSPEC,
+
+	ACTUATOR_CMD_GET,		/* can dump */
+	ACTUATOR_CMD_NEW,
+	ACTUATOR_CMD_DEL,
+
+	ACTUATOR_CMD_MOVE,
+	ACTUATOR_CMD_STOP,
+
+	/* add new commands above here */
+	__ACTUATOR_CMD_MAX,
+	ACTUATOR_CMD_MAX = __ACTUATOR_CMD_MAX - 1
+};
+
+enum actuator_type {
+	ACTUATOR_TYPE_UNSPEC,
+	ACTUATOR_TYPE_LINEAR,
+};
+
+enum actuator_units {
+	ACTUATOR_UNITS_UNSPEC,
+	ACTUATOR_UNITS_UM, /* micrometres */
+};
+
+enum actuator_move_state {
+	ACTUATOR_MOVE_STATE_UNSPEC,
+	ACTUATOR_MOVE_STATE_STOPPED,
+	ACTUATOR_MOVE_STATE_POSITIVE,
+	ACTUATOR_MOVE_STATE_NEGATIVE,
+};
+
+enum actuator_attr {
+	/* don't change the order or add anything between, this is ABI! */
+	ACTUATOR_ATTR_UNSPEC,
+
+	/* bus name + dev name + index together are a handle
+	 * for actuator entity
+	 */
+	ACTUATOR_ATTR_BUS_NAME,		/* string */
+	ACTUATOR_ATTR_DEV_NAME,		/* string */
+	ACTUATOR_ATTR_INDEX,		/* u32 */
+
+	ACTUATOR_ATTR_DRIVER_NAME,	/* string */
+	ACTUATOR_ATTR_TYPE,		/* u8, enum actuator_type */
+	ACTUATOR_ATTR_UNITS,		/* u8, enum actuator_units */
+	ACTUATOR_ATTR_VALUE,		/* u32, in units */
+	ACTUATOR_ATTR_MOVE_STATE,	/* u8, enum actuator_move_state */
+
+	/* add new attributes above here, update the policy in actuator.c */
+
+	__ACTUATOR_ATTR_MAX,
+	ACTUATOR_ATTR_MAX = __ACTUATOR_ATTR_MAX - 1
+};
+
+#endif /* _UAPI_LINUX_ACTUATOR_H_ */
-- 
2.9.3

[patch 2/2] actuator: introduce Linak USB2LIN cable support

[Jiri Pirko]

Benefit from the previously introduced actuator infrastructure and
introduce the first actuator device driver. This driver controls Linak
table moving actuators over USB2LIN usb cable.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/actuator/Kconfig   |  11 ++
 drivers/actuator/Makefile  |   1 +
 drivers/actuator/usb2lin.c | 433 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/hid/hid-core.c     |   1 +
 drivers/hid/hid-ids.h      |   3 +
 5 files changed, 449 insertions(+)
 create mode 100644 drivers/actuator/usb2lin.c

diff --git a/drivers/actuator/Kconfig b/drivers/actuator/Kconfig
index 6deac93..43150d8 100644
--- a/drivers/actuator/Kconfig
+++ b/drivers/actuator/Kconfig
@@ -12,4 +12,15 @@ menuconfig ACTUATOR
 
 if ACTUATOR
 
+menuconfig ACTUATOR_USB2LIN
+	tristate "Linak USB2LIN cable support"
+	depends on USB
+	---help---
+	  This provides a support for Linak USB2LIN devices.
+
+	  If you want this support, you should say Y here.
+
+	  This support can also be built as a module.  If so, the module
+	  will be called usb2lin.
+
 endif
diff --git a/drivers/actuator/Makefile b/drivers/actuator/Makefile
index c45af36..743dc18 100644
--- a/drivers/actuator/Makefile
+++ b/drivers/actuator/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_ACTUATOR)		+= actuator.o
+obj-$(CONFIG_ACTUATOR_USB2LIN)	+= usb2lin.o
diff --git a/drivers/actuator/usb2lin.c b/drivers/actuator/usb2lin.c
new file mode 100644
index 0000000..a39a994
--- /dev/null
+++ b/drivers/actuator/usb2lin.c
@@ -0,0 +1,433 @@
+/*
+ * drivers/actuator/usb2lin.c - Linak USB2LIN cable support
+ * Copyright (c) 2016-2017 Jiri Pirko <jiri@resnulli.us>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/workqueue.h>
+#include <linux/usb.h>
+#include <linux/actuator.h>
+#include "../../drivers/hid/hid-ids.h"
+
+static const char u2l_driver_name[] = "usb2lin";
+
+static const struct usb_device_id u2l_id_table[] = {
+	{ USB_DEVICE(USB_VENDOR_ID_LINAK, USB_DEVICE_ID_LINAK_USB2LIN) },
+	{ },
+};
+
+struct u2l_status {
+	__le16 header;
+	__le16 move_status;
+#define U2L_HEIGHT_MULTIPLIER 100
+	__le16 height;
+#define U2L_STATUS_MOVE_FLAGS_BLOCKED BIT(1)
+#define U2L_STATUS_MOVE_FLAGS_DIR BIT(7)
+	u8 move_flags;
+	u8 move_magic; /* not 0 when moving :O */
+	u8 unknown1[12];
+#define U2L_STATUS_TARGET_HEIGHT_MAGIC_STOPPED 0x8001
+	__le16 target_height;
+	u8 unknown2[42];
+} __packed;
+
+struct u2l_move {
+	u8 header;
+#define U2L_MOVE_TARGET_HEIGHT_MAGIC_UP 0x8000
+#define U2L_MOVE_TARGET_HEIGHT_MAGIC_DOWN 0x7fff
+	__le16 target_height[4]; /* unaligned access */
+	u8 pad[59]; /* zero padding */
+} __packed;
+
+struct u2l_init {
+	__le32 header;
+	u8 pad[60]; /* zero padding */
+} __packed;
+
+struct u2l {
+	struct mutex lock;
+	struct actuator *act;
+	struct usb_device *udev;
+	struct u2l_status last_status;
+	struct u2l_init init;
+	struct delayed_work dw;
+	struct {
+		bool in_progress;
+		bool started;
+		unsigned long start_jiffies;
+		u16 target_height;
+	} move;
+	union {
+		struct u2l_move status;
+		struct u2l_move move;
+	} msg;
+};
+
+struct u2l_usbmsg_info {
+	u8 request;
+	u8 requesttype;
+	u16 value;
+};
+
+enum u2l_usbmsg_type {
+	U2L_USBMSG_HELLO,
+	U2L_USBMSG_INIT,
+	U2L_USBMSG_STATUS,
+	U2L_USBMSG_MOVE,
+};
+
+#define U2L_USBMSG_REQUEST_TYPE_IN 0xa1
+#define U2L_USBMSG_REQUEST_TYPE_OUT 0x21
+
+static const struct u2l_usbmsg_info u2l_ubsmsg_infos[] = {
+	[U2L_USBMSG_HELLO] = {
+		.request = 0x0a,
+		.requesttype = U2L_USBMSG_REQUEST_TYPE_OUT,
+	},
+	[U2L_USBMSG_INIT] = {
+		.request = 0x09,
+		.requesttype = U2L_USBMSG_REQUEST_TYPE_OUT,
+		.value = 0x0303,
+	},
+	[U2L_USBMSG_STATUS] = {
+		.request = 0x01,
+		.requesttype = U2L_USBMSG_REQUEST_TYPE_IN,
+		.value = 0x0304,
+	},
+	[U2L_USBMSG_MOVE] = {
+		.request = 0x09,
+		.requesttype = U2L_USBMSG_REQUEST_TYPE_OUT,
+		.value = 0x0305,
+	},
+};
+
+#define U2L_USB_CONTROL_MSG_TIMEOUT 300
+
+static int u2l_usb_control_msg(struct u2l *u2l, enum u2l_usbmsg_type type,
+			       void *data, u16 size)
+{
+	const struct u2l_usbmsg_info *info = &u2l_ubsmsg_infos[type];
+	unsigned int pipe;
+	int err;
+
+	pipe = info->requesttype == U2L_USBMSG_REQUEST_TYPE_IN ?
+	       usb_rcvctrlpipe(u2l->udev, 0) :
+	       usb_sndctrlpipe(u2l->udev, 0);
+
+	err = usb_control_msg(u2l->udev, pipe, info->request, info->requesttype,
+			      info->value, 0, data, size,
+			      U2L_USB_CONTROL_MSG_TIMEOUT);
+	return err < 0 ? err : 0;
+}
+
+static int u2l_cmd_hello(struct u2l *u2l)
+{
+	return u2l_usb_control_msg(u2l, U2L_USBMSG_HELLO, NULL, 0);
+}
+
+#define U2L_USBMSG_INIT_HEADER_MAGIC 0xfb000403
+
+static int u2l_cmd_init(struct u2l *u2l)
+{
+	u2l->init.header = cpu_to_le32(U2L_USBMSG_INIT_HEADER_MAGIC);
+	return u2l_usb_control_msg(u2l, U2L_USBMSG_INIT,
+				   &u2l->init, sizeof(u2l->init));
+}
+
+static int u2l_cmd_status(struct u2l *u2l, struct u2l_status *status)
+{
+	int err;
+
+	err = u2l_usb_control_msg(u2l, U2L_USBMSG_STATUS,
+				  &u2l->msg.status, sizeof(*status));
+	if (err)
+		return err;
+	memcpy(status, &u2l->msg.status, sizeof(*status));
+	dev_dbg(&u2l->udev->dev, "move_status %04x, height %04x (%u), move_flags %02x (%s%s), move_magic %02x, target_height %04x (%u)\n",
+		status->move_status, status->height, status->height,
+		status->move_flags,
+		status->move_flags & U2L_STATUS_MOVE_FLAGS_BLOCKED ?
+				     "blocked " : "",
+		status->move_flags & U2L_STATUS_MOVE_FLAGS_DIR ?
+				     "down" : "up",
+		status->move_magic,
+		status->target_height, status->target_height);
+	return 0;
+}
+
+static void u2l_cmd_last_status_save(struct u2l *u2l, struct u2l_status *status)
+{
+	memcpy(&u2l->last_status, status, sizeof(u2l->last_status));
+}
+
+#define U2L_USBMSG_MOVE_HEADER_MAGIC 0x05
+
+static int u2l_cmd_move(struct u2l *u2l, u16 target_height)
+{
+	__le16 tmp;
+	int i;
+
+	u2l->msg.move.header = U2L_USBMSG_MOVE_HEADER_MAGIC;
+	tmp = cpu_to_le16(target_height);
+	for (i = 0; i < 4; i++)
+		memcpy(&u2l->msg.move.target_height[i], &tmp, sizeof(tmp));
+
+	return u2l_usb_control_msg(u2l, U2L_USBMSG_MOVE,
+				   &u2l->msg.move, sizeof(u2l->move));
+}
+
+static void u2l_actuator_status_fill(struct u2l *u2l,
+				     struct actuator_status *act_status,
+				     struct u2l_status *status)
+{
+	act_status->value = le16_to_cpu(status->height) * U2L_HEIGHT_MULTIPLIER;
+	if (!status->move_magic) {
+		act_status->move_state = ACTUATOR_MOVE_STATE_STOPPED;
+	} else {
+		act_status->move_state =
+			status->move_flags & U2L_STATUS_MOVE_FLAGS_DIR ?
+			ACTUATOR_MOVE_STATE_NEGATIVE :
+			ACTUATOR_MOVE_STATE_POSITIVE;
+	}
+}
+
+static int u2l_actuator_status(struct actuator *act,
+			       struct actuator_status *act_status)
+{
+	struct u2l *u2l = actuator_priv(act);
+	struct u2l_status status;
+	int err;
+
+	mutex_lock(&u2l->lock);
+	err = u2l_cmd_status(u2l, &status);
+	mutex_unlock(&u2l->lock);
+	if (err)
+		return err;
+	u2l_actuator_status_fill(u2l, act_status, &status);
+	return 0;
+}
+
+#define U2L_MOVE_START_DELAY 500 /* ms */
+
+static void u2l_check_move(struct u2l *u2l, struct u2l_status *status)
+{
+	if (!u2l->move.in_progress)
+		return;
+
+	if (!u2l->move.started) {
+		if (status->move_magic)
+			u2l->move.started = true;
+		else if (jiffies_to_msecs(jiffies - u2l->move.start_jiffies) >
+			 U2L_MOVE_START_DELAY) {
+			goto finish; /* Move start timed-out */
+		}
+	} else if (!status->move_magic) {
+		goto finish; /* We are at the destination or limit */
+	}
+
+	u2l_cmd_move(u2l, u2l->move.target_height);
+	return;
+finish:
+	u2l->move.in_progress = false;
+}
+
+static void u2l_check_notify(struct u2l *u2l, struct u2l_status *status)
+{
+	struct actuator_status act_status;
+
+	if (!!status->move_magic != !!u2l->last_status.move_magic) {
+		u2l_actuator_status_fill(u2l, &act_status, status);
+		actuator_notify_status(u2l->act, &act_status);
+	}
+}
+
+#define U2L_WORK_DELAY 100 /* ms */
+
+static void u2l_schedule_work(struct u2l *u2l)
+{
+	schedule_delayed_work(&u2l->dw, U2L_WORK_DELAY);
+}
+
+static void u2l_work(struct work_struct *work)
+{
+	struct u2l_status status;
+	struct u2l *u2l;
+	int err;
+
+	u2l = container_of(work, struct u2l, dw.work);
+
+	mutex_lock(&u2l->lock);
+	err = u2l_cmd_status(u2l, &status);
+	if (err)
+		goto unlock;
+	u2l_check_move(u2l, &status);
+	u2l_check_notify(u2l, &status);
+	u2l_cmd_last_status_save(u2l, &status);
+	u2l_schedule_work(u2l);
+unlock:
+	mutex_unlock(&u2l->lock);
+}
+
+static int u2l_actuator_move(struct actuator *act, u32 value)
+{
+	struct u2l *u2l = actuator_priv(act);
+	int err;
+
+	value /= U2L_HEIGHT_MULTIPLIER;
+	if (value < 0 || value > 0xFFFF)
+		return -EINVAL;
+
+	mutex_lock(&u2l->lock);
+
+	u2l->move.in_progress = true;
+	u2l->move.started = false;
+	u2l->move.start_jiffies = jiffies;
+	u2l->move.target_height = value;
+
+	/* Do move cmd twice, first one would wake up the control in case
+	 * it sleeps. Unfortunately there is no known way to find out.
+	 */
+	err = u2l_cmd_move(u2l, u2l->move.target_height);
+	if (err)
+		goto unlock;
+	err = u2l_cmd_move(u2l, u2l->move.target_height);
+	if (err)
+		goto unlock;
+
+unlock:
+	mutex_unlock(&u2l->lock);
+	return err;
+}
+
+static int u2l_actuator_stop(struct actuator *act)
+{
+	struct u2l *u2l = actuator_priv(act);
+	struct u2l_status status;
+	u16 wakeup_target_height;
+	int err;
+
+	mutex_lock(&u2l->lock);
+
+	err = u2l_cmd_status(u2l, &status);
+	if (err)
+		goto unlock;
+	if (le16_to_cpu(status.target_height) ==
+	    U2L_STATUS_TARGET_HEIGHT_MAGIC_STOPPED)
+		goto unlock;
+
+	wakeup_target_height =
+		status.move_flags & U2L_STATUS_MOVE_FLAGS_DIR ?
+		U2L_MOVE_TARGET_HEIGHT_MAGIC_UP :
+		U2L_MOVE_TARGET_HEIGHT_MAGIC_DOWN;
+
+	err = u2l_cmd_move(u2l, wakeup_target_height);
+
+	u2l->move.in_progress = false;
+
+unlock:
+	mutex_unlock(&u2l->lock);
+	return err;
+}
+
+static int u2l_init(struct u2l *u2l)
+{
+	struct u2l_status status;
+	u16 current_height;
+	int err;
+
+	err = u2l_cmd_hello(u2l);
+	if (err)
+		return err;
+
+	err = u2l_cmd_status(u2l, &status);
+	if (err)
+		return err;
+	current_height = le16_to_cpu(status.height);
+	u2l_cmd_last_status_save(u2l, &status);
+
+	err = u2l_cmd_init(u2l);
+	if (err)
+		return err;
+
+	return u2l_cmd_move(u2l, current_height);
+}
+
+static const struct actuator_ops u2l_actuator_ops = {
+	.priv_size = sizeof(struct u2l),
+	.driver_name = u2l_driver_name,
+	.type = ACTUATOR_TYPE_LINEAR,
+	.units = ACTUATOR_UNITS_UM,
+	.status = u2l_actuator_status,
+	.move = u2l_actuator_move,
+	.stop = u2l_actuator_stop,
+};
+
+static int u2l_probe(struct usb_interface *interface,
+		     const struct usb_device_id *id)
+{
+	struct usb_device *udev = interface_to_usbdev(interface);
+	struct actuator *act;
+	struct u2l *u2l;
+	int err;
+
+	act = actuator_alloc(&u2l_actuator_ops);
+	if (!act)
+		return -ENOMEM;
+	u2l = actuator_priv(act);
+	u2l->act = act;
+	u2l->udev = udev;
+	mutex_init(&u2l->lock);
+	INIT_DELAYED_WORK(&u2l->dw, u2l_work);
+
+	err = u2l_init(u2l);
+	if (err) {
+		dev_err(&u2l->udev->dev, "Initialization failed\n");
+		goto err_init;
+	}
+
+	err = actuator_register(act, &udev->dev, 0);
+	if (err)
+		goto err_actuator_register;
+
+	usb_set_intfdata(interface, act);
+	u2l_schedule_work(u2l);
+	return 0;
+
+err_actuator_register:
+err_init:
+	actuator_free(act);
+	return err;
+}
+
+static void u2l_disconnect(struct usb_interface *interface)
+{
+	struct actuator *act = usb_get_intfdata(interface);
+	struct u2l *u2l = actuator_priv(act);
+
+	cancel_delayed_work_sync(&u2l->dw);
+	actuator_unregister(act);
+	actuator_free(act);
+}
+
+static struct usb_driver u2l_driver = {
+	.name = u2l_driver_name,
+	.probe = u2l_probe,
+	.disconnect = u2l_disconnect,
+	.id_table = u2l_id_table,
+};
+
+module_usb_driver(u2l_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Jiri Pirko <jiri@resnulli.us>");
+MODULE_DESCRIPTION("Linak USB2LIN cable support");
+MODULE_DEVICE_TABLE(usb, u2l_id_table);
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 6fd01a6..3311a96 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2740,6 +2740,7 @@ static const struct hid_device_id hid_ignore_list[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_MCT) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HYBRID) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HEATCONTROL) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LINAK, USB_DEVICE_ID_LINAK_USB2LIN) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_BEATPAD) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1024LS) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1208LS) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 3d911bf..0650fe9 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -663,6 +663,9 @@
 #define USB_DEVICE_ID_LG_MULTITOUCH	0x0064
 #define USB_DEVICE_ID_LG_MELFAS_MT	0x6007
 
+#define USB_VENDOR_ID_LINAK		0x12d3
+#define USB_DEVICE_ID_LINAK_USB2LIN	0x0002
+
 #define USB_VENDOR_ID_LOGITECH		0x046d
 #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
 #define USB_DEVICE_ID_LOGITECH_T651	0xb00c
-- 
2.9.3

Re: [patch 0/2] add actuators support

[Greg KH]

On Mon, Jul 17, 2017 at 06:19:12PM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> I am owner of height adjustable desk and naturally, as it has an USB
> interface, I need to controll it from my computer. Started to think
> about what would be the best way, I realized that I need to introduce
> a new driver class in kernel. The reason is a need to have one API
> for all possible kinds of actuator devices (USB, I2C, gpio, etc).

Why does this have to be a kernel driver at all?  Your USB driver should
really just be a simple userspace application (use libusb to have it
work on all operating systems.)

While the idea of a "generic class" is nice, given that there is only
one device of this type out there so far, and it's a USB one, kind of
implies that a generic class isn't needed just yet.

Oh, and it's nice to cc: the linux-usb@vger mailing list for USB patches
in the future :)

thanks,

greg k-h

Re: [patch 0/2] add actuators support

[Jiri Pirko]

Mon, Jul 17, 2017 at 06:28:38PM CEST, gregkh@linuxfoundation.org wrote:
>On Mon, Jul 17, 2017 at 06:19:12PM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> I am owner of height adjustable desk and naturally, as it has an USB
>> interface, I need to controll it from my computer. Started to think
>> about what would be the best way, I realized that I need to introduce
>> a new driver class in kernel. The reason is a need to have one API
>> for all possible kinds of actuator devices (USB, I2C, gpio, etc).
>
>Why does this have to be a kernel driver at all?  Your USB driver should
>really just be a simple userspace application (use libusb to have it
>work on all operating systems.)

Yeah, I was thinking about it as well. To have some kind of single
purpose app like sispmctl (control of surge protector). But that would
limit you only for this specific device. And for multiple devices you
would have to have multiple apps.

So it seems to me like a suitable solution to have this as a driver
class.


>
>While the idea of a "generic class" is nice, given that there is only
>one device of this type out there so far, and it's a USB one, kind of
>implies that a generic class isn't needed just yet.

We have to start somewhere. The world is full of actuators. Connected
over diferent buses. I think it is good to have a class,lib,app so the
drivers could be easily added, apps could be written using single api.


>
>Oh, and it's nice to cc: the linux-usb@vger mailing list for USB patches
>in the future :)

Will do.


>
>thanks,
>
>greg k-h

Re: [patch 0/2] add actuators support

[Greg KH]

On Mon, Jul 17, 2017 at 06:34:58PM +0200, Jiri Pirko wrote:
> Mon, Jul 17, 2017 at 06:28:38PM CEST, gregkh@linuxfoundation.org wrote:
> >On Mon, Jul 17, 2017 at 06:19:12PM +0200, Jiri Pirko wrote:
> >> From: Jiri Pirko <jiri@mellanox.com>
> >> 
> >> I am owner of height adjustable desk and naturally, as it has an USB
> >> interface, I need to controll it from my computer. Started to think
> >> about what would be the best way, I realized that I need to introduce
> >> a new driver class in kernel. The reason is a need to have one API
> >> for all possible kinds of actuator devices (USB, I2C, gpio, etc).
> >
> >Why does this have to be a kernel driver at all?  Your USB driver should
> >really just be a simple userspace application (use libusb to have it
> >work on all operating systems.)
> 
> Yeah, I was thinking about it as well. To have some kind of single
> purpose app like sispmctl (control of surge protector). But that would
> limit you only for this specific device. And for multiple devices you
> would have to have multiple apps.
> 
> So it seems to me like a suitable solution to have this as a driver
> class.

For USB devices, especially ones as simple as this, we don't want them
to have kernel drivers.  It's just overkill.

And never create a class if you only have one type of device, otherwise
you don't know if you really have it properly defined or not.  3 is a
good number to start with.

sorry,

greg k-h

Re: [patch 0/2] add actuators support

[Jiri Pirko]

Mon, Jul 17, 2017 at 06:40:06PM CEST, gregkh@linuxfoundation.org wrote:
>On Mon, Jul 17, 2017 at 06:34:58PM +0200, Jiri Pirko wrote:
>> Mon, Jul 17, 2017 at 06:28:38PM CEST, gregkh@linuxfoundation.org wrote:
>> >On Mon, Jul 17, 2017 at 06:19:12PM +0200, Jiri Pirko wrote:
>> >> From: Jiri Pirko <jiri@mellanox.com>
>> >> 
>> >> I am owner of height adjustable desk and naturally, as it has an USB
>> >> interface, I need to controll it from my computer. Started to think
>> >> about what would be the best way, I realized that I need to introduce
>> >> a new driver class in kernel. The reason is a need to have one API
>> >> for all possible kinds of actuator devices (USB, I2C, gpio, etc).
>> >
>> >Why does this have to be a kernel driver at all?  Your USB driver should
>> >really just be a simple userspace application (use libusb to have it
>> >work on all operating systems.)
>> 
>> Yeah, I was thinking about it as well. To have some kind of single
>> purpose app like sispmctl (control of surge protector). But that would
>> limit you only for this specific device. And for multiple devices you
>> would have to have multiple apps.
>> 
>> So it seems to me like a suitable solution to have this as a driver
>> class.
>
>For USB devices, especially ones as simple as this, we don't want them
>to have kernel drivers.  It's just overkill.
>
>And never create a class if you only have one type of device, otherwise
>you don't know if you really have it properly defined or not.  3 is a
>good number to start with.

You are making it hard for me :) Have to find other 2 then. And I thought
that this spare time quest is coming to an end... 


>
>sorry,
>
>greg k-h