Help: Writing to USB feature port from within kernel driver

Help: Writing to USB feature port from within kernel driver

[simon]

[-- Attachment #1: Type: text/plain, Size: 2972 bytes --]

Hi,
I'm attempting to write to the feature port of a USB HID device during
config, however this doesn't seem to be functioning the same as the python
test code I have.

Code is:
--
        if (quirks & LG_WIIWHEEL) {
                unsigned char buf[] = { 0xAF,  0x01, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00 };
                int limit = 3;

                do {
                        printk("Logitech 0xAF\n");
                        ret = hdev->hid_output_raw_report(hdev, buf,
sizeof(buf), HID_FEATURE_REPORT);
                } while (ret < 0 && limit-- > 0);

                limit = 3;
                buf[0] = 0xB2;

                /* Select random Address */
                get_random_bytes(&buf[1], 1);
                get_random_bytes(&buf[2], 1);

                do {
                        printk("Logitech 0xB2\n");
                        ret = hdev->hid_output_raw_report(hdev, buf,
sizeof(buf), HID_FEATURE_REPORT);
                } while (ret < 0 && limit-- > 0);
        }
--

USBMon shows the difference as follows (python code is polling after write).
--
Kernel Driver
--
ef172800 122703999 S Ci:5:007:0 s 81 06 2200 0000 0065 101 <
ef172800 122705940 C Ci:5:007:0 0 101 = 05010904 a101a102 9501750a
150026ff 03350046 ff030930 81020600 ff950275
ef172500 122706972 S Ci:5:007:0 s a1 01 0100 0000 0005 8 <
ef172500 122707938 C Ci:5:007:0 0 5 = 020a00ff ff
ef172500 122707974 S Ci:5:007:0 s a1 01 0300 0000 0008 8 <
ef172500 122708939 C Ci:5:007:0 0 8 = 00000000 00000000
ef190800 122709548 S Io:5:007:2 -115:2 8 = af010000 00000000
ef190800 122712942 C Io:5:007:2 0:2 8 >
ef172580 122713035 S Io:5:007:2 -115:2 8 = b2174800 00000000
ef172580 122716940 C Io:5:007:2 0:2 8 >

Python code
--
ef190380 194682511 S Ci:5:007:0 s 81 06 2100 0000 0009 9 <
ef190380 194684370 C Ci:5:007:0 0 9 = 09211101 00012265 00
ef190380 194684463 S Ci:5:007:0 s 81 06 2200 0000 0065 101 <
ef190380 194686367 C Ci:5:007:0 0 101 = 05010904 a101a102 9501750a
150026ff 03350046 ff030930 81020600 ff950275

ef190380 194686547 S Co:5:007:0 s 21 09 0300 0000 0008 8 = af010000 00000000
ef190380 194687366 C Co:5:007:0 0 8 >
ef190380 194687443 S Ci:5:007:0 s a1 01 0300 0000 0008 8 <
ef190380 194688365 C Ci:5:007:0 0 8 = af010000 00000000
ef190380 194688481 S Ci:5:007:0 s a1 01 0300 0000 0008 8 <
ef190380 194689365 C Ci:5:007:0 0 8 = af010000 00000000
ef190380 194689453 S Ci:5:007:0 s a1 01 0300 0000 0008 8 <
ef190380 194690365 C Ci:5:007:0 0 8 = 2f010000 00000000

ef190380 194690524 S Co:5:007:0 s 21 09 0300 0000 0008 8 = b2e8ba00 00000000
ef190380 194691365 C Co:5:007:0 0 8 >
ef190380 194691437 S Ci:5:007:0 s a1 01 0300 0000 0008 8 <
ef190380 194693365 C Ci:5:007:0 0 8 = b2e8ba00 00000000
ef190380 194693482 S Ci:5:007:0 s a1 01 0300 0000 0008 8 <
ef190380 194701364 C Ci:5:007:0 0 8 = b2e8ba00 00000000
ef190380 194701472 S Ci:5:007:0 s a1 01 0300 0000 0008 8 <
ef190380 194725369 C Ci:5:007:0 0 8 = b2e8ba00 00000000
--


What should I be doing differently?
Simon

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: hid-lg.c --]
[-- Type: text/x-csrc; name="hid-lg.c", Size: 13740 bytes --]

/*
 *  HID driver for some logitech "special" devices
 *
 *  Copyright (c) 1999 Andreas Gal
 *  Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
 *  Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
 *  Copyright (c) 2006-2007 Jiri Kosina
 *  Copyright (c) 2007 Paul Walmsley
 *  Copyright (c) 2008 Jiri Slaby
 */

/*
 * 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/device.h>
#include <linux/hid.h>
#include <linux/module.h>
#include <linux/random.h>

#include "hid-ids.h"
#include "hid-lg.h"

#define LG_RDESC		0x001
#define LG_BAD_RELATIVE_KEYS	0x002
#define LG_DUPLICATE_USAGES	0x004
#define LG_EXPANDED_KEYMAP	0x010
#define LG_IGNORE_DOUBLED_WHEEL	0x020
#define LG_WIRELESS		0x040
#define LG_INVERT_HWHEEL	0x080
#define LG_NOGET		0x100
#define LG_FF			0x200
#define LG_FF2			0x400
#define LG_RDESC_REL_ABS	0x800
#define LG_FF3			0x1000
#define LG_WIIWHEEL		0x2000

/*
 * Certain Logitech keyboards send in report #3 keys which are far
 * above the logical maximum described in descriptor. This extends
 * the original value of 0x28c of logical maximum to 0x104d
 */
static void lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
		unsigned int rsize)
{
	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

	if ((quirks & LG_RDESC) && rsize >= 90 && rdesc[83] == 0x26 &&
			rdesc[84] == 0x8c && rdesc[85] == 0x02) {
		dev_info(&hdev->dev, "fixing up Logitech keyboard report "
				"descriptor\n");
		rdesc[84] = rdesc[89] = 0x4d;
		rdesc[85] = rdesc[90] = 0x10;
	}
	if ((quirks & LG_RDESC_REL_ABS) && rsize >= 50 &&
			rdesc[32] == 0x81 && rdesc[33] == 0x06 &&
			rdesc[49] == 0x81 && rdesc[50] == 0x06) {
		dev_info(&hdev->dev, "fixing up rel/abs in Logitech "
				"report descriptor\n");
		rdesc[33] = rdesc[50] = 0x02;
	}

        if ((quirks & LG_WIIWHEEL) && rsize >= 101 && 
                        rdesc[41] == 0x95 && rdesc[42] == 0x0B &&
                        rdesc[47] == 0x05 && rdesc[48] == 0x09) {
                dev_info(&hdev->dev, "fixing up Logitech WiiWheel button "
                                "descriptor\n");
                rdesc[41] = 0x05;
                rdesc[42] = 0x09;
                rdesc[47] = 0x95;
                rdesc[48] = 0x0B;
        }

}

#define lg_map_key_clear(c)	hid_map_usage_clear(hi, usage, bit, max, \
		EV_KEY, (c))

static int lg_ultrax_remote_mapping(struct hid_input *hi,
		struct hid_usage *usage, unsigned long **bit, int *max)
{
	if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
		return 0;

	set_bit(EV_REP, hi->input->evbit);
	switch (usage->hid & HID_USAGE) {
	/* Reported on Logitech Ultra X Media Remote */
	case 0x004: lg_map_key_clear(KEY_AGAIN);	break;
	case 0x00d: lg_map_key_clear(KEY_HOME);		break;
	case 0x024: lg_map_key_clear(KEY_SHUFFLE);	break;
	case 0x025: lg_map_key_clear(KEY_TV);		break;
	case 0x026: lg_map_key_clear(KEY_MENU);		break;
	case 0x031: lg_map_key_clear(KEY_AUDIO);	break;
	case 0x032: lg_map_key_clear(KEY_TEXT);		break;
	case 0x033: lg_map_key_clear(KEY_LAST);		break;
	case 0x047: lg_map_key_clear(KEY_MP3);		break;
	case 0x048: lg_map_key_clear(KEY_DVD);		break;
	case 0x049: lg_map_key_clear(KEY_MEDIA);	break;
	case 0x04a: lg_map_key_clear(KEY_VIDEO);	break;
	case 0x04b: lg_map_key_clear(KEY_ANGLE);	break;
	case 0x04c: lg_map_key_clear(KEY_LANGUAGE);	break;
	case 0x04d: lg_map_key_clear(KEY_SUBTITLE);	break;
	case 0x051: lg_map_key_clear(KEY_RED);		break;
	case 0x052: lg_map_key_clear(KEY_CLOSE);	break;

	default:
		return 0;
	}
	return 1;
}

static int lg_dinovo_mapping(struct hid_input *hi, struct hid_usage *usage,
		unsigned long **bit, int *max)
{
	if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
		return 0;

	switch (usage->hid & HID_USAGE) {

	case 0x00d: lg_map_key_clear(KEY_MEDIA);	break;
	default:
		return 0;

	}
	return 1;
}

static int lg_wireless_mapping(struct hid_input *hi, struct hid_usage *usage,
		unsigned long **bit, int *max)
{
	if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
		return 0;

	switch (usage->hid & HID_USAGE) {
	case 0x1001: lg_map_key_clear(KEY_MESSENGER);		break;
	case 0x1003: lg_map_key_clear(KEY_SOUND);		break;
	case 0x1004: lg_map_key_clear(KEY_VIDEO);		break;
	case 0x1005: lg_map_key_clear(KEY_AUDIO);		break;
	case 0x100a: lg_map_key_clear(KEY_DOCUMENTS);		break;
	/* The following two entries are Playlist 1 and 2 on the MX3200 */
	case 0x100f: lg_map_key_clear(KEY_FN_1);		break;
	case 0x1010: lg_map_key_clear(KEY_FN_2);		break;
	case 0x1011: lg_map_key_clear(KEY_PREVIOUSSONG);	break;
	case 0x1012: lg_map_key_clear(KEY_NEXTSONG);		break;
	case 0x1013: lg_map_key_clear(KEY_CAMERA);		break;
	case 0x1014: lg_map_key_clear(KEY_MESSENGER);		break;
	case 0x1015: lg_map_key_clear(KEY_RECORD);		break;
	case 0x1016: lg_map_key_clear(KEY_PLAYER);		break;
	case 0x1017: lg_map_key_clear(KEY_EJECTCD);		break;
	case 0x1018: lg_map_key_clear(KEY_MEDIA);		break;
	case 0x1019: lg_map_key_clear(KEY_PROG1);		break;
	case 0x101a: lg_map_key_clear(KEY_PROG2);		break;
	case 0x101b: lg_map_key_clear(KEY_PROG3);		break;
	case 0x101c: lg_map_key_clear(KEY_CYCLEWINDOWS);	break;
	case 0x101f: lg_map_key_clear(KEY_ZOOMIN);		break;
	case 0x1020: lg_map_key_clear(KEY_ZOOMOUT);		break;
	case 0x1021: lg_map_key_clear(KEY_ZOOMRESET);		break;
	case 0x1023: lg_map_key_clear(KEY_CLOSE);		break;
	case 0x1027: lg_map_key_clear(KEY_MENU);		break;
	/* this one is marked as 'Rotate' */
	case 0x1028: lg_map_key_clear(KEY_ANGLE);		break;
	case 0x1029: lg_map_key_clear(KEY_SHUFFLE);		break;
	case 0x102a: lg_map_key_clear(KEY_BACK);		break;
	case 0x102b: lg_map_key_clear(KEY_CYCLEWINDOWS);	break;
	case 0x102d: lg_map_key_clear(KEY_WWW);			break;
	/* The following two are 'Start/answer call' and 'End/reject call'
	   on the MX3200 */
	case 0x1031: lg_map_key_clear(KEY_OK);			break;
	case 0x1032: lg_map_key_clear(KEY_CANCEL);		break;
	case 0x1041: lg_map_key_clear(KEY_BATTERY);		break;
	case 0x1042: lg_map_key_clear(KEY_WORDPROCESSOR);	break;
	case 0x1043: lg_map_key_clear(KEY_SPREADSHEET);		break;
	case 0x1044: lg_map_key_clear(KEY_PRESENTATION);	break;
	case 0x1045: lg_map_key_clear(KEY_UNDO);		break;
	case 0x1046: lg_map_key_clear(KEY_REDO);		break;
	case 0x1047: lg_map_key_clear(KEY_PRINT);		break;
	case 0x1048: lg_map_key_clear(KEY_SAVE);		break;
	case 0x1049: lg_map_key_clear(KEY_PROG1);		break;
	case 0x104a: lg_map_key_clear(KEY_PROG2);		break;
	case 0x104b: lg_map_key_clear(KEY_PROG3);		break;
	case 0x104c: lg_map_key_clear(KEY_PROG4);		break;

	default:
		return 0;
	}
	return 1;
}

static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
		struct hid_field *field, struct hid_usage *usage,
		unsigned long **bit, int *max)
{
	/* extended mapping for certain Logitech hardware (Logitech cordless
	   desktop LX500) */
	static const u8 e_keymap[] = {
		  0,216,  0,213,175,156,  0,  0,  0,  0,
		144,  0,  0,  0,  0,  0,  0,  0,  0,212,
		174,167,152,161,112,  0,  0,  0,154,  0,
		  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
		  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
		  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
		  0,  0,  0,  0,  0,183,184,185,186,187,
		188,189,190,191,192,193,194,  0,  0,  0
	};
	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
	unsigned int hid = usage->hid;

	if (hdev->product == USB_DEVICE_ID_LOGITECH_RECEIVER &&
			lg_ultrax_remote_mapping(hi, usage, bit, max))
		return 1;

	if (hdev->product == USB_DEVICE_ID_DINOVO_MINI &&
			lg_dinovo_mapping(hi, usage, bit, max))
		return 1;

	if ((quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
		return 1;

	if ((hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
		return 0;

	hid &= HID_USAGE;

	/* Special handling for Logitech Cordless Desktop */
	if (field->application == HID_GD_MOUSE) {
		if ((quirks & LG_IGNORE_DOUBLED_WHEEL) &&
				(hid == 7 || hid == 8))
			return -1;
	} else {
		if ((quirks & LG_EXPANDED_KEYMAP) &&
				hid < ARRAY_SIZE(e_keymap) &&
				e_keymap[hid] != 0) {
			hid_map_usage(hi, usage, bit, max, EV_KEY,
					e_keymap[hid]);
			return 1;
		}
	}

	return 0;
}

static int lg_input_mapped(struct hid_device *hdev, struct hid_input *hi,
		struct hid_field *field, struct hid_usage *usage,
		unsigned long **bit, int *max)
{
	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

	if ((quirks & LG_BAD_RELATIVE_KEYS) && usage->type == EV_KEY &&
			(field->flags & HID_MAIN_ITEM_RELATIVE))
		field->flags &= ~HID_MAIN_ITEM_RELATIVE;

	if ((quirks & LG_DUPLICATE_USAGES) && (usage->type == EV_KEY ||
			 usage->type == EV_REL || usage->type == EV_ABS))
		clear_bit(usage->code, *bit);

	return 0;
}

static int lg_event(struct hid_device *hdev, struct hid_field *field,
		struct hid_usage *usage, __s32 value)
{
	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

	if ((quirks & LG_INVERT_HWHEEL) && usage->code == REL_HWHEEL) {
		input_event(field->hidinput->input, usage->type, usage->code,
				-value);
		return 1;
	}

	return 0;
}

static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
	unsigned long quirks = id->driver_data;
	unsigned int connect_mask = HID_CONNECT_DEFAULT;
	int ret;

	hid_set_drvdata(hdev, (void *)quirks);

	if (quirks & LG_NOGET)
		hdev->quirks |= HID_QUIRK_NOGET;

	ret = hid_parse(hdev);
	if (ret) {
		dev_err(&hdev->dev, "parse failed\n");
		goto err_free;
	}

	if (quirks & (LG_FF | LG_FF2 | LG_FF3))
		connect_mask &= ~HID_CONNECT_FF;

	ret = hid_hw_start(hdev, connect_mask);
	if (ret) {
		dev_err(&hdev->dev, "hw start failed\n");
		goto err_free;
	}

	if (quirks & LG_WIIWHEEL) {
		unsigned char buf[] = { 0xAF,  0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
		int limit = 3;

		do {
			printk("Logitech 0xAF\n");
			ret = hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
		} while (ret < 0 && limit-- > 0);

		limit = 3;
		buf[0] = 0xB2;

		/* Select random Address */
		get_random_bytes(&buf[1], 1);
		get_random_bytes(&buf[2], 1);

		do {
			printk("Logitech 0xB2\n");
			ret = hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
		} while (ret < 0 && limit-- > 0);
	}

	if (quirks & LG_FF)
		lgff_init(hdev);
	if (quirks & LG_FF2)
		lg2ff_init(hdev);
	if (quirks & LG_FF3)
		lg3ff_init(hdev);

	if (quirks & LG_WIIWHEEL) {
	}

	return 0;
err_free:
	return ret;
}

static const struct hid_device_id lg_devices[] = {
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER),
		.driver_data = LG_RDESC | LG_WIRELESS },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER),
		.driver_data = LG_RDESC | LG_WIRELESS },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2),
		.driver_data = LG_RDESC | LG_WIRELESS },

	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER),
		.driver_data = LG_BAD_RELATIVE_KEYS },

	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP),
		.driver_data = LG_DUPLICATE_USAGES },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE),
		.driver_data = LG_DUPLICATE_USAGES },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI),
		.driver_data = LG_DUPLICATE_USAGES },

	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_ELITE_KBD),
		.driver_data = LG_IGNORE_DOUBLED_WHEEL | LG_EXPANDED_KEYMAP },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_CORDLESS_DESKTOP_LX500),
		.driver_data = LG_IGNORE_DOUBLED_WHEEL | LG_EXPANDED_KEYMAP },

	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_EXTREME_3D),
		.driver_data = LG_NOGET },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WHEEL),
		.driver_data = LG_NOGET | LG_FF },

	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD),
		.driver_data = LG_FF },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2),
		.driver_data = LG_FF },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WINGMAN_F3D),
		.driver_data = LG_FF },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_FORCE3D_PRO),
		.driver_data = LG_FF },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOMO_WHEEL),
		.driver_data = LG_FF },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2),
		.driver_data = LG_FF },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G25_WHEEL),
		.driver_data = LG_FF },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WII_WHEEL),
		.driver_data = LG_FF | LG_WIIWHEEL },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WINGMAN_FFG ),
		.driver_data = LG_FF },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2),
		.driver_data = LG_FF2 },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_FLIGHT_SYSTEM_G940),
		.driver_data = LG_FF3 },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACENAVIGATOR),
		.driver_data = LG_RDESC_REL_ABS },
	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACETRAVELLER),
		.driver_data = LG_RDESC_REL_ABS },
	{ }
};

MODULE_DEVICE_TABLE(hid, lg_devices);

static struct hid_driver lg_driver = {
	.name = "logitech",
	.id_table = lg_devices,
	.report_fixup = lg_report_fixup,
	.input_mapping = lg_input_mapping,
	.input_mapped = lg_input_mapped,
	.event = lg_event,
	.probe = lg_probe,
};

static int __init lg_init(void)
{
	return hid_register_driver(&lg_driver);
}

static void __exit lg_exit(void)
{
	hid_unregister_driver(&lg_driver);
}

module_init(lg_init);
module_exit(lg_exit);
MODULE_LICENSE("GPL");

[PATCH 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Ott]

Feature reports should only be sent on the control endpoint.

Reported-by: simon@mungewell.org
Signed-off-by: Alan Ott <alan@signal11.us>
---
 drivers/hid/usbhid/hid-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index b729c02..b0ccc42 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -807,7 +807,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
 	struct usb_host_interface *interface = intf->cur_altsetting;
 	int ret;
 
-	if (usbhid->urbout) {
+	if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
 		int actual_length;
 		int skipped_report_id = 0;
 		if (buf[0] == 0x0) {
-- 
1.7.0.4

[PATCH 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Ott]

Feature reports should only be sent on the control endpoint.

Reported-by: simon@mungewell.org
Signed-off-by: Alan Ott <alan@signal11.us>
---
 drivers/hid/usbhid/hid-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index b729c02..b0ccc42 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -807,7 +807,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
 	struct usb_host_interface *interface = intf->cur_altsetting;
 	int ret;
 
-	if (usbhid->urbout) {
+	if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
 		int actual_length;
 		int skipped_report_id = 0;
 		if (buf[0] == 0x0) {
-- 
1.7.0.4

Re: [PATCH 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Stern]

On Wed, 15 Sep 2010, Alan Ott wrote:

> Feature reports should only be sent on the control endpoint.

Where is this requirement?  Section 5.6 of the HID spec says: 

	Note  Only Input reports are sent via the Interrupt In pipe. 
	Feature and Output reports must be initiated by the host via
	the Control pipe or an optional Interrupt Out pipe.

So if there is an Interrupt-OUT endpoint, it should be valid to use it 
for a Feature report.

Alan Stern

Re: [PATCH 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Stern]

On Wed, 15 Sep 2010, Alan Ott wrote:

> Feature reports should only be sent on the control endpoint.

Where is this requirement?  Section 5.6 of the HID spec says: 

	Note  Only Input reports are sent via the Interrupt In pipe. 
	Feature and Output reports must be initiated by the host via
	the Control pipe or an optional Interrupt Out pipe.

So if there is an Interrupt-OUT endpoint, it should be valid to use it 
for a Feature report.

Alan Stern

Re: [PATCH 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Ott]

On 09/15/2010 12:10 PM, Alan Stern wrote:
> On Wed, 15 Sep 2010, Alan Ott wrote:
>
>    
>> Feature reports should only be sent on the control endpoint.
>>      
> Where is this requirement?  Section 5.6 of the HID spec says:
>
> 	Note  Only Input reports are sent via the Interrupt In pipe.
> 	Feature and Output reports must be initiated by the host via
> 	the Control pipe or an optional Interrupt Out pipe.
>
> So if there is an Interrupt-OUT endpoint, it should be valid to use it
> for a Feature report.
>
> Alan Stern
>    

That is true, the standard does say that. Thanks for checking my work.

I expected to go into the HID standard and find something that would 
back me up. All I ended up finding was contradictions. I know I had seen 
it somewhere (that Feature reports must use the Control endpoint), and 
eventually I remembered that I had read it in Jan Axelson's USB 
Complete, 3rd Edition[1]. I sent her an email asking what she based it 
on, and I cited parts of the HID standard which I thought to be either 
unspecific, or even seemed to indicate the opposite of what she asserts 
in her book (one good example of which is the one you cited). I have not 
heard back from her yet (but she does indeed answer her email, so I 
expect something in a day or two).

In addition to the section you cited, there's also section 6.2.2.5 which 
says, at the very end of the section, on page 32:

    Output type reports can optionally be sent via an Interrupt Out pipe.
    While similar in function, Output and Feature items differ in the
    following
    ways:
         [snip]
       Like Output items, Feature items make up Feature Reports
    accessible via the Control
       pipe with the Get_Report (Feature) and Set_Report (Feature) requests.

That section seems to say both ways, depending on how you read "Output 
type" (ie: does it mean OUTPUT reports or does it mean "reports which go 
out from the host"). Note that particular section is the only place 
where the wording "input type" or "output type" is used, indicating it 
may mean "reports which go out from the host."

The second part of that quote says feature reports are accessible 
through the control pipe. (It doesn't say that they _aren't_ accessible 
any other way). It's curious to me why it would say it in that way, 
without saying "only accessible" or "also accessible."

In all, although somewhat unclear, the HID document does seem to suggest 
that the Interrupt OUT pipe can handle Feature Reports. However, there 
are several things which make me think otherwise.

1. The Windows implementation will refuse to send feature reports 
through WriteFile() (which is the function used to send reports out the 
interrupt OUT pipe if it exists). The HidD_SetFeature() function will 
ONLY send feature reports out the Control pipe, regardless of the 
presence of an Interrupt OUT endpoint.

2. The Macintosh HID implementation does the same thing as the Windows 
version. On the Mac, there's IOHIDDeviceSetReport() which allows you to 
specify a report type of Feature or Output. Output reports go to the 
Interrupt OUT pipe if it exists; feature reports do not. Feature reports 
only go out the Control endpoint.

3. Jan Axelson's book. While it's not an official standard, it's widely 
accepted as a good general reference on USB. Like I said, I have an 
email in to the author asking her to cite her source on Feature Reports. 
That said, the book is very Windows-centric on the host side, and she 
may be basing her assertion on #1 above.

4. Simon (Mungewell)'s email which started this whole thing. First, he 
indicates that his device doesn't handle Feature reports in the 
Interrupt OUT pipe. Second, he asserts that libhid works the same way 
Windows and Mac do, sending Feature reports out the Control pipe[2]. I 
have a PS3 controller that I borrowed which also seems to work the same 
way (ie: Feature reports don't work if they go out the Interrupt OUT 
endpoint).

5. The Bluetooth HID specification says in multiple places that "Feature 
Reports must be carried on the Control channel." Yes yes, you don't have 
to say it. Bluetooth is not USB, so the bluetooth standard shouldn't 
apply. Maybe true.

So there it is. That's everything I know about this particular problem. 
The standard to me is unclear at best; Every single other 
implementation[3] (Windows, Mac, libhid) uses the control endpoint only; 
Jan's book says that only the control endpoint can be used; Simon's 
hardware only works using the control endpoint for feature reports; my 
hardware only works using the control endpoint for Feature reports; the 
Bluetooth spec says Feature reports must use the control endpoint.

PS: I'll throw out one more thing. HID transfers which use the control 
endpoint have their report type identified in the wValue field of their 
header. Transfers which use the Interrupt OUT endpoint do not have their 
type identified in any way. (they only have the report ID, and even 
then, only when numbered reports are used). If a device which did not 
use report IDs had both a single OUTPUT and a single FEATURE report, how 
could the reports be differentiated by the device if it were possible 
for the FEATURE report to go out on the OUT endpoint? I think that 
actually closes the argument in my mind.

I'd be happy to hear alternate theories.

Alan.


[1] Fourth edition is out. I don't have it. Can anyone here confirm or 
deny that the 4th edition has similar language?
[2] I've confirmed this at:
      
http://libhid.alioth.debian.org/doc/hid__exchange_8c-source.html#l00191
[3] I'm not counting my own hidapi library 
(http://www.signal11.us/oss/hidapi), since it was written by me and is 
of course implemented the way I'm arguing for.

Re: [PATCH 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Stern]

On Wed, 15 Sep 2010, Alan Ott wrote:

> On 09/15/2010 12:10 PM, Alan Stern wrote:
> > On Wed, 15 Sep 2010, Alan Ott wrote:
> >
> >    
> >> Feature reports should only be sent on the control endpoint.
> >>      
> > Where is this requirement?  Section 5.6 of the HID spec says:
> >
> > 	Note  Only Input reports are sent via the Interrupt In pipe.
> > 	Feature and Output reports must be initiated by the host via
> > 	the Control pipe or an optional Interrupt Out pipe.
> >
> > So if there is an Interrupt-OUT endpoint, it should be valid to use it
> > for a Feature report.
> >
> > Alan Stern
> >    
> 
> That is true, the standard does say that. Thanks for checking my work.
> 
> I expected to go into the HID standard and find something that would 
> back me up. All I ended up finding was contradictions. I know I had seen 
> it somewhere (that Feature reports must use the Control endpoint), and 
> eventually I remembered that I had read it in Jan Axelson's USB 
> Complete, 3rd Edition[1]. I sent her an email asking what she based it 
> on, and I cited parts of the HID standard which I thought to be either 
> unspecific, or even seemed to indicate the opposite of what she asserts 
> in her book (one good example of which is the one you cited). I have not 
> heard back from her yet (but she does indeed answer her email, so I 
> expect something in a day or two).
> 
> In addition to the section you cited, there's also section 6.2.2.5 which 
> says, at the very end of the section, on page 32:
> 
>     Output type reports can optionally be sent via an Interrupt Out pipe.
>     While similar in function, Output and Feature items differ in the
>     following
>     ways:
>          [snip]
>        Like Output items, Feature items make up Feature Reports
>     accessible via the Control
>        pipe with the Get_Report (Feature) and Set_Report (Feature) requests.
> 
> That section seems to say both ways, depending on how you read "Output 
> type" (ie: does it mean OUTPUT reports or does it mean "reports which go 
> out from the host"). Note that particular section is the only place 
> where the wording "input type" or "output type" is used, indicating it 
> may mean "reports which go out from the host."
> 
> The second part of that quote says feature reports are accessible 
> through the control pipe. (It doesn't say that they _aren't_ accessible 
> any other way). It's curious to me why it would say it in that way, 
> without saying "only accessible" or "also accessible."
> 
> In all, although somewhat unclear, the HID document does seem to suggest 
> that the Interrupt OUT pipe can handle Feature Reports. However, there 
> are several things which make me think otherwise.
> 
> 1. The Windows implementation will refuse to send feature reports 
> through WriteFile() (which is the function used to send reports out the 
> interrupt OUT pipe if it exists). The HidD_SetFeature() function will 
> ONLY send feature reports out the Control pipe, regardless of the 
> presence of an Interrupt OUT endpoint.
> 
> 2. The Macintosh HID implementation does the same thing as the Windows 
> version. On the Mac, there's IOHIDDeviceSetReport() which allows you to 
> specify a report type of Feature or Output. Output reports go to the 
> Interrupt OUT pipe if it exists; feature reports do not. Feature reports 
> only go out the Control endpoint.
> 
> 3. Jan Axelson's book. While it's not an official standard, it's widely 
> accepted as a good general reference on USB. Like I said, I have an 
> email in to the author asking her to cite her source on Feature Reports. 
> That said, the book is very Windows-centric on the host side, and she 
> may be basing her assertion on #1 above.
> 
> 4. Simon (Mungewell)'s email which started this whole thing. First, he 
> indicates that his device doesn't handle Feature reports in the 
> Interrupt OUT pipe. Second, he asserts that libhid works the same way 
> Windows and Mac do, sending Feature reports out the Control pipe[2]. I 
> have a PS3 controller that I borrowed which also seems to work the same 
> way (ie: Feature reports don't work if they go out the Interrupt OUT 
> endpoint).
> 
> 5. The Bluetooth HID specification says in multiple places that "Feature 
> Reports must be carried on the Control channel." Yes yes, you don't have 
> to say it. Bluetooth is not USB, so the bluetooth standard shouldn't 
> apply. Maybe true.
> 
> So there it is. That's everything I know about this particular problem. 
> The standard to me is unclear at best; Every single other 
> implementation[3] (Windows, Mac, libhid) uses the control endpoint only; 
> Jan's book says that only the control endpoint can be used; Simon's 
> hardware only works using the control endpoint for feature reports; my 
> hardware only works using the control endpoint for Feature reports; the 
> Bluetooth spec says Feature reports must use the control endpoint.
> 
> PS: I'll throw out one more thing. HID transfers which use the control 
> endpoint have their report type identified in the wValue field of their 
> header. Transfers which use the Interrupt OUT endpoint do not have their 
> type identified in any way. (they only have the report ID, and even 
> then, only when numbered reports are used). If a device which did not 
> use report IDs had both a single OUTPUT and a single FEATURE report, how 
> could the reports be differentiated by the device if it were possible 
> for the FEATURE report to go out on the OUT endpoint? I think that 
> actually closes the argument in my mind.
> 
> I'd be happy to hear alternate theories.

That's all fine, and I have no objection to the patch itself.  You 
should include (in brief form) some of this explanation in the patch 
description so that people will know _why_ this has to be done.

Alan Stern

Re: [PATCH 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Stern]

On Wed, 15 Sep 2010, Alan Ott wrote:

> On 09/15/2010 12:10 PM, Alan Stern wrote:
> > On Wed, 15 Sep 2010, Alan Ott wrote:
> >
> >    
> >> Feature reports should only be sent on the control endpoint.
> >>      
> > Where is this requirement?  Section 5.6 of the HID spec says:
> >
> > 	Note  Only Input reports are sent via the Interrupt In pipe.
> > 	Feature and Output reports must be initiated by the host via
> > 	the Control pipe or an optional Interrupt Out pipe.
> >
> > So if there is an Interrupt-OUT endpoint, it should be valid to use it
> > for a Feature report.
> >
> > Alan Stern
> >    
> 
> That is true, the standard does say that. Thanks for checking my work.
> 
> I expected to go into the HID standard and find something that would 
> back me up. All I ended up finding was contradictions. I know I had seen 
> it somewhere (that Feature reports must use the Control endpoint), and 
> eventually I remembered that I had read it in Jan Axelson's USB 
> Complete, 3rd Edition[1]. I sent her an email asking what she based it 
> on, and I cited parts of the HID standard which I thought to be either 
> unspecific, or even seemed to indicate the opposite of what she asserts 
> in her book (one good example of which is the one you cited). I have not 
> heard back from her yet (but she does indeed answer her email, so I 
> expect something in a day or two).
> 
> In addition to the section you cited, there's also section 6.2.2.5 which 
> says, at the very end of the section, on page 32:
> 
>     Output type reports can optionally be sent via an Interrupt Out pipe.
>     While similar in function, Output and Feature items differ in the
>     following
>     ways:
>          [snip]
>        Like Output items, Feature items make up Feature Reports
>     accessible via the Control
>        pipe with the Get_Report (Feature) and Set_Report (Feature) requests.
> 
> That section seems to say both ways, depending on how you read "Output 
> type" (ie: does it mean OUTPUT reports or does it mean "reports which go 
> out from the host"). Note that particular section is the only place 
> where the wording "input type" or "output type" is used, indicating it 
> may mean "reports which go out from the host."
> 
> The second part of that quote says feature reports are accessible 
> through the control pipe. (It doesn't say that they _aren't_ accessible 
> any other way). It's curious to me why it would say it in that way, 
> without saying "only accessible" or "also accessible."
> 
> In all, although somewhat unclear, the HID document does seem to suggest 
> that the Interrupt OUT pipe can handle Feature Reports. However, there 
> are several things which make me think otherwise.
> 
> 1. The Windows implementation will refuse to send feature reports 
> through WriteFile() (which is the function used to send reports out the 
> interrupt OUT pipe if it exists). The HidD_SetFeature() function will 
> ONLY send feature reports out the Control pipe, regardless of the 
> presence of an Interrupt OUT endpoint.
> 
> 2. The Macintosh HID implementation does the same thing as the Windows 
> version. On the Mac, there's IOHIDDeviceSetReport() which allows you to 
> specify a report type of Feature or Output. Output reports go to the 
> Interrupt OUT pipe if it exists; feature reports do not. Feature reports 
> only go out the Control endpoint.
> 
> 3. Jan Axelson's book. While it's not an official standard, it's widely 
> accepted as a good general reference on USB. Like I said, I have an 
> email in to the author asking her to cite her source on Feature Reports. 
> That said, the book is very Windows-centric on the host side, and she 
> may be basing her assertion on #1 above.
> 
> 4. Simon (Mungewell)'s email which started this whole thing. First, he 
> indicates that his device doesn't handle Feature reports in the 
> Interrupt OUT pipe. Second, he asserts that libhid works the same way 
> Windows and Mac do, sending Feature reports out the Control pipe[2]. I 
> have a PS3 controller that I borrowed which also seems to work the same 
> way (ie: Feature reports don't work if they go out the Interrupt OUT 
> endpoint).
> 
> 5. The Bluetooth HID specification says in multiple places that "Feature 
> Reports must be carried on the Control channel." Yes yes, you don't have 
> to say it. Bluetooth is not USB, so the bluetooth standard shouldn't 
> apply. Maybe true.
> 
> So there it is. That's everything I know about this particular problem. 
> The standard to me is unclear at best; Every single other 
> implementation[3] (Windows, Mac, libhid) uses the control endpoint only; 
> Jan's book says that only the control endpoint can be used; Simon's 
> hardware only works using the control endpoint for feature reports; my 
> hardware only works using the control endpoint for Feature reports; the 
> Bluetooth spec says Feature reports must use the control endpoint.
> 
> PS: I'll throw out one more thing. HID transfers which use the control 
> endpoint have their report type identified in the wValue field of their 
> header. Transfers which use the Interrupt OUT endpoint do not have their 
> type identified in any way. (they only have the report ID, and even 
> then, only when numbered reports are used). If a device which did not 
> use report IDs had both a single OUTPUT and a single FEATURE report, how 
> could the reports be differentiated by the device if it were possible 
> for the FEATURE report to go out on the OUT endpoint? I think that 
> actually closes the argument in my mind.
> 
> I'd be happy to hear alternate theories.

That's all fine, and I have no objection to the patch itself.  You 
should include (in brief form) some of this explanation in the patch 
description so that people will know _why_ this has to be done.

Alan Stern

Re: [PATCH 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Jiri Kosina]

On Thu, 16 Sep 2010, Alan Stern wrote:

> That's all fine, and I have no objection to the patch itself.  You 
> should include (in brief form) some of this explanation in the patch 
> description so that people will know _why_ this has to be done.

Fully, agreed. Alan, could you please write a short description into the 
patch changelog and resubmit it to me?

Thanks a lot,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

[PATCH v2 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Ott]

Feature reports should only be sent on the control endpoint.

The USB HID standard is unclear and confusing on this issue. It seems to
suggest that Feature reports can be sent on a HID device's Interrupt OUT
endpoint. This cannot be the case because the report type is not
encoded in transfers sent out the Interrput OUT endpoint. If Feature 
reports were sent on the Interrupt OUT endpint, they would be 
indistinguishable from Output reports in the case where Report IDs were 
not used.

Further, Windows and Mac OS X do not send Feature reports out the 
interrupt OUT Endpoint. They will only go out the Control Endpoint.

In addition, many devices simply do not hande Feature reports sent out 
the Interrupt OUT endpoint.

Reported-by: simon@mungewell.org
Signed-off-by: Alan Ott <alan@signal11.us>
---
  drivers/hid/usbhid/hid-core.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index b729c02..b0ccc42 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -807,7 +807,7 @@ static int usbhid_output_raw_report(struct 
hid_device *hid, __u8 *buf, size_t co
  	struct usb_host_interface *interface = intf->cur_altsetting;
  	int ret;
  -	if (usbhid->urbout) {
+	if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
  		int actual_length;
  		int skipped_report_id = 0;
  		if (buf[0] == 0x0) {
-- 
1.7.0.4

[PATCH v2 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Ott]

Feature reports should only be sent on the control endpoint.

The USB HID standard is unclear and confusing on this issue. It seems to
suggest that Feature reports can be sent on a HID device's Interrupt OUT
endpoint. This cannot be the case because the report type is not
encoded in transfers sent out the Interrput OUT endpoint. If Feature 
reports were sent on the Interrupt OUT endpint, they would be 
indistinguishable from Output reports in the case where Report IDs were 
not used.

Further, Windows and Mac OS X do not send Feature reports out the 
interrupt OUT Endpoint. They will only go out the Control Endpoint.

In addition, many devices simply do not hande Feature reports sent out 
the Interrupt OUT endpoint.

Reported-by: simon@mungewell.org
Signed-off-by: Alan Ott <alan@signal11.us>
---
  drivers/hid/usbhid/hid-core.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index b729c02..b0ccc42 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -807,7 +807,7 @@ static int usbhid_output_raw_report(struct 
hid_device *hid, __u8 *buf, size_t co
  	struct usb_host_interface *interface = intf->cur_altsetting;
  	int ret;
  -	if (usbhid->urbout) {
+	if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
  		int actual_length;
  		int skipped_report_id = 0;
  		if (buf[0] == 0x0) {
-- 
1.7.0.4

Re: [PATCH v2 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Sergei Shtylyov]

Hello.

Alan Ott wrote:

> Feature reports should only be sent on the control endpoint.

> The USB HID standard is unclear and confusing on this issue. It seems to
> suggest that Feature reports can be sent on a HID device's Interrupt OUT
> endpoint. This cannot be the case because the report type is not
> encoded in transfers sent out the Interrput OUT endpoint. If Feature 
> reports were sent on the Interrupt OUT endpint, they would be 
> indistinguishable from Output reports in the case where Report IDs were 
> not used.

> Further, Windows and Mac OS X do not send Feature reports out the 
> interrupt OUT Endpoint. They will only go out the Control Endpoint.

> In addition, many devices simply do not hande Feature reports sent out 
> the Interrupt OUT endpoint.

> Reported-by: simon@mungewell.org
> Signed-off-by: Alan Ott <alan@signal11.us>
> ---
>  drivers/hid/usbhid/hid-core.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index b729c02..b0ccc42 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -807,7 +807,7 @@ static int usbhid_output_raw_report(struct 
> hid_device *hid, __u8 *buf, size_t co
>      struct usb_host_interface *interface = intf->cur_altsetting;
>      int ret;

    It's high time to add an empty line here...

>  -    if (usbhid->urbout) {

    The patch is damaged -- something inserted spaces at the start of lines...

WBR, Sergei

[PATCH v3 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Ott]

Feature reports should only be sent on the control endpoint.

The USB HID standard is unclear and confusing on this issue. It seems to
suggest that Feature reports can be sent on a HID device's Interrupt OUT
endpoint.  This cannot be the case because the report type is not encoded in
transfers sent out the Interrput OUT endpoint.  If Feature reports were sent
on the Interrupt OUT endpint, they would be indistinguishable from Output
reports in the case where Report IDs were not used.

Further, Windows and Mac OS X do not send Feature reports out the interrupt
OUT Endpoint.  They will only go out the Control Endpoint.

In addition, many devices simply do not hande Feature reports sent out the Interrupt OUT endpoint.

Reported-by: simon@mungewell.org
Signed-off-by: Alan Ott <alan@signal11.us>
---
 drivers/hid/usbhid/hid-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index b729c02..b0ccc42 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -807,7 +807,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
 	struct usb_host_interface *interface = intf->cur_altsetting;
 	int ret;
 
-	if (usbhid->urbout) {
+	if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
 		int actual_length;
 		int skipped_report_id = 0;
 		if (buf[0] == 0x0) {
-- 
1.7.0.4

[PATCH v3 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Alan Ott]

Feature reports should only be sent on the control endpoint.

The USB HID standard is unclear and confusing on this issue. It seems to
suggest that Feature reports can be sent on a HID device's Interrupt OUT
endpoint.  This cannot be the case because the report type is not encoded in
transfers sent out the Interrput OUT endpoint.  If Feature reports were sent
on the Interrupt OUT endpint, they would be indistinguishable from Output
reports in the case where Report IDs were not used.

Further, Windows and Mac OS X do not send Feature reports out the interrupt
OUT Endpoint.  They will only go out the Control Endpoint.

In addition, many devices simply do not hande Feature reports sent out the Interrupt OUT endpoint.

Reported-by: simon@mungewell.org
Signed-off-by: Alan Ott <alan@signal11.us>
---
 drivers/hid/usbhid/hid-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index b729c02..b0ccc42 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -807,7 +807,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
 	struct usb_host_interface *interface = intf->cur_altsetting;
 	int ret;
 
-	if (usbhid->urbout) {
+	if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
 		int actual_length;
 		int skipped_report_id = 0;
 		if (buf[0] == 0x0) {
-- 
1.7.0.4

[PATCH v2 1/1] trivial: usbhid: Formatting fix

[Alan Ott]

Added blank line after declarations.

Signed-off-by: Alan Ott <alan@signal11.us>
---
 drivers/hid/usbhid/hid-core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index b729c02..ff90069 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -810,6 +810,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
 	if (usbhid->urbout) {
 		int actual_length;
 		int skipped_report_id = 0;
+
 		if (buf[0] == 0x0) {
 			/* Don't send the Report ID */
 			buf++;
-- 
1.7.0.4

[PATCH v2 1/1] trivial: usbhid: Formatting fix

[Alan Ott]

Added blank line after declarations.

Signed-off-by: Alan Ott <alan@signal11.us>
---
 drivers/hid/usbhid/hid-core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index b729c02..ff90069 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -810,6 +810,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
 	if (usbhid->urbout) {
 		int actual_length;
 		int skipped_report_id = 0;
+
 		if (buf[0] == 0x0) {
 			/* Don't send the Report ID */
 			buf++;
-- 
1.7.0.4

Re: [PATCH v3 1/1] Don't Send Feature Reports on Interrupt Endpoint

[Jiri Kosina]

On Tue, 21 Sep 2010, Alan Ott wrote:

> Feature reports should only be sent on the control endpoint.
> 
> The USB HID standard is unclear and confusing on this issue. It seems to
> suggest that Feature reports can be sent on a HID device's Interrupt OUT
> endpoint.  This cannot be the case because the report type is not encoded in
> transfers sent out the Interrput OUT endpoint.  If Feature reports were sent
> on the Interrupt OUT endpint, they would be indistinguishable from Output
> reports in the case where Report IDs were not used.
> 
> Further, Windows and Mac OS X do not send Feature reports out the interrupt
> OUT Endpoint.  They will only go out the Control Endpoint.
> 
> In addition, many devices simply do not hande Feature reports sent out the Interrupt OUT endpoint.
> 
> Reported-by: simon@mungewell.org
> Signed-off-by: Alan Ott <alan@signal11.us>
> ---
>  drivers/hid/usbhid/hid-core.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index b729c02..b0ccc42 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -807,7 +807,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
>  	struct usb_host_interface *interface = intf->cur_altsetting;
>  	int ret;
>  
> -	if (usbhid->urbout) {
> +	if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
>  		int actual_length;
>  		int skipped_report_id = 0;
>  		if (buf[0] == 0x0) {

Applied to the 'logitech' branch (as Simon's new Logitech driver is 
depending on this).

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH v2 1/1] trivial: usbhid: Formatting fix

[Jiri Kosina]

On Tue, 21 Sep 2010, Alan Ott wrote:

> Added blank line after declarations.
> 
> Signed-off-by: Alan Ott <alan@signal11.us>
> ---
>  drivers/hid/usbhid/hid-core.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index b729c02..ff90069 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -810,6 +810,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
>  	if (usbhid->urbout) {
>  		int actual_length;
>  		int skipped_report_id = 0;
> +
>  		if (buf[0] == 0x0) {
>  			/* Don't send the Report ID */
>  			buf++;

Applied, thank you.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.