[PATCHv2 1/3] HID: hid-lg: Allow for custom device properties to be stored in private driver data

[PATCHv2 1/3] HID: hid-lg: Allow for custom device properties to be stored in private driver data

[Michal Malý]

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

Hello,

I revised and broke the patch out, the first part introduces "struct lg_drv_data" which
can store custom device properties. The second part removes the sysfs interface before
deallocating memory to prevent a race.

Signed-off-by: Michal Malý <madcatxster@gmail.com>

>From 2c58258689730de5292995ab577ac80286d2fe84 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20Mal=C3=BD?= <madcatxster@gmail.com>
Date: Wed, 14 Mar 2012 18:00:04 +0100
Subject: [PATCH 1/2] HID: hid-lg: Allow for custom device properties to be
 stored in private driver data

---
 Makefile             |    2 +-
 drivers/hid/hid-lg.c |   55 +++++++++++++++++++++++++++++---------------------
 drivers/hid/hid-lg.h |    7 +++++++
 3 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/Makefile b/Makefile
index 56d4817..97f016a 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 VERSION = 3
 PATCHLEVEL = 3
 SUBLEVEL = 0
-EXTRAVERSION = -rc7
+EXTRAVERSION = -1
 NAME = Saber-toothed Squirrel
 
 # *DOCUMENTATION*
diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index e7a7bd1..fc37ed6 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -109,23 +109,23 @@ static __u8 dfp_rdesc_fixed[] = {
 static __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
 {
-	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
 
-	if ((quirks & LG_RDESC) && *rsize >= 90 && rdesc[83] == 0x26 &&
+	if ((drv_data->quirks & LG_RDESC) && *rsize >= 90 && rdesc[83] == 0x26 &&
 			rdesc[84] == 0x8c && rdesc[85] == 0x02) {
 		hid_info(hdev,
 			 "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 &&
+	if ((drv_data->quirks & LG_RDESC_REL_ABS) && *rsize >= 50 &&
 			rdesc[32] == 0x81 && rdesc[33] == 0x06 &&
 			rdesc[49] == 0x81 && rdesc[50] == 0x06) {
 		hid_info(hdev,
 			 "fixing up rel/abs in Logitech report descriptor\n");
 		rdesc[33] = rdesc[50] = 0x02;
 	}
-	if ((quirks & LG_FF4) && *rsize >= 101 &&
+	if ((drv_data->quirks & LG_FF4) && *rsize >= 101 &&
 			rdesc[41] == 0x95 && rdesc[42] == 0x0B &&
 			rdesc[47] == 0x05 && rdesc[48] == 0x09) {
 		hid_info(hdev, "fixing up Logitech Speed Force Wireless button descriptor\n");
@@ -278,7 +278,7 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		  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);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
 	unsigned int hid = usage->hid;
 
 	if (hdev->product == USB_DEVICE_ID_LOGITECH_RECEIVER &&
@@ -289,7 +289,7 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			lg_dinovo_mapping(hi, usage, bit, max))
 		return 1;
 
-	if ((quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
+	if ((drv_data->quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
 		return 1;
 
 	if ((hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
@@ -299,11 +299,11 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 
 	/* Special handling for Logitech Cordless Desktop */
 	if (field->application == HID_GD_MOUSE) {
-		if ((quirks & LG_IGNORE_DOUBLED_WHEEL) &&
+		if ((drv_data->quirks & LG_IGNORE_DOUBLED_WHEEL) &&
 				(hid == 7 || hid == 8))
 			return -1;
 	} else {
-		if ((quirks & LG_EXPANDED_KEYMAP) &&
+		if ((drv_data->quirks & LG_EXPANDED_KEYMAP) &&
 				hid < ARRAY_SIZE(e_keymap) &&
 				e_keymap[hid] != 0) {
 			hid_map_usage(hi, usage, bit, max, EV_KEY,
@@ -319,13 +319,13 @@ 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);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
 
-	if ((quirks & LG_BAD_RELATIVE_KEYS) && usage->type == EV_KEY &&
+	if ((drv_data->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 ||
+	if ((drv_data->quirks & LG_DUPLICATE_USAGES) && (usage->type == EV_KEY ||
 			 usage->type == EV_REL || usage->type == EV_ABS))
 		clear_bit(usage->code, *bit);
 
@@ -335,9 +335,9 @@ static int lg_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 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);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
 
-	if ((quirks & LG_INVERT_HWHEEL) && usage->code == REL_HWHEEL) {
+	if ((drv_data->quirks & LG_INVERT_HWHEEL) && usage->code == REL_HWHEEL) {
 		input_event(field->hidinput->input, usage->type, usage->code,
 				-value);
 		return 1;
@@ -348,13 +348,20 @@ static int lg_event(struct hid_device *hdev, struct hid_field *field,
 
 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;
+	struct lg_drv_data *drv_data;
 	int ret;
 
-	hid_set_drvdata(hdev, (void *)quirks);
+	drv_data = kzalloc(sizeof(struct lg_drv_data), GFP_KERNEL);
+	if (!drv_data) {
+		hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
+		return -ENOMEM;
+	}
+	drv_data->quirks = id->driver_data;
+	
+	hid_set_drvdata(hdev, (void *)drv_data);
 
-	if (quirks & LG_NOGET)
+	if (drv_data->quirks & LG_NOGET)
 		hdev->quirks |= HID_QUIRK_NOGET;
 
 	ret = hid_parse(hdev);
@@ -363,7 +370,7 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_free;
 	}
 
-	if (quirks & (LG_FF | LG_FF2 | LG_FF3 | LG_FF4))
+	if (drv_data->quirks & (LG_FF | LG_FF2 | LG_FF3 | LG_FF4))
 		connect_mask &= ~HID_CONNECT_FF;
 
 	ret = hid_hw_start(hdev, connect_mask);
@@ -392,27 +399,29 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		}
 	}
 
-	if (quirks & LG_FF)
+	if (drv_data->quirks & LG_FF)
 		lgff_init(hdev);
-	if (quirks & LG_FF2)
+	if (drv_data->quirks & LG_FF2)
 		lg2ff_init(hdev);
-	if (quirks & LG_FF3)
+	if (drv_data->quirks & LG_FF3)
 		lg3ff_init(hdev);
-	if (quirks & LG_FF4)
+	if (drv_data->quirks & LG_FF4)
 		lg4ff_init(hdev);
 
 	return 0;
 err_free:
+	kfree(drv_data);
 	return ret;
 }
 
 static void lg_remove(struct hid_device *hdev)
 {
-	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
-	if(quirks & LG_FF4)
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
+	if (drv_data->quirks & LG_FF4)
 		lg4ff_deinit(hdev);
 
 	hid_hw_stop(hdev);
+	kfree(drv_data);
 }
 
 static const struct hid_device_id lg_devices[] = {
diff --git a/drivers/hid/hid-lg.h b/drivers/hid/hid-lg.h
index 4b09728..500457b 100644
--- a/drivers/hid/hid-lg.h
+++ b/drivers/hid/hid-lg.h
@@ -1,6 +1,13 @@
 #ifndef __HID_LG_H
 #define __HID_LG_H
 
+#include <linux/spinlock.h>
+
+struct lg_drv_data {
+	unsigned long quirks;
+	void *device_props;	/* Device specific properties */
+};
+
 #ifdef CONFIG_LOGITECH_FF
 int lgff_init(struct hid_device *hdev);
 #else
-- 
1.7.9.4


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[PATCHv3 1/3] HID: hid-lg: Allow for custom device properties to be stored in private driver data

[Michal Malý]

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

For some reason the Makefile got modified along with the driver and I didn't
catch it. This is the updated version of the first part of the patch that leaves
the Makefile alone.

Signed-off-by: Michal Malý <madcatxster@gmail.com>

---
 drivers/hid/hid-lg.c |   55 +++++++++++++++++++++++++++++---------------------
 drivers/hid/hid-lg.h |    7 +++++++
 2 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index e7a7bd1..fc37ed6 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -109,23 +109,23 @@ static __u8 dfp_rdesc_fixed[] = {
 static __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
 {
-	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
 
-	if ((quirks & LG_RDESC) && *rsize >= 90 && rdesc[83] == 0x26 &&
+	if ((drv_data->quirks & LG_RDESC) && *rsize >= 90 && rdesc[83] == 0x26 &&
 			rdesc[84] == 0x8c && rdesc[85] == 0x02) {
 		hid_info(hdev,
 			 "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 &&
+	if ((drv_data->quirks & LG_RDESC_REL_ABS) && *rsize >= 50 &&
 			rdesc[32] == 0x81 && rdesc[33] == 0x06 &&
 			rdesc[49] == 0x81 && rdesc[50] == 0x06) {
 		hid_info(hdev,
 			 "fixing up rel/abs in Logitech report descriptor\n");
 		rdesc[33] = rdesc[50] = 0x02;
 	}
-	if ((quirks & LG_FF4) && *rsize >= 101 &&
+	if ((drv_data->quirks & LG_FF4) && *rsize >= 101 &&
 			rdesc[41] == 0x95 && rdesc[42] == 0x0B &&
 			rdesc[47] == 0x05 && rdesc[48] == 0x09) {
 		hid_info(hdev, "fixing up Logitech Speed Force Wireless button descriptor\n");
@@ -278,7 +278,7 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		  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);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
 	unsigned int hid = usage->hid;
 
 	if (hdev->product == USB_DEVICE_ID_LOGITECH_RECEIVER &&
@@ -289,7 +289,7 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			lg_dinovo_mapping(hi, usage, bit, max))
 		return 1;
 
-	if ((quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
+	if ((drv_data->quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
 		return 1;
 
 	if ((hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
@@ -299,11 +299,11 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 
 	/* Special handling for Logitech Cordless Desktop */
 	if (field->application == HID_GD_MOUSE) {
-		if ((quirks & LG_IGNORE_DOUBLED_WHEEL) &&
+		if ((drv_data->quirks & LG_IGNORE_DOUBLED_WHEEL) &&
 				(hid == 7 || hid == 8))
 			return -1;
 	} else {
-		if ((quirks & LG_EXPANDED_KEYMAP) &&
+		if ((drv_data->quirks & LG_EXPANDED_KEYMAP) &&
 				hid < ARRAY_SIZE(e_keymap) &&
 				e_keymap[hid] != 0) {
 			hid_map_usage(hi, usage, bit, max, EV_KEY,
@@ -319,13 +319,13 @@ 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);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
 
-	if ((quirks & LG_BAD_RELATIVE_KEYS) && usage->type == EV_KEY &&
+	if ((drv_data->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 ||
+	if ((drv_data->quirks & LG_DUPLICATE_USAGES) && (usage->type == EV_KEY ||
 			 usage->type == EV_REL || usage->type == EV_ABS))
 		clear_bit(usage->code, *bit);
 
@@ -335,9 +335,9 @@ static int lg_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 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);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
 
-	if ((quirks & LG_INVERT_HWHEEL) && usage->code == REL_HWHEEL) {
+	if ((drv_data->quirks & LG_INVERT_HWHEEL) && usage->code == REL_HWHEEL) {
 		input_event(field->hidinput->input, usage->type, usage->code,
 				-value);
 		return 1;
@@ -348,13 +348,20 @@ static int lg_event(struct hid_device *hdev, struct hid_field *field,
 
 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;
+	struct lg_drv_data *drv_data;
 	int ret;
 
-	hid_set_drvdata(hdev, (void *)quirks);
+	drv_data = kzalloc(sizeof(struct lg_drv_data), GFP_KERNEL);
+	if (!drv_data) {
+		hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
+		return -ENOMEM;
+	}
+	drv_data->quirks = id->driver_data;
+	
+	hid_set_drvdata(hdev, (void *)drv_data);
 
-	if (quirks & LG_NOGET)
+	if (drv_data->quirks & LG_NOGET)
 		hdev->quirks |= HID_QUIRK_NOGET;
 
 	ret = hid_parse(hdev);
@@ -363,7 +370,7 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_free;
 	}
 
-	if (quirks & (LG_FF | LG_FF2 | LG_FF3 | LG_FF4))
+	if (drv_data->quirks & (LG_FF | LG_FF2 | LG_FF3 | LG_FF4))
 		connect_mask &= ~HID_CONNECT_FF;
 
 	ret = hid_hw_start(hdev, connect_mask);
@@ -392,27 +399,29 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		}
 	}
 
-	if (quirks & LG_FF)
+	if (drv_data->quirks & LG_FF)
 		lgff_init(hdev);
-	if (quirks & LG_FF2)
+	if (drv_data->quirks & LG_FF2)
 		lg2ff_init(hdev);
-	if (quirks & LG_FF3)
+	if (drv_data->quirks & LG_FF3)
 		lg3ff_init(hdev);
-	if (quirks & LG_FF4)
+	if (drv_data->quirks & LG_FF4)
 		lg4ff_init(hdev);
 
 	return 0;
 err_free:
+	kfree(drv_data);
 	return ret;
 }
 
 static void lg_remove(struct hid_device *hdev)
 {
-	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
-	if(quirks & LG_FF4)
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
+	if (drv_data->quirks & LG_FF4)
 		lg4ff_deinit(hdev);
 
 	hid_hw_stop(hdev);
+	kfree(drv_data);
 }
 
 static const struct hid_device_id lg_devices[] = {
diff --git a/drivers/hid/hid-lg.h b/drivers/hid/hid-lg.h
index 4b09728..500457b 100644
--- a/drivers/hid/hid-lg.h
+++ b/drivers/hid/hid-lg.h
@@ -1,6 +1,13 @@
 #ifndef __HID_LG_H
 #define __HID_LG_H
 
+#include <linux/spinlock.h>
+
+struct lg_drv_data {
+	unsigned long quirks;
+	void *device_props;	/* Device specific properties */
+};
+
 #ifdef CONFIG_LOGITECH_FF
 int lgff_init(struct hid_device *hdev);
 #else
-- 
1.7.9.4


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[RFC] HID: hid-lg4ff g27 leds using LED subsystem

[Simon Wood]

I'm posting this more as a 'Request for Comments' following on from Michal's patch
this morning.

As you can see there is are two blocks of alternative code (using current list approach
and the new structure approach - which is preferable but Michal is yet to submit that
code).

Once we get some feedback on Michals code I will re-submit this properly.

Simon.

PS. If you want to confirm the LED system works (at least doesn't crash) on another
wheel replace:
  if (lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_G27_WHEEL) {
with (or something):
  if (lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_WHEEL) {


---
 drivers/hid/hid-lg4ff.c |  183 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 182 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 1145292..1f75fd4 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -58,7 +58,8 @@ struct lg4ff_device_entry {
 	__u16 range;
 	__u16 min_range;
 	__u16 max_range;
-	__u8  leds;
+	__u8  led_state;
+	struct led_classdev *led[5];
 	struct list_head list;
 	void (*set_range)(struct hid_device *hid, u16 range);
 };
@@ -336,6 +337,124 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
 	return count;
 }
 
+static void lg4ff_set_leds(struct hid_device *hid, __u8 leds)
+{
+	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
+	struct hid_report *report = list_entry(report_list->next, struct hid_report, list);
+
+	report->field[0]->value[0] = 0xf8;
+	report->field[0]->value[1] = 0x12;
+	report->field[0]->value[2] = leds;
+	report->field[0]->value[3] = 0x00;
+	report->field[0]->value[4] = 0x00;
+	report->field[0]->value[5] = 0x00;
+	report->field[0]->value[6] = 0x00;
+	usbhid_submit_report(hid, report, USB_DIR_OUT);
+}
+
+static void lg4ff_led_set_brightness(struct led_classdev *led_cdev,
+			enum led_brightness value)
+{
+	struct device *dev;
+	struct hid_device *hid;
+	struct lg4ff_device_entry *uninitialized_var(entry);
+	int i, state = 0;
+
+#if 1	// old list based method
+	struct list_head *h;
+	dev = led_cdev->dev->parent;
+	hid = to_hid_device(dev);
+
+	list_for_each(h, &device_list.list) {
+		entry = list_entry(h, struct lg4ff_device_entry, list);
+		if (strcmp(entry->device_id, (&hid->dev)->kobj.name) == 0)
+			break;
+	}
+	if (h == &device_list.list) {
+		dbg_hid("Device not found!");
+		return;
+	}
+#else	//new method (prefered)
+	struct lg_drv_data* drv_data;
+	dev = led_cdev->dev->parent;
+	hid = container_of(dev, struct hid_device, dev);
+	drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+
+	if (!drv_data) {
+		hid_err(hid, "Device data not found.");
+		return;
+	}
+
+	entry = (struct lg4ff_device_entry *)drv_data->dev_props; 
+#endif
+	if (!entry) {
+		hid_err(hid, "Device properties not found.");
+		return;
+	}
+
+	for (i = 0; i < 5; i++) {
+		if (led_cdev != entry->led[i])
+			continue;
+		state = (entry->led_state >> i) & 1;
+		if (value == LED_OFF && state) {
+			entry->led_state &= ~(1 << i);
+			lg4ff_set_leds(hid, entry->led_state);
+		} else if (value != LED_OFF && !state) {
+			entry->led_state |= 1 << i;
+			lg4ff_set_leds(hid, entry->led_state);
+		}
+		break;
+	}
+}
+
+static enum led_brightness lg4ff_led_get_brightness(struct led_classdev *led_cdev)
+{
+	struct device *dev;
+	struct hid_device *hid;
+	struct lg4ff_device_entry *uninitialized_var(entry);
+	int i, value = 0;
+
+#if 1	// old list based method
+	struct list_head *h;
+	dev = led_cdev->dev->parent;
+	hid = to_hid_device(dev);
+
+	list_for_each(h, &device_list.list) {
+		entry = list_entry(h, struct lg4ff_device_entry, list);
+		if (strcmp(entry->device_id, (&hid->dev)->kobj.name) == 0)
+			break;
+	}
+	if (h == &device_list.list) {
+		dbg_hid("Device not found!");
+		return LED_OFF;
+	}
+#else	//new method (prefered)
+	struct lg_drv_data* drv_data;
+	dev = led_cdev->dev->parent;
+	hid = container_of(dev, struct hid_device, dev);
+	drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+
+	if (!drv_data) {
+		hid_err(hid, "Device data not found.");
+		return LED_OFF;
+	}
+
+	entry = (struct lg4ff_device_entry *)drv_data->dev_props; 
+#endif
+	if (!entry) {
+		hid_err(hid, "Device properties not found.");
+		return LED_OFF;
+	}
+
+	for (i = 0; i < 5; i++)
+		if (led_cdev == entry->led[i]) {
+			value = (entry->led_state >> i) & 1;
+			break;
+		}
+
+	return value ? LED_FULL : LED_OFF;
+}
+
 int lg4ff_init(struct hid_device *hid)
 {
 	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
@@ -347,6 +466,9 @@ int lg4ff_init(struct hid_device *hid)
 	struct usb_device_descriptor *udesc;
 	int error, i, j;
 	__u16 bcdDevice, rev_maj, rev_min;
+	struct led_classdev *led;
+	size_t name_sz;
+	char *name;
 
 	/* Find the report to use */
 	if (list_empty(report_list)) {
@@ -457,8 +579,55 @@ int lg4ff_init(struct hid_device *hid)
 	if (entry->set_range != NULL)
 		entry->set_range(hid, entry->range);
 
+	/* register led subsystem - G27 only */
+	entry->led_state = 0;
+	if (lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_G27_WHEEL) {
+		lg4ff_set_leds(hid, 0);
+
+		name_sz = strlen(dev_name(&hid->dev)) + 8;
+
+		for (i = 0; i < 5; i++) {
+			led = kzalloc(sizeof(struct led_classdev)+name_sz, GFP_KERNEL);
+			if (!led) {
+				hid_err(hid, "can't allocate memory for LED %d\n", i);
+				error = -ENOMEM;
+				goto err;
+			}
+
+			name = (void *)(&led[1]);
+			snprintf(name, name_sz, "%s::RPM%d", dev_name(&hid->dev), i+1);
+			led->name = name;
+			led->brightness = 0;
+			led->max_brightness = 1;
+			led->brightness_get = lg4ff_led_get_brightness;
+			led->brightness_set = lg4ff_led_set_brightness;
+
+			entry->led[i] = led;
+			error = led_classdev_register(&hid->dev, led);
+			if (error) {
+				hid_err(hid, "failed to register LED %d. Aborting.\n", i);
+				goto err;
+			}
+		}
+
+		dbg_hid("sysfs interface created for leds\n");
+	}
+
 	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by Simon Wood <simon@mungewell.org>\n");
 	return 0;
+
+err:
+	/* Deregister LEDs (if any) but let the driver continue */
+	for (i = 0; i < 5; i++) {
+		led = entry->led[i];
+		entry->led[i] = NULL;
+		if (!led)
+			continue;
+		led_classdev_unregister(led);
+		kfree(led);
+	}
+
+	return 0;
 }
 
 int lg4ff_deinit(struct hid_device *hid)
@@ -466,12 +635,24 @@ int lg4ff_deinit(struct hid_device *hid)
 	bool found = 0;
 	struct lg4ff_device_entry *entry;
 	struct list_head *h, *g;
+	int i;
+	struct led_classdev *led;
 	
 	device_remove_file(&hid->dev, &dev_attr_range);
 
 	list_for_each_safe(h, g, &device_list.list) {
 		entry = list_entry(h, struct lg4ff_device_entry, list);
 		if (strcmp(entry->device_id, (&hid->dev)->kobj.name) == 0) {
+			/* Deregister LEDs (if any) */
+			for (i = 0; i < 5; i++) {
+				led = entry->led[i];
+				entry->led[i] = NULL;
+				if (!led)
+					continue;
+				led_classdev_unregister(led);
+				kfree(led);
+			}
+
 			list_del(h);
 			kfree(entry->device_id);
 			kfree(entry);
-- 
1.7.4.1

Re: [RFC] HID: hid-lg4ff g27 leds using LED subsystem

[simon]

> I'm posting this more as a 'Request for Comments' following on from
> Michal's patch this morning.

One thing I note sure about is permissions, the LEDs register as
--
Mar 14 14:57:35 ubuntu kernel: [ 6462.204185] Registered led device:
0003:046D:C294.0009::RPM1
Mar 14 14:57:35 ubuntu kernel: [ 6462.204251] Registered led device:
0003:046D:C294.0009::RPM2
Mar 14 14:57:35 ubuntu kernel: [ 6462.204301] Registered led device:
0003:046D:C294.0009::RPM3
Mar 14 14:57:35 ubuntu kernel: [ 6462.204354] Registered led device:
0003:046D:C294.0009::RPM4
Mar 14 14:57:35 ubuntu kernel: [ 6462.204402] Registered led device:
0003:046D:C294.0009::RPM5
--

But I have to be root to perform
--
root@ubuntu:/sys/devices/pci0000:00/0000:00:11.0/0000:02:00.0/usb2/2-1/2-1:1.0/0003:046D:C294.0009/leds/0003:046D:C294.0009::RPM1#
echo 1 > brightness
--

Is it possible to register so that all/any user can control the LED status?
Simon

Re: [RFC] HID: hid-lg4ff g27 leds using LED subsystem

[Jiri Kosina]

On Wed, 14 Mar 2012, simon@mungewell.org wrote:

> > I'm posting this more as a 'Request for Comments' following on from
> > Michal's patch this morning.
> 
> One thing I note sure about is permissions, the LEDs register as
> --
> Mar 14 14:57:35 ubuntu kernel: [ 6462.204185] Registered led device:
> 0003:046D:C294.0009::RPM1
> Mar 14 14:57:35 ubuntu kernel: [ 6462.204251] Registered led device:
> 0003:046D:C294.0009::RPM2
> Mar 14 14:57:35 ubuntu kernel: [ 6462.204301] Registered led device:
> 0003:046D:C294.0009::RPM3
> Mar 14 14:57:35 ubuntu kernel: [ 6462.204354] Registered led device:
> 0003:046D:C294.0009::RPM4
> Mar 14 14:57:35 ubuntu kernel: [ 6462.204402] Registered led device:
> 0003:046D:C294.0009::RPM5
> --
> 
> But I have to be root to perform
> --
> root@ubuntu:/sys/devices/pci0000:00/0000:00:11.0/0000:02:00.0/usb2/2-1/2-1:1.0/0003:046D:C294.0009/leds/0003:046D:C294.0009::RPM1#
> echo 1 > brightness
> --
> 
> Is it possible to register so that all/any user can control the LED status?

Looking at the code, I don't think that's possible. Adding Richard to CC, 
he definitely might have an opinion on this.

-- 
Jiri Kosina
SUSE Labs

Re: [RFC] HID: hid-lg4ff g27 leds using LED subsystem

[Richard Purdie]

On Mon, 2012-03-19 at 17:33 +0100, Jiri Kosina wrote:
> On Wed, 14 Mar 2012, simon@mungewell.org wrote:
> 
> > > I'm posting this more as a 'Request for Comments' following on from
> > > Michal's patch this morning.
> > 
> > One thing I note sure about is permissions, the LEDs register as
> > --
> > Mar 14 14:57:35 ubuntu kernel: [ 6462.204185] Registered led device:
> > 0003:046D:C294.0009::RPM1
> > Mar 14 14:57:35 ubuntu kernel: [ 6462.204251] Registered led device:
> > 0003:046D:C294.0009::RPM2
> > Mar 14 14:57:35 ubuntu kernel: [ 6462.204301] Registered led device:
> > 0003:046D:C294.0009::RPM3
> > Mar 14 14:57:35 ubuntu kernel: [ 6462.204354] Registered led device:
> > 0003:046D:C294.0009::RPM4
> > Mar 14 14:57:35 ubuntu kernel: [ 6462.204402] Registered led device:
> > 0003:046D:C294.0009::RPM5
> > --
> > 
> > But I have to be root to perform
> > --
> > root@ubuntu:/sys/devices/pci0000:00/0000:00:11.0/0000:02:00.0/usb2/2-1/2-1:1.0/0003:046D:C294.0009/leds/0003:046D:C294.0009::RPM1#
> > echo 1 > brightness
> > --
> > 
> > Is it possible to register so that all/any user can control the LED status?
> 
> Looking at the code, I don't think that's possible. Adding Richard to CC, 
> he definitely might have an opinion on this.

Whilst there is kernel side API you could potentially (ab)use, its not
encouraged or easy to do with the LED class attributes. Its really a
userspace policy issue. You can change the permissions on the files in
sysfs to reflect the access you require from something like udev. 

Cheers,

Richard

Re: [PATCHv3 1/3] HID: hid-lg: Allow for custom device properties to be stored in private driver data

[Jiri Kosina]

On Wed, 14 Mar 2012, Michal Malý wrote:

> For some reason the Makefile got modified along with the driver and I didn't
> catch it. This is the updated version of the first part of the patch that leaves
> the Makefile alone.

Please resend the patch with proper changelog, I can't apply it this way 
:) Thanks.

> 
> Signed-off-by: Michal Malý <madcatxster@gmail.com>
> 
> ---
>  drivers/hid/hid-lg.c |   55 +++++++++++++++++++++++++++++---------------------
>  drivers/hid/hid-lg.h |    7 +++++++
>  2 files changed, 39 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
> index e7a7bd1..fc37ed6 100644
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -109,23 +109,23 @@ static __u8 dfp_rdesc_fixed[] = {
>  static __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		unsigned int *rsize)
>  {
> -	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> +	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
>  
> -	if ((quirks & LG_RDESC) && *rsize >= 90 && rdesc[83] == 0x26 &&
> +	if ((drv_data->quirks & LG_RDESC) && *rsize >= 90 && rdesc[83] == 0x26 &&
>  			rdesc[84] == 0x8c && rdesc[85] == 0x02) {
>  		hid_info(hdev,
>  			 "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 &&
> +	if ((drv_data->quirks & LG_RDESC_REL_ABS) && *rsize >= 50 &&
>  			rdesc[32] == 0x81 && rdesc[33] == 0x06 &&
>  			rdesc[49] == 0x81 && rdesc[50] == 0x06) {
>  		hid_info(hdev,
>  			 "fixing up rel/abs in Logitech report descriptor\n");
>  		rdesc[33] = rdesc[50] = 0x02;
>  	}
> -	if ((quirks & LG_FF4) && *rsize >= 101 &&
> +	if ((drv_data->quirks & LG_FF4) && *rsize >= 101 &&
>  			rdesc[41] == 0x95 && rdesc[42] == 0x0B &&
>  			rdesc[47] == 0x05 && rdesc[48] == 0x09) {
>  		hid_info(hdev, "fixing up Logitech Speed Force Wireless button descriptor\n");
> @@ -278,7 +278,7 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		  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);
> +	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
>  	unsigned int hid = usage->hid;
>  
>  	if (hdev->product == USB_DEVICE_ID_LOGITECH_RECEIVER &&
> @@ -289,7 +289,7 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  			lg_dinovo_mapping(hi, usage, bit, max))
>  		return 1;
>  
> -	if ((quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
> +	if ((drv_data->quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
>  		return 1;
>  
>  	if ((hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
> @@ -299,11 +299,11 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  
>  	/* Special handling for Logitech Cordless Desktop */
>  	if (field->application == HID_GD_MOUSE) {
> -		if ((quirks & LG_IGNORE_DOUBLED_WHEEL) &&
> +		if ((drv_data->quirks & LG_IGNORE_DOUBLED_WHEEL) &&
>  				(hid == 7 || hid == 8))
>  			return -1;
>  	} else {
> -		if ((quirks & LG_EXPANDED_KEYMAP) &&
> +		if ((drv_data->quirks & LG_EXPANDED_KEYMAP) &&
>  				hid < ARRAY_SIZE(e_keymap) &&
>  				e_keymap[hid] != 0) {
>  			hid_map_usage(hi, usage, bit, max, EV_KEY,
> @@ -319,13 +319,13 @@ 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);
> +	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
>  
> -	if ((quirks & LG_BAD_RELATIVE_KEYS) && usage->type == EV_KEY &&
> +	if ((drv_data->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 ||
> +	if ((drv_data->quirks & LG_DUPLICATE_USAGES) && (usage->type == EV_KEY ||
>  			 usage->type == EV_REL || usage->type == EV_ABS))
>  		clear_bit(usage->code, *bit);
>  
> @@ -335,9 +335,9 @@ static int lg_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>  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);
> +	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
>  
> -	if ((quirks & LG_INVERT_HWHEEL) && usage->code == REL_HWHEEL) {
> +	if ((drv_data->quirks & LG_INVERT_HWHEEL) && usage->code == REL_HWHEEL) {
>  		input_event(field->hidinput->input, usage->type, usage->code,
>  				-value);
>  		return 1;
> @@ -348,13 +348,20 @@ static int lg_event(struct hid_device *hdev, struct hid_field *field,
>  
>  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;
> +	struct lg_drv_data *drv_data;
>  	int ret;
>  
> -	hid_set_drvdata(hdev, (void *)quirks);
> +	drv_data = kzalloc(sizeof(struct lg_drv_data), GFP_KERNEL);
> +	if (!drv_data) {
> +		hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
> +		return -ENOMEM;
> +	}
> +	drv_data->quirks = id->driver_data;
> +	
> +	hid_set_drvdata(hdev, (void *)drv_data);
>  
> -	if (quirks & LG_NOGET)
> +	if (drv_data->quirks & LG_NOGET)
>  		hdev->quirks |= HID_QUIRK_NOGET;
>  
>  	ret = hid_parse(hdev);
> @@ -363,7 +370,7 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		goto err_free;
>  	}
>  
> -	if (quirks & (LG_FF | LG_FF2 | LG_FF3 | LG_FF4))
> +	if (drv_data->quirks & (LG_FF | LG_FF2 | LG_FF3 | LG_FF4))
>  		connect_mask &= ~HID_CONNECT_FF;
>  
>  	ret = hid_hw_start(hdev, connect_mask);
> @@ -392,27 +399,29 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		}
>  	}
>  
> -	if (quirks & LG_FF)
> +	if (drv_data->quirks & LG_FF)
>  		lgff_init(hdev);
> -	if (quirks & LG_FF2)
> +	if (drv_data->quirks & LG_FF2)
>  		lg2ff_init(hdev);
> -	if (quirks & LG_FF3)
> +	if (drv_data->quirks & LG_FF3)
>  		lg3ff_init(hdev);
> -	if (quirks & LG_FF4)
> +	if (drv_data->quirks & LG_FF4)
>  		lg4ff_init(hdev);
>  
>  	return 0;
>  err_free:
> +	kfree(drv_data);
>  	return ret;
>  }
>  
>  static void lg_remove(struct hid_device *hdev)
>  {
> -	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> -	if(quirks & LG_FF4)
> +	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);
> +	if (drv_data->quirks & LG_FF4)
>  		lg4ff_deinit(hdev);
>  
>  	hid_hw_stop(hdev);
> +	kfree(drv_data);
>  }
>  
>  static const struct hid_device_id lg_devices[] = {
> diff --git a/drivers/hid/hid-lg.h b/drivers/hid/hid-lg.h
> index 4b09728..500457b 100644
> --- a/drivers/hid/hid-lg.h
> +++ b/drivers/hid/hid-lg.h
> @@ -1,6 +1,13 @@
>  #ifndef __HID_LG_H
>  #define __HID_LG_H
>  
> +#include <linux/spinlock.h>
> +
> +struct lg_drv_data {
> +	unsigned long quirks;
> +	void *device_props;	/* Device specific properties */
> +};
> +
>  #ifdef CONFIG_LOGITECH_FF
>  int lgff_init(struct hid_device *hdev);
>  #else
> -- 
> 1.7.9.4
> 
> 

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html