[PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[Michal Malý]

This mini series adds a simple skeleton module whose only purpose is to bring
devices that at first appear as a generic USB device into another mode that
can be handled by a more specific subsystem.

This patch was originally requested by Dmitry, reasoning that loading the
entire xpad module just to switch a Logitech G920 wheel into HID mode is
excessive and does not make much sense.

The module can be extended to handle any other USB device that might require
such a switch.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>

Michal Malý (2):
  Add usb_skelswitch skeleton module to do basic initialization of
    devices that     at first appear as a generic USB device.
  Use usb_skelswitch module to switch Logitech G920 Racing Wheel to HID 
       mode.

 drivers/usb/Kconfig                 |   2 +
 drivers/usb/common/Kconfig          |  17 +++++
 drivers/usb/common/Makefile         |   2 +
 drivers/usb/common/usb-skelswitch.c | 141 ++++++++++++++++++++++++++++++++++++
 4 files changed, 162 insertions(+)
 create mode 100644 drivers/usb/common/Kconfig
 create mode 100644 drivers/usb/common/usb-skelswitch.c

-- 
2.7.0

[PATCH 1/2] Add usb_skelswitch skeleton module to do basic initialization of devices that at first appear as a generic USB device.

[Michal Malý]

This gets rid of the need to handle such devices in more specific
drivers that will be loaded for no purpose other than to do some basic
initialization on the device.

Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/usb/Kconfig                 |  2 +
 drivers/usb/common/Kconfig          | 15 +++++++
 drivers/usb/common/Makefile         |  2 +
 drivers/usb/common/usb-skelswitch.c | 81 +++++++++++++++++++++++++++++++++++++
 4 files changed, 100 insertions(+)
 create mode 100644 drivers/usb/common/Kconfig
 create mode 100644 drivers/usb/common/usb-skelswitch.c

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8ed451d..3de5d35 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -78,6 +78,8 @@ if USB
 
 source "drivers/usb/core/Kconfig"
 
+source "drivers/usb/common/Kconfig"
+
 source "drivers/usb/mon/Kconfig"
 
 source "drivers/usb/wusbcore/Kconfig"
diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
new file mode 100644
index 0000000..6d9ec79
--- /dev/null
+++ b/drivers/usb/common/Kconfig
@@ -0,0 +1,15 @@
+#
+# USB common modules
+#
+
+if USB_COMMON
+
+config USB_SKELSWITCH
+	tristate "Simple USB module for basic device initialization"
+	help
+	  Simple module that performs basic initialization on devices that at first
+	  appear as a generic USB device.
+
+	  Say Y if you intend to use a device that requires this initial switch.
+
+endif # USB_COMMON
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 6bbb3ec..5e7c15a 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -8,3 +8,5 @@ usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 
 obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
 obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
+
+obj-$(CONFIG_USB_SKELSWITCH)	   += usb-skelswitch.o
diff --git a/drivers/usb/common/usb-skelswitch.c b/drivers/usb/common/usb-skelswitch.c
new file mode 100644
index 0000000..ae72068
--- /dev/null
+++ b/drivers/usb/common/usb-skelswitch.c
@@ -0,0 +1,81 @@
+#include <linux/module.h>
+#include <linux/usb.h>
+
+MODULE_LICENSE("GPL");
+
+struct usb_skelswitch_product {
+	const u16 idProduct;
+	int (*action)(struct usb_interface *);
+};
+
+struct usb_skelswitch_vendor {
+	const u16 idVendor;
+	const struct usb_skelswitch_product *products;
+};
+
+static const struct usb_device_id usb_skelswitch_table[] = {
+	{ }
+};
+
+static const struct usb_skelswitch_vendor usb_skelswitch_vendors[] = {
+	{ 0, NULL }
+};
+
+static int usb_skelswitch_process_products(struct usb_interface *intf, const struct usb_skelswitch_product *products, const u16 idProduct)
+{
+	size_t idx = 0;
+	const struct usb_device *udev = interface_to_usbdev(intf);
+
+	while (1) {
+		const struct usb_skelswitch_product *product = &products[idx];
+
+		if (product->idProduct == 0) {
+			dev_err(&udev->dev, "usb_skelswitch: Unhandled idProduct 0x%04x\n", idProduct);
+			return -EINVAL;
+		}
+
+		if (product->idProduct == idProduct) {
+			if (product->action)
+				return product->action(intf);
+			else
+				return 0;
+		}
+
+		idx++;
+	}
+}
+
+static int usb_skelswitch_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	size_t idx = 0;
+	const struct usb_device *udev = interface_to_usbdev(intf);
+
+	while (1) {
+		const struct usb_skelswitch_vendor *vendor = &usb_skelswitch_vendors[idx];
+
+		if (vendor->idVendor == 0) {
+			dev_err(&udev->dev, "Unhandled idVendor 0x%04x", id->idVendor);
+			return -EINVAL;
+		}
+
+		if (id->idVendor == vendor->idVendor)
+			return usb_skelswitch_process_products(intf, vendor->products, id->idProduct);
+
+		idx++;
+	}
+}
+
+static void usb_skelswitch_disconnect(struct usb_interface *intf)
+{
+	(void)intf;
+}
+
+static struct usb_driver usb_skelswitch_driver = {
+	.disconnect = usb_skelswitch_disconnect,
+	.name = "usb_skelswitch",
+	.probe = usb_skelswitch_probe,
+	.id_table = usb_skelswitch_table
+};
+
+module_usb_driver(usb_skelswitch_driver);
+
-- 
2.7.0

[PATCH 2/2] Use usb_skelswitch module to switch Logitech G920 Racing Wheel to HID mode.

[Michal Malý]

Tested-by: Elias Vanderstuyft <elias.vds@gmail.com>
Signed-off-by: Michal Malý <madcatxster@devoid-pointer.net>
---
 drivers/usb/common/Kconfig          |  2 ++
 drivers/usb/common/usb-skelswitch.c | 60 +++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
index 6d9ec79..8ae9a1e 100644
--- a/drivers/usb/common/Kconfig
+++ b/drivers/usb/common/Kconfig
@@ -11,5 +11,7 @@ config USB_SKELSWITCH
 	  appear as a generic USB device.
 
 	  Say Y if you intend to use a device that requires this initial switch.
+	  Devices that currently require this module:
+	   - Logitech G920 Racing Wheel
 
 endif # USB_COMMON
diff --git a/drivers/usb/common/usb-skelswitch.c b/drivers/usb/common/usb-skelswitch.c
index ae72068..fc85c70 100644
--- a/drivers/usb/common/usb-skelswitch.c
+++ b/drivers/usb/common/usb-skelswitch.c
@@ -14,10 +14,70 @@ struct usb_skelswitch_vendor {
 };
 
 static const struct usb_device_id usb_skelswitch_table[] = {
+	{ USB_DEVICE(0x046d, 0xc261) },
 	{ }
 };
 
+MODULE_DEVICE_TABLE(usb, usb_skelswitch_table);
+
+static int usb_skelswitch_lg_g920(struct usb_interface *intf)
+{
+	struct usb_host_interface *iface_desc;
+	struct usb_endpoint_descriptor *endpoint;
+	struct usb_device *udev;
+	int idx;
+	int ret;
+	int xferred;
+	size_t buffer_size;
+	unsigned char cmd[] = { 0x0f, 0x00, 0x01, 0x01, 0x42 };
+	const size_t cmd_len = ARRAY_SIZE(cmd);
+	u8 intr_out_addr = 0;
+
+	udev = usb_get_dev(interface_to_usbdev(intf));
+	iface_desc = intf->cur_altsetting;
+	for (idx = 0; idx < iface_desc->desc.bNumEndpoints; idx++) {
+		endpoint = &iface_desc->endpoint[idx].desc;
+
+		if (usb_endpoint_is_int_out(endpoint)) {
+			intr_out_addr = endpoint->bEndpointAddress;
+			buffer_size = usb_endpoint_maxp(endpoint);
+			break;
+		}
+	}
+
+	if (!intr_out_addr) {
+		dev_err(&udev->dev, "Logitech G920 - No interrupt out endpoint found");
+		return -ENODEV;
+	}
+
+	if (buffer_size < cmd_len) {
+		dev_err(&udev->dev, "usb_skelswitch: Logitech G920 - Output buffer is too small");
+		return -ENODEV;
+	}
+
+
+	ret = usb_interrupt_msg(udev, usb_sndintpipe(udev, intr_out_addr),
+				cmd, cmd_len, &xferred, USB_CTRL_SET_TIMEOUT);
+
+	if (ret) {
+		dev_err(&udev->dev, "LG G920: Failed to submit URB, errno: %d", ret);
+		return ret;
+	}
+	if (xferred != cmd_len) {
+		dev_err(&udev->dev, "LG G920: Incorrect number of bytes transferred: %d", xferred);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static const struct usb_skelswitch_product usb_skelswitch_logitech_devs[] = {
+	{ 0xc261, usb_skelswitch_lg_g920 },
+	{ 0, NULL }
+};
+
 static const struct usb_skelswitch_vendor usb_skelswitch_vendors[] = {
+	{ 0x046d, usb_skelswitch_logitech_devs },
 	{ 0, NULL }
 };
 
-- 
2.7.0

Re: [PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[Bjørn Mork]

Michal Malý <madcatxster@devoid-pointer.net> writes:

> This mini series adds a simple skeleton module whose only purpose is to bring
> devices that at first appear as a generic USB device into another mode that
> can be handled by a more specific subsystem.
>
> This patch was originally requested by Dmitry, reasoning that loading the
> entire xpad module just to switch a Logitech G920 wheel into HID mode is
> excessive and does not make much sense.
>
> The module can be extended to handle any other USB device that might require
> such a switch.

Can this switching be done in userspace? That's what we normally do, ref
usb_modeswitch.


Bjørn

Re: [PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[Michal Malý]

On sobota 23. ledna 2016 13:46:32 CET Bjørn Mork wrote:
> Michal Malý <madcatxster@devoid-pointer.net> writes:
> > This mini series adds a simple skeleton module whose only purpose is to
> > bring devices that at first appear as a generic USB device into another
> > mode that can be handled by a more specific subsystem.
> > 
> > This patch was originally requested by Dmitry, reasoning that loading the
> > entire xpad module just to switch a Logitech G920 wheel into HID mode is
> > excessive and does not make much sense.
> > 
> > The module can be extended to handle any other USB device that might
> > require such a switch.
> 
> Can this switching be done in userspace? That's what we normally do, ref
> usb_modeswitch.

I briefly considered leaving the switch up to the userspace and handling the 
device in the kernel only once it's been switched. I am however uncertain how 
to advertise this to the users. Writing a tiny app that would currently handle 
just one device seems like an overkill, abusing usb_modeswitch seems counter-
intuitive as it's purpose is to handle various USB modems. Having a tiny 
module in the kernel looks like the most straightforward thing to do as far as 
user experience is concerned.

I would not object to deferring the switch to userspace as long as there is an 
easy way how to communicate the need for a small switching tool to the users.

Michal

Re: [PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[Greg KH]

On Sat, Jan 23, 2016 at 04:56:27PM +0100, Michal Malý wrote:
> On sobota 23. ledna 2016 13:46:32 CET Bjørn Mork wrote:
> > Michal Malý <madcatxster@devoid-pointer.net> writes:
> > > This mini series adds a simple skeleton module whose only purpose is to
> > > bring devices that at first appear as a generic USB device into another
> > > mode that can be handled by a more specific subsystem.
> > > 
> > > This patch was originally requested by Dmitry, reasoning that loading the
> > > entire xpad module just to switch a Logitech G920 wheel into HID mode is
> > > excessive and does not make much sense.
> > > 
> > > The module can be extended to handle any other USB device that might
> > > require such a switch.
> > 
> > Can this switching be done in userspace? That's what we normally do, ref
> > usb_modeswitch.
> 
> I briefly considered leaving the switch up to the userspace and handling the 
> device in the kernel only once it's been switched. I am however uncertain how 
> to advertise this to the users. Writing a tiny app that would currently handle 
> just one device seems like an overkill,

Writing a kernel driver that can be done in userspace is also overkill :)

> abusing usb_modeswitch seems counter-
> intuitive as it's purpose is to handle various USB modems. Having a tiny 
> module in the kernel looks like the most straightforward thing to do as far as 
> user experience is concerned.
> 
> I would not object to deferring the switch to userspace as long as there is an 
> easy way how to communicate the need for a small switching tool to the users.

Have your program install a udev rule to make it run automatically when
the device is plugged in, and then just file a bug with the different
distros to have them pick up your program and add it to their releases.
With free services like Github to host the code, and OBS to build
packages for every different distro out there, there has never been an
easier time to get programs to distros.

So please just do this in userspace, we don't like to take kernel
drivers for functions that can be done in userspace.

thanks,

greg k-h

Re: [PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[Oliver Neukum]

On Sat, 2016-01-23 at 08:39 -0800, Greg KH wrote:
> Have your program install a udev rule to make it run automatically
> when
> the device is plugged in, and then just file a bug with the different
> distros to have them pick up your program and add it to their
> releases.
> With free services like Github to host the code, and OBS to build
> packages for every different distro out there, there has never been an
> easier time to get programs to distros.
> 
> So please just do this in userspace, we don't like to take kernel
> drivers for functions that can be done in userspace.

That raises a point. If we wish to do port power switching
as a form of runtime PM, we will need to do mode switching
in kernel space. The notion that this can be done in user space
is likely to become wrong as things develop.

	Regards
		Oliver

Re: [PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[Greg KH]

On Sun, Jan 24, 2016 at 11:05:05PM +0100, Oliver Neukum wrote:
> On Sat, 2016-01-23 at 08:39 -0800, Greg KH wrote:
> > Have your program install a udev rule to make it run automatically
> > when
> > the device is plugged in, and then just file a bug with the different
> > distros to have them pick up your program and add it to their
> > releases.
> > With free services like Github to host the code, and OBS to build
> > packages for every different distro out there, there has never been an
> > easier time to get programs to distros.
> > 
> > So please just do this in userspace, we don't like to take kernel
> > drivers for functions that can be done in userspace.
> 
> That raises a point. If we wish to do port power switching
> as a form of runtime PM, we will need to do mode switching
> in kernel space. The notion that this can be done in user space
> is likely to become wrong as things develop.

Maybe, if a device doesn't remember it's "mode" after sleeping, then it
might get messy, it will be interesting to see how that works out over
time...

Re: [PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[Oliver Neukum]

On Sun, 2016-01-24 at 14:48 -0800, Greg KH wrote:
> On Sun, Jan 24, 2016 at 11:05:05PM +0100, Oliver Neukum wrote:
> > On Sat, 2016-01-23 at 08:39 -0800, Greg KH wrote:
> > > Have your program install a udev rule to make it run automatically
> > > when
> > > the device is plugged in, and then just file a bug with the different
> > > distros to have them pick up your program and add it to their
> > > releases.
> > > With free services like Github to host the code, and OBS to build
> > > packages for every different distro out there, there has never been an
> > > easier time to get programs to distros.
> > > 
> > > So please just do this in userspace, we don't like to take kernel
> > > drivers for functions that can be done in userspace.
> > 
> > That raises a point. If we wish to do port power switching
> > as a form of runtime PM, we will need to do mode switching
> > in kernel space. The notion that this can be done in user space
> > is likely to become wrong as things develop.
> 
> Maybe, if a device doesn't remember it's "mode" after sleeping, then it
> might get messy, it will be interesting to see how that works out over
> time...

We are cutting power. So unless they have a battery or are self powered,
I don't see how they could retain their mode. But it is a mess.

	Regards
		Oliver

Re: [PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[Jiri Kosina]

On Sat, 23 Jan 2016, Michal Malý wrote:

> I briefly considered leaving the switch up to the userspace and handling the 
> device in the kernel only once it's been switched. I am however uncertain how 
> to advertise this to the users. Writing a tiny app that would currently handle 
> just one device seems like an overkill, abusing usb_modeswitch seems counter-
> intuitive as it's purpose is to handle various USB modems. 

Well, originally usb_modeswitch (beware, it's called "modeswitch", not 
"modemswitch" :) ) was purely for handling ZeroCD on USB 3G modems, and 
it's currently still by far the most common case, but there are other 
devices being handled by that package now; I for example recall some 
HP printers needing a magic USB command to be sent to it before they would 
start printing, and usb_modeswitch is handling those now AFAIK.

So if we do this in userspace, I think usb_modeswitch is not a bad choice.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/2] Add a skeleton module to perform a basic initialization on certain USB devices

[Michal Malý]

On Mon, 2016-01-25 at 15:17 +0100, Jiri Kosina wrote:
> On Sat, 23 Jan 2016, Michal Malý wrote:
> 
> > I briefly considered leaving the switch up to the userspace and
> handling the 
> > device in the kernel only once it's been switched. I am however
> uncertain how 
> > to advertise this to the users. Writing a tiny app that would
> currently handle 
> > just one device seems like an overkill, abusing usb_modeswitch
> seems counter-
> > intuitive as it's purpose is to handle various USB modems. 
> 
> Well, originally usb_modeswitch (beware, it's called "modeswitch",
> not 
> "modemswitch" :) ) was purely for handling ZeroCD on USB 3G modems,
> and 
> it's currently still by far the most common case, but there are
> other 
> devices being handled by that package now; I for example recall some 
> HP printers needing a magic USB command to be sent to it before they
> would 
> start printing, and usb_modeswitch is handling those now AFAIK.
> 
> So if we do this in userspace, I think usb_modeswitch is not a bad
> choice.

Ok, that's interesting... I tinkered with usb_modeswitch a bit and I
made it switch the G920 just fine after a while. Consider the patches
dropped then, letting usb_modeswitch take care of this sounds
reasonable.

Michal