linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] HID: hid-lg4ff: Add support for G27 LEDs
       [not found] <1649210.pcO5IpzBEF@qosmio-x300>
@ 2012-04-09 16:08 ` Simon Wood
  2012-04-13 12:59   ` Jiri Kosina
  0 siblings, 1 reply; 25+ messages in thread
From: Simon Wood @ 2012-04-09 16:08 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Jiri Kosina, Michael Bauer, Michal Maly, Simon Wood

This patch adds supports for controlling the LED 'tachometer' on
the G27 wheel, via the LED subsystem.

The 5 LEDs are arranged from right (1=grn, 2=grn, 3=yel, 4=yel, 5=red)
and 'mirrored' to the left (10 LEDs in total).

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-lg4ff.c |  149 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 148 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 32c173f..8960347 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -55,7 +55,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);
 };
@@ -335,6 +336,86 @@ 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 = led_cdev->dev->parent;
+	struct hid_device *hid = container_of(dev, struct hid_device, dev);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+	struct lg4ff_device_entry *entry;
+	int i, state = 0;
+
+	if (!drv_data) {
+		hid_err(hid, "Device data not found.");
+		return;
+	}
+
+	entry = (struct lg4ff_device_entry *)drv_data->device_props;
+
+	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 = led_cdev->dev->parent;
+	struct hid_device *hid = container_of(dev, struct hid_device, dev);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+	struct lg4ff_device_entry *entry;
+	int i, value = 0;
+
+	if (!drv_data) {
+		hid_err(hid, "Device data not found.");
+		return LED_OFF;
+	}
+
+	entry = (struct lg4ff_device_entry *)drv_data->device_props;
+
+	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 +428,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)) {
@@ -453,14 +537,66 @@ 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;
+	for (j = 0; j < 5; j++)
+		entry->led[j] = NULL;
+
+	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 (j = 0; j < 5; j++) {
+			led = kzalloc(sizeof(struct led_classdev)+name_sz, GFP_KERNEL);
+			if (!led) {
+				hid_err(hid, "can't allocate memory for LED %d\n", j);
+				error = -ENOMEM;
+				goto err;
+			}
+
+			name = (void *)(&led[1]);
+			snprintf(name, name_sz, "%s::RPM%d", dev_name(&hid->dev), j+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[j] = led;
+			error = led_classdev_register(&hid->dev, led);
+			if (error) {
+				hid_err(hid, "failed to register LED %d. Aborting.\n", j);
+				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 (j = 0; j < 5; j++) {
+		led = entry->led[j];
+		entry->led[j] = NULL;
+		if (!led)
+			continue;
+		led_classdev_unregister(led);
+		kfree(led);
+	}
+
+	return 0;
 }
 
 int lg4ff_deinit(struct hid_device *hid)
 {
 	struct lg4ff_device_entry *entry;
 	struct lg_drv_data *drv_data;
+	int j;
+	struct led_classdev *led;
 
 	device_remove_file(&hid->dev, &dev_attr_range);
 
@@ -474,6 +610,17 @@ int lg4ff_deinit(struct hid_device *hid)
 		hid_err(hid, "Error while deinitializing device, no device properties data.\n");
 		return -1;
 	}
+
+	/* Deregister LEDs (if any) */
+	for (j = 0; j < 5; j++) {
+		led = entry->led[j];
+		entry->led[j] = NULL;
+		if (!led)
+			continue;
+		led_classdev_unregister(led);
+		kfree(led);
+	}
+
 	/* Deallocate memory */
 	kfree(entry);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] HID: hid-lg4ff: Add support for G27 LEDs
  2012-04-09 16:08 ` [PATCH] HID: hid-lg4ff: Add support for G27 LEDs Simon Wood
@ 2012-04-13 12:59   ` Jiri Kosina
  2012-04-18  7:03     ` [PATCH 1/2] " Simon Wood
  0 siblings, 1 reply; 25+ messages in thread
From: Jiri Kosina @ 2012-04-13 12:59 UTC (permalink / raw)
  To: Simon Wood; +Cc: linux-input, linux-kernel, Michael Bauer, Michal Maly

On Mon, 9 Apr 2012, Simon Wood wrote:

> This patch adds supports for controlling the LED 'tachometer' on
> the G27 wheel, via the LED subsystem.
> 
> The 5 LEDs are arranged from right (1=grn, 2=grn, 3=yel, 4=yel, 5=red)
> and 'mirrored' to the left (10 LEDs in total).
> 
> Signed-off-by: Simon Wood <simon@mungewell.org>
> ---
>  drivers/hid/hid-lg4ff.c |  149 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 148 insertions(+), 1 deletions(-)

This patch introduces a clear dependency of hid-lg4ff driver on 
CONFIG_LEDS_CLASS, but the code doesn't reflect it.

So either you have to put #ifdef into source to compile the leds part out 
if leds subsystem is not enabled, or you have to introduce Kconfig rule to 
cover this.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs
  2012-04-13 12:59   ` Jiri Kosina
@ 2012-04-18  7:03     ` Simon Wood
  2012-04-18  7:03       ` [PATCH 2/2] HID: hid-lg4ff: Updated Comments Simon Wood
  2012-04-18 11:24       ` [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs Jiri Kosina
  0 siblings, 2 replies; 25+ messages in thread
From: Simon Wood @ 2012-04-18  7:03 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Jiri Kosina, Michael Bauer, Michal Maly, Simon Wood

This patch adds supports for controlling the LED 'tachometer' on
the G27 wheel, via the LED subsystem.

The 5 LEDs are arranged from right (1=grn, 2=grn, 3=yel, 4=yel, 5=red)
and 'mirrored' to the left (10 LEDs in total).

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-lg4ff.c |  155 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 154 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 32c173f..ab5232e 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -55,7 +55,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);
 };
@@ -335,6 +336,88 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
 	return count;
 }
 
+#ifdef CONFIG_LEDS_CLASS
+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 = led_cdev->dev->parent;
+	struct hid_device *hid = container_of(dev, struct hid_device, dev);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+	struct lg4ff_device_entry *entry;
+	int i, state = 0;
+
+	if (!drv_data) {
+		hid_err(hid, "Device data not found.");
+		return;
+	}
+
+	entry = (struct lg4ff_device_entry *)drv_data->device_props;
+
+	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 = led_cdev->dev->parent;
+	struct hid_device *hid = container_of(dev, struct hid_device, dev);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+	struct lg4ff_device_entry *entry;
+	int i, value = 0;
+
+	if (!drv_data) {
+		hid_err(hid, "Device data not found.");
+		return LED_OFF;
+	}
+
+	entry = (struct lg4ff_device_entry *)drv_data->device_props;
+
+	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;
+}
+#endif
+
 int lg4ff_init(struct hid_device *hid)
 {
 	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
@@ -453,6 +536,57 @@ 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;
+	for (j = 0; j < 5; j++)
+		entry->led[j] = NULL;
+
+#ifdef CONFIG_LEDS_CLASS
+	if (lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_G27_WHEEL) {
+		struct led_classdev *led;
+		size_t name_sz;
+		char *name;
+
+		lg4ff_set_leds(hid, 0);
+
+		name_sz = strlen(dev_name(&hid->dev)) + 8;
+
+		for (j = 0; j < 5; j++) {
+			led = kzalloc(sizeof(struct led_classdev)+name_sz, GFP_KERNEL);
+			if (!led) {
+				hid_err(hid, "can't allocate memory for LED %d\n", j);
+				goto err;
+			}
+
+			name = (void *)(&led[1]);
+			snprintf(name, name_sz, "%s::RPM%d", dev_name(&hid->dev), j+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[j] = led;
+			error = led_classdev_register(&hid->dev, led);
+
+			if (error) {
+				hid_err(hid, "failed to register LED %d. Aborting.\n", j);
+err:
+				/* Deregister LEDs (if any) but let the driver continue */
+				for (j = 0; j < 5; j++) {
+					led = entry->led[j];
+					entry->led[j] = NULL;
+					if (!led)
+						continue;
+					led_classdev_unregister(led);
+				kfree(led);
+				}
+				break;	/* Don't create any more LEDs */
+			}
+		}
+	}
+#endif
+
 	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by Simon Wood <simon@mungewell.org>\n");
 	return 0;
 }
@@ -474,6 +608,25 @@ int lg4ff_deinit(struct hid_device *hid)
 		hid_err(hid, "Error while deinitializing device, no device properties data.\n");
 		return -1;
 	}
+
+#ifdef CONFIG_LEDS_CLASS
+	{
+		int j;
+		struct led_classdev *led;
+
+		/* Deregister LEDs (if any) */
+		for (j = 0; j < 5; j++) {
+
+			led = entry->led[j];
+			entry->led[j] = NULL;
+			if (!led)
+				continue;
+			led_classdev_unregister(led);
+			kfree(led);
+		}
+	}
+#endif
+
 	/* Deallocate memory */
 	kfree(entry);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/2] HID: hid-lg4ff: Updated Comments
  2012-04-18  7:03     ` [PATCH 1/2] " Simon Wood
@ 2012-04-18  7:03       ` Simon Wood
  2012-04-18 11:24       ` [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs Jiri Kosina
  1 sibling, 0 replies; 25+ messages in thread
From: Simon Wood @ 2012-04-18  7:03 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Jiri Kosina, Michael Bauer, Michal Maly, Simon Wood

Updated comments to say that this driver now supports all Logitech
gaming wheels, and not just the WiiWheel

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-lg4ff.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index ab5232e..cbafb46 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -1,7 +1,8 @@
 /*
- *  Force feedback support for Logitech Speed Force Wireless
+ *  Force feedback support for Logitech Gaming Wheels
  *
- *  http://wiibrew.org/wiki/Logitech_USB_steering_wheel
+ *  Including G27, G25, DFP, DFGT, FFEX, Momo, Momo2 &
+ *  Wireless Speed Force (WiiWheel)
  *
  *  Copyright (c) 2010 Simon Wood <simon@mungewell.org>
  */
@@ -587,7 +588,7 @@ err:
 	}
 #endif
 
-	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by Simon Wood <simon@mungewell.org>\n");
+	hid_info(hid, "Force feedback for Logitech Gaming Wheels\n");
 	return 0;
 }
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs
  2012-04-18  7:03     ` [PATCH 1/2] " Simon Wood
  2012-04-18  7:03       ` [PATCH 2/2] HID: hid-lg4ff: Updated Comments Simon Wood
@ 2012-04-18 11:24       ` Jiri Kosina
  2012-04-18 14:58         ` simon
  1 sibling, 1 reply; 25+ messages in thread
From: Jiri Kosina @ 2012-04-18 11:24 UTC (permalink / raw)
  To: Simon Wood; +Cc: linux-input, linux-kernel, Michael Bauer, Michal Maly

On Wed, 18 Apr 2012, Simon Wood wrote:

> This patch adds supports for controlling the LED 'tachometer' on
> the G27 wheel, via the LED subsystem.
> 
> The 5 LEDs are arranged from right (1=grn, 2=grn, 3=yel, 4=yel, 5=red)
> and 'mirrored' to the left (10 LEDs in total).
> 
> Signed-off-by: Simon Wood <simon@mungewell.org>
> ---
>  drivers/hid/hid-lg4ff.c |  155 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 154 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
> index 32c173f..ab5232e 100644
> --- a/drivers/hid/hid-lg4ff.c
> +++ b/drivers/hid/hid-lg4ff.c
> @@ -55,7 +55,8 @@ struct lg4ff_device_entry {
>  	__u16 range;
>  	__u16 min_range;
>  	__u16 max_range;
> -	__u8  leds;
> +	__u8  led_state;
> +	struct led_classdev *led[5];

Why not put this under the CONFIG_LEDS_CLASS condition as well?

>  	struct list_head list;
>  	void (*set_range)(struct hid_device *hid, u16 range);
>  };
> @@ -335,6 +336,88 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
>  	return count;
>  }
>  
> +#ifdef CONFIG_LEDS_CLASS
> +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 = led_cdev->dev->parent;
> +	struct hid_device *hid = container_of(dev, struct hid_device, dev);
> +	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
> +	struct lg4ff_device_entry *entry;
> +	int i, state = 0;
> +
> +	if (!drv_data) {
> +		hid_err(hid, "Device data not found.");
> +		return;
> +	}
> +
> +	entry = (struct lg4ff_device_entry *)drv_data->device_props;
> +
> +	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 = led_cdev->dev->parent;
> +	struct hid_device *hid = container_of(dev, struct hid_device, dev);
> +	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
> +	struct lg4ff_device_entry *entry;
> +	int i, value = 0;
> +
> +	if (!drv_data) {
> +		hid_err(hid, "Device data not found.");
> +		return LED_OFF;
> +	}
> +
> +	entry = (struct lg4ff_device_entry *)drv_data->device_props;
> +
> +	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;
> +}
> +#endif
> +
>  int lg4ff_init(struct hid_device *hid)
>  {
>  	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
> @@ -453,6 +536,57 @@ 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;
> +	for (j = 0; j < 5; j++)
> +		entry->led[j] = NULL;
> +

The same here.

> +#ifdef CONFIG_LEDS_CLASS
> +	if (lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_G27_WHEEL) {
> +		struct led_classdev *led;
> +		size_t name_sz;
> +		char *name;
> +
> +		lg4ff_set_leds(hid, 0);
> +
> +		name_sz = strlen(dev_name(&hid->dev)) + 8;
> +
> +		for (j = 0; j < 5; j++) {
> +			led = kzalloc(sizeof(struct led_classdev)+name_sz, GFP_KERNEL);
> +			if (!led) {
> +				hid_err(hid, "can't allocate memory for LED %d\n", j);
> +				goto err;
> +			}
> +
> +			name = (void *)(&led[1]);
> +			snprintf(name, name_sz, "%s::RPM%d", dev_name(&hid->dev), j+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[j] = led;
> +			error = led_classdev_register(&hid->dev, led);
> +
> +			if (error) {
> +				hid_err(hid, "failed to register LED %d. Aborting.\n", j);
> +err:
> +				/* Deregister LEDs (if any) but let the driver continue */
> +				for (j = 0; j < 5; j++) {
> +					led = entry->led[j];
> +					entry->led[j] = NULL;
> +					if (!led)
> +						continue;
> +					led_classdev_unregister(led);
> +				kfree(led);

Indentation seems to be wrong here.

> +				}
> +				break;	/* Don't create any more LEDs */

Putting the whole err: label outside the outer for cycle would make the 
whole thing much more readable (and would save you the break). Could you 
please redo this?

> +			}
> +		}
> +	}
> +#endif
> +
>  	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by Simon Wood <simon@mungewell.org>\n");
>  	return 0;
>  }
> @@ -474,6 +608,25 @@ int lg4ff_deinit(struct hid_device *hid)
>  		hid_err(hid, "Error while deinitializing device, no device properties data.\n");
>  		return -1;
>  	}
> +
> +#ifdef CONFIG_LEDS_CLASS
> +	{
> +		int j;
> +		struct led_classdev *led;
> +
> +		/* Deregister LEDs (if any) */
> +		for (j = 0; j < 5; j++) {
> +
> +			led = entry->led[j];
> +			entry->led[j] = NULL;
> +			if (!led)
> +				continue;
> +			led_classdev_unregister(led);
> +			kfree(led);
> +		}
> +	}
> +#endif
> +
>  	/* Deallocate memory */
>  	kfree(entry);
>  

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs
  2012-04-18 11:24       ` [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs Jiri Kosina
@ 2012-04-18 14:58         ` simon
  2012-04-20 10:05           ` Jiri Kosina
  0 siblings, 1 reply; 25+ messages in thread
From: simon @ 2012-04-18 14:58 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Simon Wood, linux-input, linux-kernel, Michael Bauer, Michal Maly

>> +	__u8  led_state;
>> +	struct led_classdev *led[5];
>
> Why not put this under the CONFIG_LEDS_CLASS condition as well?

Can do, I don't think these are referenced anywhere else.

>> +	/* register led subsystem - G27 only */
>> +	entry->led_state = 0;
>> +	for (j = 0; j < 5; j++)
>> +		entry->led[j] = NULL;
>> +
>
> The same here.

OK.

>> +			if (error) {
>> +				hid_err(hid, "failed to register LED %d. Aborting.\n", j);
>> +err:
>> +				/* Deregister LEDs (if any) but let the driver continue */
>> +				for (j = 0; j < 5; j++) {
>> +					led = entry->led[j];
>> +					entry->led[j] = NULL;
>> +					if (!led)
>> +						continue;
>> +					led_classdev_unregister(led);
>> +				kfree(led);
>
> Indentation seems to be wrong here.

Bugger...

>
>> +				}
>> +				break;	/* Don't create any more LEDs */
>
> Putting the whole err: label outside the outer for cycle would make the
> whole thing much more readable (and would save you the break). Could you
> please redo this?

The 'break' is actually not needed as 'j' has reached maximum value and
the outer for loop would be at completion. I put the 'break' there just to
highlight this fact.

Maybe just a comment would be OK, such as 'on error fall though to driver
completion'.

The reason I'd like this code here is that it seems traditional to output
the following 'hid_info' line once the driver is fully active.

>>  	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by
>> Simon Wood <simon@mungewell.org>\n");

Moving the 'err:' code would have to:
1). Jump back to hit this hid_info line
2). Duplicate the hid_info line
3). Have err code called as a function

Any major objections to keeping it as is?


Man, it seems to be taking me ages to get this code right....
Simon


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs
  2012-04-18 14:58         ` simon
@ 2012-04-20 10:05           ` Jiri Kosina
  2012-04-21 12:41             ` Simon Wood
  0 siblings, 1 reply; 25+ messages in thread
From: Jiri Kosina @ 2012-04-20 10:05 UTC (permalink / raw)
  To: Simon Wood; +Cc: linux-input, linux-kernel, Michael Bauer, Michal Maly

On Wed, 18 Apr 2012, simon@mungewell.org wrote:

> > Putting the whole err: label outside the outer for cycle would make the
> > whole thing much more readable (and would save you the break). Could you
> > please redo this?
> 
> The 'break' is actually not needed as 'j' has reached maximum value and
> the outer for loop would be at completion. 

Ah, right, I have even missed this fact, that you are using the same index 
in the inner loop as in the outter loop.

Quite unreadable indeed :)

> I put the 'break' there just to highlight this fact.
> 
> Maybe just a comment would be OK, such as 'on error fall though to driver
> completion'.
> 
> The reason I'd like this code here is that it seems traditional to output
> the following 'hid_info' line once the driver is fully active.
> 
> >>  	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by
> >> Simon Wood <simon@mungewell.org>\n");
> 
> Moving the 'err:' code would have to:
> 1). Jump back to hit this hid_info line
> 2). Duplicate the hid_info line
> 3). Have err code called as a function

I am not getting it. Why can't you do just something like


	for ( .... ) {
		/* outter loop */
		if (error) {
			for ( ... ) {
				/* inner loop */
			}
			goto out;
		}
	}
out:
	hid_info(....);
	return;

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs
  2012-04-20 10:05           ` Jiri Kosina
@ 2012-04-21 12:41             ` Simon Wood
  2012-04-21 12:41               ` [PATCH 2/2] HID: hid-lg4ff: Updated Comments Simon Wood
  2012-04-23 18:55               ` [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs Jiri Kosina
  0 siblings, 2 replies; 25+ messages in thread
From: Simon Wood @ 2012-04-21 12:41 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Jiri Kosina, Michael Bauer, Michal Maly, Simon Wood

This patch adds supports for controlling the LED 'tachometer' on
the G27 wheel, via the LED subsystem.

The 5 LEDs are arranged from right (1=grn, 2=grn, 3=yel, 4=yel, 5=red)
and 'mirrored' to the left (10 LEDs in total).

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-lg4ff.c |  158 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 157 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 32c173f..1e942a0 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -55,7 +55,10 @@ struct lg4ff_device_entry {
 	__u16 range;
 	__u16 min_range;
 	__u16 max_range;
-	__u8  leds;
+#ifdef CONFIG_LEDS_CLASS
+	__u8  led_state;
+	struct led_classdev *led[5];
+#endif
 	struct list_head list;
 	void (*set_range)(struct hid_device *hid, u16 range);
 };
@@ -335,6 +338,88 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
 	return count;
 }
 
+#ifdef CONFIG_LEDS_CLASS
+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 = led_cdev->dev->parent;
+	struct hid_device *hid = container_of(dev, struct hid_device, dev);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+	struct lg4ff_device_entry *entry;
+	int i, state = 0;
+
+	if (!drv_data) {
+		hid_err(hid, "Device data not found.");
+		return;
+	}
+
+	entry = (struct lg4ff_device_entry *)drv_data->device_props;
+
+	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 = led_cdev->dev->parent;
+	struct hid_device *hid = container_of(dev, struct hid_device, dev);
+	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
+	struct lg4ff_device_entry *entry;
+	int i, value = 0;
+
+	if (!drv_data) {
+		hid_err(hid, "Device data not found.");
+		return LED_OFF;
+	}
+
+	entry = (struct lg4ff_device_entry *)drv_data->device_props;
+
+	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;
+}
+#endif
+
 int lg4ff_init(struct hid_device *hid)
 {
 	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
@@ -453,6 +538,58 @@ int lg4ff_init(struct hid_device *hid)
 	if (entry->set_range != NULL)
 		entry->set_range(hid, entry->range);
 
+#ifdef CONFIG_LEDS_CLASS
+	/* register led subsystem - G27 only */
+	entry->led_state = 0;
+	for (j = 0; j < 5; j++)
+		entry->led[j] = NULL;
+
+	if (lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_G27_WHEEL) {
+		struct led_classdev *led;
+		size_t name_sz;
+		char *name;
+
+		lg4ff_set_leds(hid, 0);
+
+		name_sz = strlen(dev_name(&hid->dev)) + 8;
+
+		for (j = 0; j < 5; j++) {
+			led = kzalloc(sizeof(struct led_classdev)+name_sz, GFP_KERNEL);
+			if (!led) {
+				hid_err(hid, "can't allocate memory for LED %d\n", j);
+				goto err;
+			}
+
+			name = (void *)(&led[1]);
+			snprintf(name, name_sz, "%s::RPM%d", dev_name(&hid->dev), j+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[j] = led;
+			error = led_classdev_register(&hid->dev, led);
+
+			if (error) {
+				hid_err(hid, "failed to register LED %d. Aborting.\n", j);
+err:
+				/* Deregister LEDs (if any) */
+				for (j = 0; j < 5; j++) {
+					led = entry->led[j];
+					entry->led[j] = NULL;
+					if (!led)
+						continue;
+					led_classdev_unregister(led);
+					kfree(led);
+				}
+				goto out;	/* Let the driver continue without LEDs */
+			}
+		}
+	}
+#endif
+
+out:
 	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by Simon Wood <simon@mungewell.org>\n");
 	return 0;
 }
@@ -474,6 +611,25 @@ int lg4ff_deinit(struct hid_device *hid)
 		hid_err(hid, "Error while deinitializing device, no device properties data.\n");
 		return -1;
 	}
+
+#ifdef CONFIG_LEDS_CLASS
+	{
+		int j;
+		struct led_classdev *led;
+
+		/* Deregister LEDs (if any) */
+		for (j = 0; j < 5; j++) {
+
+			led = entry->led[j];
+			entry->led[j] = NULL;
+			if (!led)
+				continue;
+			led_classdev_unregister(led);
+			kfree(led);
+		}
+	}
+#endif
+
 	/* Deallocate memory */
 	kfree(entry);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/2] HID: hid-lg4ff: Updated Comments
  2012-04-21 12:41             ` Simon Wood
@ 2012-04-21 12:41               ` Simon Wood
  2012-04-23 18:55               ` [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs Jiri Kosina
  1 sibling, 0 replies; 25+ messages in thread
From: Simon Wood @ 2012-04-21 12:41 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Jiri Kosina, Michael Bauer, Michal Maly, Simon Wood

Updated comments to say that this driver now supports all Logitech
gaming wheels, and not just the WiiWheel.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-lg4ff.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 1e942a0..24cf037 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -1,7 +1,8 @@
 /*
- *  Force feedback support for Logitech Speed Force Wireless
+ *  Force feedback support for Logitech Gaming Wheels
  *
- *  http://wiibrew.org/wiki/Logitech_USB_steering_wheel
+ *  Including G27, G25, DFP, DFGT, FFEX, Momo, Momo2 &
+ *  Speed Force Wireless (WiiWheel)
  *
  *  Copyright (c) 2010 Simon Wood <simon@mungewell.org>
  */
@@ -590,7 +591,7 @@ err:
 #endif
 
 out:
-	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by Simon Wood <simon@mungewell.org>\n");
+	hid_info(hid, "Force feedback support for Logitech Gaming Wheels\n");
 	return 0;
 }
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs
  2012-04-21 12:41             ` Simon Wood
  2012-04-21 12:41               ` [PATCH 2/2] HID: hid-lg4ff: Updated Comments Simon Wood
@ 2012-04-23 18:55               ` Jiri Kosina
  1 sibling, 0 replies; 25+ messages in thread
From: Jiri Kosina @ 2012-04-23 18:55 UTC (permalink / raw)
  To: Simon Wood; +Cc: linux-input, linux-kernel, Michael Bauer, Michal Maly

On Sat, 21 Apr 2012, Simon Wood wrote:

> This patch adds supports for controlling the LED 'tachometer' on
> the G27 wheel, via the LED subsystem.
> 
> The 5 LEDs are arranged from right (1=grn, 2=grn, 3=yel, 4=yel, 5=red)
> and 'mirrored' to the left (10 LEDs in total).
> 
> Signed-off-by: Simon Wood <simon@mungewell.org>

Both patches applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-13 21:24 ` Jiri Kosina
  2012-03-13 21:44   ` simon
@ 2012-03-14  1:51   ` simon
  1 sibling, 0 replies; 25+ messages in thread
From: simon @ 2012-03-14  1:51 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: simon, Mark Brown, linux-input, linux-kernel, Michael Bauer, Michal Maly


>
> Could you please post the full oops?

Just so you don't burn time on this. I think I've got it figured, no
longer Oops (just locks up the machine the second time wheel is
connected).

Going home to try with a real G27...
Simon


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-13 21:24 ` Jiri Kosina
@ 2012-03-13 21:44   ` simon
  2012-03-14  1:51   ` simon
  1 sibling, 0 replies; 25+ messages in thread
From: simon @ 2012-03-13 21:44 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: simon, Mark Brown, linux-input, linux-kernel, Michael Bauer, Michal Maly


> Could you please post the full oops?

--
Mar 13 14:05:55 ubuntu kernel: [ 3286.170380] logitech
0003:046D:C294.0002: input,hidraw0: USB HID v1.11 Joystick [PS3/USB
Cordless Wheel] on usb-0000:02:00.0-1/input0
Mar 13 14:05:55 ubuntu kernel: [ 3286.171462] BUG: unable to handle kernel
NULL pointer dereference at 00000091
Mar 13 14:05:55 ubuntu kernel: [ 3286.171543] IP: [<e09f3998>]
lg4ff_led_get_brightness+0x28/0x90 [hid_logitech]
Mar 13 14:05:55 ubuntu kernel: [ 3286.171931] *pde = 00000000
Mar 13 14:05:55 ubuntu kernel: [ 3286.171991] Oops: 0000 [#1] SMP
Mar 13 14:05:55 ubuntu kernel: [ 3286.172253] Modules linked in:
hid_logitech joydev ff_memless usbhid hid binfmt_misc snd_ens1371 gameport
snd_ac97_codec ac97_bus snd_pcm snd_seq_midi snd_rawmidi
snd_seq_midi_event snd_seq snd_timer snd_seq_device snd soundcore
i2c_piix4 ppdev snd_page_alloc shpchp psmouse serio_raw lp vmw_balloon
parport_pc parport floppy pcnet32 mptspi mptscsih mptbase
scsi_transport_spi [last unloaded: hid_logitech]
Mar 13 14:05:55 ubuntu kernel: [ 3286.172434]
Mar 13 14:05:55 ubuntu kernel: [ 3286.172517] Pid: 16, comm: khubd Not
tainted 3.3.0-rc6-stock+ #10 VMware, Inc. VMware Virtual Platform/440BX
Desktop Reference Platform
Mar 13 14:05:55 ubuntu kernel: [ 3286.173257] EIP: 0060:[<e09f3998>]
EFLAGS: 00010202 CPU: 0
Mar 13 14:05:55 ubuntu kernel: [ 3286.173325] EIP is at
lg4ff_led_get_brightness+0x28/0x90 [hid_logitech]
Mar 13 14:05:55 ubuntu kernel: [ 3286.173393] EAX: 00000089 EBX: dd3d2540
ECX: 00000000 EDX: ddc9a000
Mar 13 14:05:55 ubuntu kernel: [ 3286.173406] ESI: dd3a51b8 EDI: dd3a51b8
EBP: de59fa94 ESP: de59fa84
Mar 13 14:05:55 ubuntu kernel: [ 3286.173419]  DS: 007b ES: 007b FS: 00d8
GS: 00e0 SS: 0068
Mar 13 14:05:55 ubuntu kernel: [ 3286.173452] Process khubd (pid: 16,
ti=de59e000 task=de4cbf70 task.ti=de59e000)
Mar 13 14:05:55 ubuntu kernel: [ 3286.173471] Stack:
Mar 13 14:05:55 ubuntu kernel: [ 3286.173517]  c13498ed de59fa94 dd3d2540
dda080a0 de59fab8 c1439119 de57c5c0 dd3a51b8
Mar 13 14:05:55 ubuntu kernel: [ 3286.173681]  00000000 dd3d2540 c16f79e0
e09f5e1d 00000000 de59fafc e09f4311 00002200
Mar 13 14:05:55 ubuntu kernel: [ 3286.173689]  00000000 dd3d23c0 00000002
ddc9ac64 00000000 dd3a51b8 00010800 dde10022
Mar 13 14:05:55 ubuntu kernel: [ 3286.173738] Call Trace:
Mar 13 14:05:55 ubuntu kernel: [ 3286.174090]  [<c13498ed>] ?
device_create+0x2d/0x30
Mar 13 14:05:55 ubuntu kernel: [ 3286.174212]  [<c1439119>]
led_classdev_register+0x99/0xf0
Mar 13 14:05:55 ubuntu kernel: [ 3286.174269]  [<e09f4311>]
lg4ff_init+0x2a1/0x570 [hid_logitech]
Mar 13 14:05:55 ubuntu kernel: [ 3286.174276]  [<e09f2427>]
lg_probe+0x147/0x2c0 [hid_logitech]
Mar 13 14:05:55 ubuntu kernel: [ 3286.174360]  [<c1027938>] ?
default_spin_lock_flags+0x8/0x10
Mar 13 14:05:55 ubuntu kernel: [ 3286.174638]  [<c153b6af>] ?
_raw_spin_lock_irqsave+0x2f/0x50
Mar 13 14:05:55 ubuntu kernel: [ 3286.174672]  [<e0a025a3>] ?
hid_match_device+0x83/0x90 [hid]
Mar 13 14:05:55 ubuntu kernel: [ 3286.174680]  [<e0a02bf0>]
hid_device_probe+0x80/0x100 [hid]
Mar 13 14:05:55 ubuntu kernel: [ 3286.174685]  [<c134b452>] ?
driver_sysfs_add+0x72/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174692]  [<c134b6cf>]
driver_probe_device+0x7f/0x190
Mar 13 14:05:55 ubuntu kernel: [ 3286.174698]  [<c134b8b9>]
__device_attach+0x49/0x60
Mar 13 14:05:55 ubuntu kernel: [ 3286.174703]  [<c134b870>] ?
__driver_attach+0x90/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174707]  [<c134a0ff>]
bus_for_each_drv+0x4f/0x70
Mar 13 14:05:55 ubuntu kernel: [ 3286.174712]  [<c134b5aa>]
device_attach+0x8a/0xa0
Mar 13 14:05:55 ubuntu kernel: [ 3286.174716]  [<c134b870>] ?
__driver_attach+0x90/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174721]  [<c134ae3f>]
bus_probe_device+0x6f/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174725]  [<c1349710>]
device_add+0x560/0x610
Mar 13 14:05:55 ubuntu kernel: [ 3286.174732]  [<e0a026b2>]
hid_add_device+0x92/0x1f0 [hid]
Mar 13 14:05:55 ubuntu kernel: [ 3286.174742]  [<e09a5c26>]
usbhid_probe+0x2e6/0x430 [usbhid]
Mar 13 14:05:55 ubuntu kernel: [ 3286.174753]  [<c13c7c06>]
usb_probe_interface+0xd6/0x1b0
Mar 13 14:05:55 ubuntu kernel: [ 3286.174766]  [<c11898c7>] ?
sysfs_create_link+0x17/0x20
Mar 13 14:05:55 ubuntu kernel: [ 3286.174772]  [<c134b6cf>]
driver_probe_device+0x7f/0x190
Mar 13 14:05:55 ubuntu kernel: [ 3286.174776]  [<c13c75fb>] ?
usb_device_match+0x6b/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174781]  [<c134b8b9>]
__device_attach+0x49/0x60
Mar 13 14:05:55 ubuntu kernel: [ 3286.174813]  [<c134b870>] ?
__driver_attach+0x90/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174821]  [<c134a0ff>]
bus_for_each_drv+0x4f/0x70
Mar 13 14:05:55 ubuntu kernel: [ 3286.174825]  [<c134b5aa>]
device_attach+0x8a/0xa0
Mar 13 14:05:55 ubuntu kernel: [ 3286.174830]  [<c134b870>] ?
__driver_attach+0x90/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174834]  [<c134ae3f>]
bus_probe_device+0x6f/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174839]  [<c1349710>]
device_add+0x560/0x610
Mar 13 14:05:55 ubuntu kernel: [ 3286.174845]  [<c13c6385>] ?
usb_set_configuration+0x4c5/0x650
Mar 13 14:05:55 ubuntu kernel: [ 3286.174850]  [<c13c63ee>]
usb_set_configuration+0x52e/0x650
Mar 13 14:05:55 ubuntu kernel: [ 3286.174859]  [<c13ce5eb>]
generic_probe+0x3b/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174864]  [<c11898c7>] ?
sysfs_create_link+0x17/0x20
Mar 13 14:05:55 ubuntu kernel: [ 3286.174868]  [<c13c7d06>]
usb_probe_device+0x26/0x50
Mar 13 14:05:55 ubuntu kernel: [ 3286.174873]  [<c134b6cf>]
driver_probe_device+0x7f/0x190
Mar 13 14:05:55 ubuntu kernel: [ 3286.174908]  [<c128c165>] ?
kobject_uevent_env+0x105/0x420
Mar 13 14:05:55 ubuntu kernel: [ 3286.174917]  [<c153b50d>] ?
_raw_spin_lock+0xd/0x10
Mar 13 14:05:55 ubuntu kernel: [ 3286.174922]  [<c134b8b9>]
__device_attach+0x49/0x60
Mar 13 14:05:55 ubuntu kernel: [ 3286.174925]  [<c134b870>] ?
__driver_attach+0x90/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174929]  [<c134a0ff>]
bus_for_each_drv+0x4f/0x70
Mar 13 14:05:55 ubuntu kernel: [ 3286.174934]  [<c134b5aa>]
device_attach+0x8a/0xa0
Mar 13 14:05:55 ubuntu kernel: [ 3286.174938]  [<c134b870>] ?
__driver_attach+0x90/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174942]  [<c134ae3f>]
bus_probe_device+0x6f/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.174947]  [<c1349710>]
device_add+0x560/0x610
Mar 13 14:05:55 ubuntu kernel: [ 3286.174956]  [<c13bcf00>] ?
usb_enumerate_device+0x20/0x100
Mar 13 14:05:55 ubuntu kernel: [ 3286.174965]  [<c13547c9>] ?
pm_runtime_forbid+0x49/0x60
Mar 13 14:05:55 ubuntu kernel: [ 3286.174969]  [<c13bf720>]
usb_new_device+0xe0/0x120
Mar 13 14:05:55 ubuntu kernel: [ 3286.174974]  [<c13c0b64>] ?
hub_thread+0xc44/0xfd0
Mar 13 14:05:55 ubuntu kernel: [ 3286.174978]  [<c13c094f>]
hub_thread+0xa2f/0xfd0
Mar 13 14:05:55 ubuntu kernel: [ 3286.174988]  [<c1052220>] ?
wake_up_bit+0x30/0x30
Mar 13 14:05:55 ubuntu kernel: [ 3286.174993]  [<c13bff20>] ?
usb_remote_wakeup+0x50/0x50
Mar 13 14:05:55 ubuntu kernel: [ 3286.174997]  [<c1051c3c>] kthread+0x7c/0x90
Mar 13 14:05:55 ubuntu kernel: [ 3286.175001]  [<c1051bc0>] ?
flush_kthread_worker+0xb0/0xb0
Mar 13 14:05:55 ubuntu kernel: [ 3286.175028]  [<c15428be>]
kernel_thread_helper+0x6/0x10
Mar 13 14:05:55 ubuntu kernel: [ 3286.175075] Code: 00 00 00 55 89 e5 56
53 83 ec 08 3e 8d 74 26 00 89 c3 8b 40 1c 8b 30 89 f0 e8 a5 78 95 e0 85 c0
74 39 8b 40 04 31 c9 85 c0 74 48 <39> 5c 88 08 74 12 83 c1 01 83 f9 05 75
f2 31 c0 83 c4 08 5b 5e
Mar 13 14:05:55 ubuntu kernel: [ 3286.175218] EIP: [<e09f3998>]
lg4ff_led_get_brightness+0x28/0x90 [hid_logitech] SS:ESP 0068:de59fa84
Mar 13 14:05:55 ubuntu kernel: [ 3286.175263] CR2: 0000000000000091
Mar 13 14:05:55 ubuntu kernel: [ 3286.175702] ---[ end trace
bae956db68dd6567 ]---
Mar 13 14:06:00 ubuntu dhclient: DHCPREQUEST of 192.168.227.143 on eth0 to
192.168.227.254 port 67
Mar 13 14:06:00 ubuntu dhclient: DHCPACK of 192.168.227.143 from
192.168.227.254
Mar 13 14:06:00 ubuntu dhclient: bound to 192.168.227.143 -- renewal in
736 seconds.
--


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-13 21:08 [PATCH 1/2] HID: hid-lg4ff add " simon
  2012-03-13 21:11 ` simon
@ 2012-03-13 21:24 ` Jiri Kosina
  2012-03-13 21:44   ` simon
  2012-03-14  1:51   ` simon
  1 sibling, 2 replies; 25+ messages in thread
From: Jiri Kosina @ 2012-03-13 21:24 UTC (permalink / raw)
  To: simon; +Cc: Mark Brown, linux-input, linux-kernel, Michael Bauer, Michal Maly

On Tue, 13 Mar 2012, simon@mungewell.org wrote:

> > Not really, it's rather simple to use and as Mark said, it's a proper
> infrastructure to use here.
> > We already have HID drivers which are using this -- you can look at
> PicoLCD or Wiimote drivers and you'll see that it's indeed fairly
> simple.
> 
> So _you_ say.... I'm getting an Oops when device attempts to init the LED
> subsytem. :-(
> --
> Mar 13 12:54:56 ubuntu kernel: [  458.639674] Call Trace:
> Mar 13 12:54:56 ubuntu kernel: [  458.641143]  [<c13498ed>] ?
> device_create+0x2d/0x30
> Mar 13 12:54:56 ubuntu kernel: [  458.641245]  [<c1439119>]
> led_classdev_register+0x99/0xf0
> Mar 13 12:54:56 ubuntu kernel: [  458.641306]  [<e0977311>]
> lg4ff_init+0x2a1/0x570 [hid_logitech]
> Mar 13 12:54:56 ubuntu kernel: [  458.641313]  [<e0975427>]
> lg_probe+0x147/0x2c0 [hid_logitech]
> --

Could you please post the full oops?

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-13 21:08 [PATCH 1/2] HID: hid-lg4ff add " simon
@ 2012-03-13 21:11 ` simon
  2012-03-13 21:24 ` Jiri Kosina
  1 sibling, 0 replies; 25+ messages in thread
From: simon @ 2012-03-13 21:11 UTC (permalink / raw)
  Cc: Jiri Kosina, Mark Brown, linux-input, linux-kernel,
	Michael Bauer, Michal Maly

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

> Code snipet below (full file attached):

Whoops, wrong file attached.
Simon.

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

/*
 *  Force feedback support for Logitech Speed Force Wireless
 *
 *  http://wiibrew.org/wiki/Logitech_USB_steering_wheel
 *
 *  Copyright (c) 2010 Simon Wood <simon@mungewell.org>
 */

/*
 * 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.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 */


#include <linux/input.h>
#include <linux/usb.h>
#include <linux/hid.h>
#include <linux/leds.h>

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

#define DFGT_REV_MAJ 0x13
#define DFGT_REV_MIN 0x22
#define DFP_REV_MAJ 0x11
#define DFP_REV_MIN 0x06
#define FFEX_REV_MAJ 0x21
#define FFEX_REV_MIN 0x00
#define G25_REV_MAJ 0x12
#define G25_REV_MIN 0x22
#define G27_REV_MAJ 0x12
#define G27_REV_MIN 0x38

#define to_hid_device(pdev) container_of(pdev, struct hid_device, dev)

static void hid_lg4ff_set_range_dfp(struct hid_device *hid, u16 range);
static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range);
static ssize_t lg4ff_range_show(struct device *dev, struct device_attribute *attr, char *buf);
static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count);

static DEVICE_ATTR(range, S_IRWXU | S_IRWXG | S_IRWXO, lg4ff_range_show, lg4ff_range_store);

#if 0
struct g27_leds {
	char * name;
};

static struct g27_leds g27_leds[] = {
#else
static char * g27_leds[] = {
#endif
	"g27:green:rpm1",
	"g27:green:rpm2",
	"g27:yellow:rpm3",
	"g27:yellow:rpm4",
	"g27:red:rpm5"
	};

struct lg4ff_device_entry {
	__u16 range;
	__u16 min_range;
	__u16 max_range;
	__u8  led_state;
	struct led_classdev *led[5];
	void (*set_range)(struct hid_device *hid, u16 range);
};

static const signed short lg4ff_wheel_effects[] = {
	FF_CONSTANT,
	FF_AUTOCENTER,
	-1
};

struct lg4ff_wheel {
	const __u32 product_id;
	const signed short *ff_effects;
	const __u16 min_range;
	const __u16 max_range;
	void (*set_range)(struct hid_device *hid, u16 range);
};

static const struct lg4ff_wheel lg4ff_devices[] = {
	{USB_DEVICE_ID_LOGITECH_WHEEL,       lg4ff_wheel_effects, 40, 270, NULL},
	{USB_DEVICE_ID_LOGITECH_MOMO_WHEEL,  lg4ff_wheel_effects, 40, 270, NULL},
	{USB_DEVICE_ID_LOGITECH_DFP_WHEEL,   lg4ff_wheel_effects, 40, 900, hid_lg4ff_set_range_dfp},
	{USB_DEVICE_ID_LOGITECH_G25_WHEEL,   lg4ff_wheel_effects, 40, 900, hid_lg4ff_set_range_g25},
	{USB_DEVICE_ID_LOGITECH_DFGT_WHEEL,  lg4ff_wheel_effects, 40, 900, hid_lg4ff_set_range_g25},
	{USB_DEVICE_ID_LOGITECH_G27_WHEEL,   lg4ff_wheel_effects, 40, 900, hid_lg4ff_set_range_g25},
	{USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2, lg4ff_wheel_effects, 40, 270, NULL},
	{USB_DEVICE_ID_LOGITECH_WII_WHEEL,   lg4ff_wheel_effects, 40, 270, NULL}
};

struct lg4ff_native_cmd {
	const u8 cmd_num;	/* Number of commands to send */
	const u8 cmd[];
};

struct lg4ff_usb_revision {
	const u16 rev_maj;
	const u16 rev_min;
	const struct lg4ff_native_cmd *command;
};

static const struct lg4ff_native_cmd native_dfp = {
	1,
	{0xf8, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00}
};

static const struct lg4ff_native_cmd native_dfgt = {
	2,
	{0xf8, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00,	/* 1st command */
	 0xf8, 0x09, 0x03, 0x01, 0x00, 0x00, 0x00}	/* 2nd command */
};

static const struct lg4ff_native_cmd native_g25 = {
	1,
	{0xf8, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00}
};

static const struct lg4ff_native_cmd native_g27 = {
	2,
	{0xf8, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00,	/* 1st command */
	 0xf8, 0x09, 0x04, 0x01, 0x00, 0x00, 0x00}	/* 2nd command */
};

static const struct lg4ff_usb_revision lg4ff_revs[] = {
	{DFGT_REV_MAJ, DFGT_REV_MIN, &native_dfgt},	/* Driving Force GT */
	{DFP_REV_MAJ,  DFP_REV_MIN,  &native_dfp},	/* Driving Force Pro */
	{G25_REV_MAJ,  G25_REV_MIN,  &native_g25},	/* G25 */
	{G27_REV_MAJ,  G27_REV_MIN,  &native_g27},	/* G27 */
};

static int hid_lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effect)
{
	struct hid_device *hid = input_get_drvdata(dev);
	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);
	int x;

#define CLAMP(x) if (x < 0) x = 0; if (x > 0xff) x = 0xff

	switch (effect->type) {
	case FF_CONSTANT:
		x = effect->u.ramp.start_level + 0x80;	/* 0x80 is no force */
		CLAMP(x);
		report->field[0]->value[0] = 0x11;	/* Slot 1 */
		report->field[0]->value[1] = 0x08;
		report->field[0]->value[2] = x;
		report->field[0]->value[3] = 0x80;
		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);
		break;
	}
	return 0;
}

/* Sends default autocentering command compatible with
 * all wheels except Formula Force EX */
static void hid_lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
{
	struct hid_device *hid = input_get_drvdata(dev);
	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] = 0xfe;
	report->field[0]->value[1] = 0x0d;
	report->field[0]->value[2] = magnitude >> 13;
	report->field[0]->value[3] = magnitude >> 13;
	report->field[0]->value[4] = magnitude >> 8;
	report->field[0]->value[5] = 0x00;
	report->field[0]->value[6] = 0x00;

	usbhid_submit_report(hid, report, USB_DIR_OUT);
}

/* Sends autocentering command compatible with Formula Force EX */
static void hid_lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
{
	struct hid_device *hid = input_get_drvdata(dev);
	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);
	magnitude = magnitude * 90 / 65535;
	

	report->field[0]->value[0] = 0xfe;
	report->field[0]->value[1] = 0x03;
	report->field[0]->value[2] = magnitude >> 14;
	report->field[0]->value[3] = magnitude >> 14;
	report->field[0]->value[4] = magnitude;
	report->field[0]->value[5] = 0x00;
	report->field[0]->value[6] = 0x00;

	usbhid_submit_report(hid, report, USB_DIR_OUT);
}

/* Sends command to set range compatible with G25/G27/Driving Force GT */
static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range)
{
	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);
	dbg_hid("G25/G27/DFGT: setting range to %u\n", range);

	report->field[0]->value[0] = 0xf8;
	report->field[0]->value[1] = 0x81;
	report->field[0]->value[2] = range & 0x00ff;
	report->field[0]->value[3] = (range & 0xff00) >> 8;
	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);
}

/* Sends commands to set range compatible with Driving Force Pro wheel */
static void hid_lg4ff_set_range_dfp(struct hid_device *hid, __u16 range)
{
	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);
	int start_left, start_right, full_range;
	dbg_hid("Driving Force Pro: setting range to %u\n", range);

	/* Prepare "coarse" limit command */
	report->field[0]->value[0] = 0xf8;
	report->field[0]->value[1] = 0x00; 	/* Set later */
	report->field[0]->value[2] = 0x00;
	report->field[0]->value[3] = 0x00;
	report->field[0]->value[4] = 0x00;
	report->field[0]->value[5] = 0x00;
	report->field[0]->value[6] = 0x00;

	if (range > 200) {
		report->field[0]->value[1] = 0x03;
		full_range = 900;
	} else {
		report->field[0]->value[1] = 0x02;
		full_range = 200;
	}
	usbhid_submit_report(hid, report, USB_DIR_OUT);

	/* Prepare "fine" limit command */
	report->field[0]->value[0] = 0x81;
	report->field[0]->value[1] = 0x0b;
	report->field[0]->value[2] = 0x00;
	report->field[0]->value[3] = 0x00;
	report->field[0]->value[4] = 0x00;
	report->field[0]->value[5] = 0x00;
	report->field[0]->value[6] = 0x00;

	if (range == 200 || range == 900) {	/* Do not apply any fine limit */
		usbhid_submit_report(hid, report, USB_DIR_OUT);
		return;
	}

	/* Construct fine limit command */
	start_left = (((full_range - range + 1) * 2047) / full_range);
	start_right = 0xfff - start_left;

	report->field[0]->value[2] = start_left >> 4;
	report->field[0]->value[3] = start_right >> 4;
	report->field[0]->value[4] = 0xff;
	report->field[0]->value[5] = (start_right & 0xe) << 4 | (start_left & 0xe);
	report->field[0]->value[6] = 0xff;

	usbhid_submit_report(hid, report, USB_DIR_OUT);
}

static void hid_lg4ff_switch_native(struct hid_device *hid, const struct lg4ff_native_cmd *cmd)
{
	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);
	__u8 i, j;

	j = 0;
	while (j < 7*cmd->cmd_num) {
		for (i = 0; i < 7; i++)
			report->field[0]->value[i] = cmd->cmd[j++];

		usbhid_submit_report(hid, report, USB_DIR_OUT);
	}
}

/* Read current range and display it in terminal */
static ssize_t lg4ff_range_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	struct hid_device *hid = to_hid_device(dev);
	struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
	struct lg4ff_device_entry *uninitialized_var(entry);
	size_t count;
	
	if (!drv_data) {
		hid_err(hid, "Device data not found.");
		return 0;
	}
	
	entry = (struct lg4ff_device_entry *)drv_data->dev_props;
	if (!entry) {
		hid_err(hid, "Device properties not found.");
		return 0;
	}

	count = scnprintf(buf, PAGE_SIZE, "%u\n", entry->range);
	return count;
}

/* Set range to user specified value, call appropriate function
 * according to the type of the wheel */
static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
{
	struct hid_device *hid = to_hid_device(dev);
	struct lg_drv_data* drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
	struct lg4ff_device_entry *uninitialized_var(entry);
	__u16 range = simple_strtoul(buf, NULL, 10);

	if (!drv_data) {
		hid_err(hid, "Device data not found.");
		return count;
	}

	entry = (struct lg4ff_device_entry *)drv_data->dev_props; 
	if (!entry) {
		hid_err(hid, "Device properties not found.");
		return count;
	}

	if (range == 0)
		range = entry->max_range;

	/* Check if the wheel supports range setting
	 * and that the range is within limits for the wheel */
	if (entry->set_range != NULL && range >= entry->min_range && range <= entry->max_range) {
		entry->set_range(hid, range);
		entry->range = range;
	}

	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 = led_cdev->dev->parent;
	struct hid_device *hid = to_hid_device(dev);
	struct lg_drv_data* drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
	struct lg4ff_device_entry *uninitialized_var(entry);
	int i, state = 0;

	if (!drv_data) {
		hid_err(hid, "Device data not found.");
		return;
	}

	entry = (struct lg4ff_device_entry *)drv_data->dev_props; 
	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 = led_cdev->dev->parent;
	struct hid_device *hid = to_hid_device(dev);
	struct lg_drv_data* drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
	struct lg4ff_device_entry *uninitialized_var(entry);
	int i, value=0;

	if (!drv_data) {
		hid_err(hid, "Device data not found.");
		return LED_OFF;
	}

	entry = (struct lg4ff_device_entry *)drv_data->dev_props; 
	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);
	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
	struct input_dev *dev = hidinput->input;
	struct hid_report *report;
	struct hid_field *field;
	struct lg_drv_data *drv_data;
	struct lg4ff_device_entry *entry;
	struct usb_device_descriptor *udesc;
	int error, i, j;
	__u16 bcdDevice, rev_maj, rev_min;
	struct led_classdev *led;
	struct device *dev2 = &dev->dev;

	/* Find the report to use */
	if (list_empty(report_list)) {
		hid_err(hid, "No output report found\n");
		return -1;
	}

	/* Check that the report looks ok */
	report = list_entry(report_list->next, struct hid_report, list);
	if (!report) {
		hid_err(hid, "NULL output report\n");
		return -1;
	}

	field = report->field[0];
	if (!field) {
		hid_err(hid, "NULL field\n");
		return -1;
	}

	/* Check what wheel has been connected */
	for (i = 0; i < ARRAY_SIZE(lg4ff_devices); i++) {
		if (hid->product == lg4ff_devices[i].product_id) {
			dbg_hid("Found compatible device, product ID %04X\n", lg4ff_devices[i].product_id);
			break;
		}
	}

	if (i == ARRAY_SIZE(lg4ff_devices)) {
		hid_err(hid, "Device is not supported by lg4ff driver. If you think it should be, consider reporting a bug to"
			     "LKML, Simon Wood <simon@mungewell.org> or Michal Maly <madcatxster@gmail.com>\n");
		return -1;
	}

	/* Attempt to switch wheel to native mode when applicable */
	udesc = &(hid_to_usb_dev(hid)->descriptor);
	if (!udesc) {
		hid_err(hid, "NULL USB device descriptor\n");
		return -1;
	}
	bcdDevice = le16_to_cpu(udesc->bcdDevice);
	rev_maj = bcdDevice >> 8;
	rev_min = bcdDevice & 0xff;

	if (lg4ff_devices[i].product_id == USB_DEVICE_ID_LOGITECH_WHEEL) {
		dbg_hid("Generic wheel detected, can it do native?\n");
		dbg_hid("USB revision: %2x.%02x\n", rev_maj, rev_min);

		for (j = 0; j < ARRAY_SIZE(lg4ff_revs); j++) {
			if (lg4ff_revs[j].rev_maj == rev_maj && lg4ff_revs[j].rev_min == rev_min) {
				hid_lg4ff_switch_native(hid, lg4ff_revs[j].command);
				hid_info(hid, "Switched to native mode\n");
			}
		}
	}

	/* Set supported force feedback capabilities */
	for (j = 0; lg4ff_devices[i].ff_effects[j] >= 0; j++)
		set_bit(lg4ff_devices[i].ff_effects[j], dev->ffbit);

	error = input_ff_create_memless(dev, NULL, hid_lg4ff_play);

	if (error)
		return error;

	/* Check if autocentering is available and
	 * set the centering force to zero by default */
	if (test_bit(FF_AUTOCENTER, dev->ffbit)) {
		if(rev_maj == FFEX_REV_MAJ && rev_min == FFEX_REV_MIN)	/* Formula Force EX expects different autocentering command */
			dev->ff->set_autocenter = hid_lg4ff_set_autocenter_ffex;
		else
			dev->ff->set_autocenter = hid_lg4ff_set_autocenter_default;

		dev->ff->set_autocenter(dev, 0);
	}

	/* Add the device to device_list */
	entry = kzalloc(sizeof(struct lg4ff_device_entry), GFP_KERNEL);
	if (!entry) {
		hid_err(hid, "Cannot add device, insufficient memory.\n");
		return -ENOMEM;
	}
	entry->min_range = lg4ff_devices[i].min_range;
	entry->max_range = lg4ff_devices[i].max_range;
	entry->set_range = lg4ff_devices[i].set_range;
	drv_data = (struct lg_drv_data *)hid_get_drvdata(hid);
	drv_data->dev_props = entry;

	/* Create sysfs interface for range */
	error = device_create_file(&hid->dev, &dev_attr_range);
	if (error)
		return error;
	dbg_hid("sysfs interface created for range\n");

	/* Set the maximum range to start with */
	entry->range = entry->max_range;
	if (entry->set_range != NULL)
		entry->set_range(hid, entry->range);

	/* register led subsystem - G27 only */
	//if (rev_maj == G27_REV_MAJ && rev_min == G27_REV_MIN) {
	if (rev_maj != G27_REV_MAJ || rev_min != G27_REV_MIN) {
		for (i = 0; i < 5; i++) {
			led = kzalloc(sizeof(struct led_classdev), GFP_KERNEL);
			if (!led) {
				dev_err(dev2, "can't allocate memory for LED %d\n", i);
				error = -ENOMEM;
				goto err;
			}
			led->name = (void *)(g27_leds[i]);
			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;
#if 1
			error = led_classdev_register(dev2, led);
			if (error) {
				dev_err(dev2, "can't register LED %d\n", i);
				goto err;
			}
#endif
		}

		dbg_hid("sysfs interface created for leds\n");

		/* Turn off all leds */
		entry->led_state = 0;
		lg4ff_set_leds(hid, 0);
	}

	hid_info(hid, "Force feedback for Logitech with led subsys\n");
	return 0;

err:
	/* 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);
	}

	return error;
}

int lg4ff_deinit(struct hid_device *hid)
{
	struct lg_drv_data *drv_data = (struct lg_drv_data *)drv_data;
	struct lg4ff_device_entry *uninitialized_var(entry);
	struct led_classdev *led;
	int i;

	if (drv_data) {
		if (drv_data->dev_props) {
			entry = (struct lg4ff_device_entry *)drv_data->dev_props;
			if (entry) {
				/* 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);
				}
			}

			kfree(drv_data->dev_props);
		}
	}

	device_remove_file(&hid->dev, &dev_attr_range);

	dbg_hid("Device successfully unregistered\n");
	return 0;
}

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
@ 2012-03-13 21:08 simon
  2012-03-13 21:11 ` simon
  2012-03-13 21:24 ` Jiri Kosina
  0 siblings, 2 replies; 25+ messages in thread
From: simon @ 2012-03-13 21:08 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: simon, Mark Brown, linux-input, linux-kernel, Michael Bauer, Michal Maly

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

> Not really, it's rather simple to use and as Mark said, it's a proper
infrastructure to use here.
> We already have HID drivers which are using this -- you can look at
PicoLCD or Wiimote drivers and you'll see that it's indeed fairly
simple.

So _you_ say.... I'm getting an Oops when device attempts to init the LED
subsytem. :-(
--
Mar 13 12:54:56 ubuntu kernel: [  458.639674] Call Trace:
Mar 13 12:54:56 ubuntu kernel: [  458.641143]  [<c13498ed>] ?
device_create+0x2d/0x30
Mar 13 12:54:56 ubuntu kernel: [  458.641245]  [<c1439119>]
led_classdev_register+0x99/0xf0
Mar 13 12:54:56 ubuntu kernel: [  458.641306]  [<e0977311>]
lg4ff_init+0x2a1/0x570 [hid_logitech]
Mar 13 12:54:56 ubuntu kernel: [  458.641313]  [<e0975427>]
lg_probe+0x147/0x2c0 [hid_logitech]
--

Code snipet below (full file attached):
--
int lg4ff_init(struct hid_device *hid)
{
    struct hid_input *hidinput = list_entry(hid->inputs.next, struct
hid_input, list);
    struct list_head *report_list =
&hid->report_enum[HID_OUTPUT_REPORT].report_list;
    struct input_dev *dev = hidinput->input;
    struct hid_report *report;
    struct hid_field *field;
    struct lg_drv_data *drv_data;
    struct lg4ff_device_entry *entry;
    struct usb_device_descriptor *udesc;
    int error, i, j;
    __u16 bcdDevice, rev_maj, rev_min;
    struct led_classdev *led;
    struct device *dev2 = &dev->dev;
--
    /* register led subsystem - G27 only */
    //if (rev_maj == G27_REV_MAJ && rev_min == G27_REV_MIN) {
    if (rev_maj != G27_REV_MAJ || rev_min != G27_REV_MIN) {
        for (i = 0; i < 5; i++) {
            led = kzalloc(sizeof(struct led_classdev), GFP_KERNEL); if
(!led) {
                dev_err(dev2, "can't allocate memory for LED %d\n", i);
error = -ENOMEM;
                goto err;
            }
            led->name = (void *)(g27_leds[i]);
            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;
#if 1
            error = led_classdev_register(dev2, entry->led[i]);
            if (error) {
                dev_err(dev2, "can't register LED %d\n", i);
                goto err;
            }
#endif
        }

        dbg_hid("sysfs interface created for leds\n");
--

Any suggestions as to what I'm doing wrong?
Simon


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

/*
 * Force feedback support for hid-compliant for some of the devices from
 * Logitech, namely:
 * - WingMan Cordless RumblePad
 * - WingMan Force 3D
 *
 *  Copyright (c) 2002-2004 Johann Deneux
 *  Copyright (c) 2006 Anssi Hannula <anssi.hannula@gmail.com>
 */

/*
 * 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.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 *
 * Should you need to contact me, the author, you can do so by
 * e-mail - mail your message to <johann.deneux@it.uu.se>
 */

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

#include <linux/input.h>
#include <linux/usb.h>
#include <linux/hid.h>

#include "usbhid/usbhid.h"
#include "hid-lg.h"

struct dev_type {
	u16 idVendor;
	u16 idProduct;
	const signed short *ff;
};

static const signed short ff_rumble[] = {
	FF_RUMBLE,
	-1
};

static const signed short ff_joystick[] = {
	FF_CONSTANT,
	-1
};

static const signed short ff_joystick_ac[] = {
	FF_CONSTANT,
	FF_AUTOCENTER,
	-1
};

static const struct dev_type devices[] = {
	{ 0x046d, 0xc211, ff_rumble },
	{ 0x046d, 0xc219, ff_rumble },
	{ 0x046d, 0xc283, ff_joystick },
	{ 0x046d, 0xc286, ff_joystick_ac },
	{ 0x046d, 0xc287, ff_joystick_ac },
	{ 0x046d, 0xc293, ff_joystick },
	{ 0x046d, 0xc295, ff_joystick },
};

static int hid_lgff_play(struct input_dev *dev, void *data, struct ff_effect *effect)
{
	struct hid_device *hid = input_get_drvdata(dev);
	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);
	int x, y;
	unsigned int left, right;

#define CLAMP(x) if (x < 0) x = 0; if (x > 0xff) x = 0xff

	switch (effect->type) {
	case FF_CONSTANT:
		x = effect->u.ramp.start_level + 0x7f;	/* 0x7f is center */
		y = effect->u.ramp.end_level + 0x7f;
		CLAMP(x);
		CLAMP(y);
		report->field[0]->value[0] = 0x51;
		report->field[0]->value[1] = 0x08;
		report->field[0]->value[2] = x;
		report->field[0]->value[3] = y;
		dbg_hid("(x, y)=(%04x, %04x)\n", x, y);
		usbhid_submit_report(hid, report, USB_DIR_OUT);
		break;

	case FF_RUMBLE:
		right = effect->u.rumble.strong_magnitude;
		left = effect->u.rumble.weak_magnitude;
		right = right * 0xff / 0xffff;
		left = left * 0xff / 0xffff;
		CLAMP(left);
		CLAMP(right);
		report->field[0]->value[0] = 0x42;
		report->field[0]->value[1] = 0x00;
		report->field[0]->value[2] = left;
		report->field[0]->value[3] = right;
		dbg_hid("(left, right)=(%04x, %04x)\n", left, right);
		usbhid_submit_report(hid, report, USB_DIR_OUT);
		break;
	}
	return 0;
}

static void hid_lgff_set_autocenter(struct input_dev *dev, u16 magnitude)
{
	struct hid_device *hid = input_get_drvdata(dev);
	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);
	__s32 *value = report->field[0]->value;
	magnitude = (magnitude >> 12) & 0xf;
	*value++ = 0xfe;
	*value++ = 0x0d;
	*value++ = magnitude;   /* clockwise strength */
	*value++ = magnitude;   /* counter-clockwise strength */
	*value++ = 0x80;
	*value++ = 0x00;
	*value = 0x00;
	usbhid_submit_report(hid, report, USB_DIR_OUT);
}

int lgff_init(struct hid_device* hid)
{
	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
	struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
	struct input_dev *dev = hidinput->input;
	struct hid_report *report;
	struct hid_field *field;
	const signed short *ff_bits = ff_joystick;
	int error;
	int i;

	/* Find the report to use */
	if (list_empty(report_list)) {
		hid_err(hid, "No output report found\n");
		return -1;
	}

	/* Check that the report looks ok */
	report = list_entry(report_list->next, struct hid_report, list);
	field = report->field[0];
	if (!field) {
		hid_err(hid, "NULL field\n");
		return -1;
	}

	for (i = 0; i < ARRAY_SIZE(devices); i++) {
		if (dev->id.vendor == devices[i].idVendor &&
		    dev->id.product == devices[i].idProduct) {
			ff_bits = devices[i].ff;
			break;
		}
	}

	for (i = 0; ff_bits[i] >= 0; i++)
		set_bit(ff_bits[i], dev->ffbit);

	error = input_ff_create_memless(dev, NULL, hid_lgff_play);
	if (error)
		return error;

	if ( test_bit(FF_AUTOCENTER, dev->ffbit) )
		dev->ff->set_autocenter = hid_lgff_set_autocenter;

	pr_info("Force feedback for Logitech force feedback devices by Johann Deneux <johann.deneux@it.uu.se>\n");

	return 0;
}

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-12 19:05           ` simon
  2012-03-12 19:12             ` Mark Brown
@ 2012-03-13  8:23             ` Jiri Kosina
  1 sibling, 0 replies; 25+ messages in thread
From: Jiri Kosina @ 2012-03-13  8:23 UTC (permalink / raw)
  To: simon; +Cc: Mark Brown, linux-input, linux-kernel, Michael Bauer, Michal Maly

On Mon, 12 Mar 2012, simon@mungewell.org wrote:

> >> The LEDs on the G27 are very much more specific, mounted on the yoke of
> >> the gaming wheel.
> >
> >> Would this require the use/complexity of the LED subsystem?
> >
> > Well, it'd mean that applications that know how to drive LEDs will be
> > able to use the LEDs on this wheel with less per device knowledge which
> > seems like a useful thing and if someone wants to remap a LED to "new
> > mail" or whatever that'll be easier (nethack has the mailer daemon
> > deliver a scroll of mail!).
> 
> I'm not familiar with the LED subsystem, but my concerns would be that
> this would add a whole load of code to implement.

Not really, it's rather simple to use and as Mark said, it's a proper 
infrastructure to use here.

We already have HID drivers which are using this -- you can look at 
PicoLCD or Wiimote drivers and you'll see that it's indeed fairly simple.

> PS. I don't really care about email when screaming around Laguna Seca ;-)

:))

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-12 19:05           ` simon
@ 2012-03-12 19:12             ` Mark Brown
  2012-03-13  8:23             ` Jiri Kosina
  1 sibling, 0 replies; 25+ messages in thread
From: Mark Brown @ 2012-03-12 19:12 UTC (permalink / raw)
  To: simon; +Cc: Jiri Kosina, linux-input, linux-kernel, Michael Bauer, Michal Maly

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

On Mon, Mar 12, 2012 at 03:05:39PM -0400, simon@mungewell.org wrote:

> If you can point me at some (simple) implementations of the LED subsystem,
> possibly with some guidance, we can look to see if the added complexity is
> actually worth it.

I don't think any of the LED drivers are massively complex...  you just
register and then you get a callback to set the brightness.  That's
really all there is to it.

> I'd be pretty sure that other gaming controllers don't abstract their LEDs
> via this subsystem,

How sad :(

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-12 18:32         ` Mark Brown
@ 2012-03-12 19:05           ` simon
  2012-03-12 19:12             ` Mark Brown
  2012-03-13  8:23             ` Jiri Kosina
  0 siblings, 2 replies; 25+ messages in thread
From: simon @ 2012-03-12 19:05 UTC (permalink / raw)
  To: Mark Brown
  Cc: simon, Jiri Kosina, linux-input, linux-kernel, Michael Bauer,
	Michal Maly


>> The LEDs on the G27 are very much more specific, mounted on the yoke of
>> the gaming wheel.
>
>> Would this require the use/complexity of the LED subsystem?
>
> Well, it'd mean that applications that know how to drive LEDs will be
> able to use the LEDs on this wheel with less per device knowledge which
> seems like a useful thing and if someone wants to remap a LED to "new
> mail" or whatever that'll be easier (nethack has the mailer daemon
> deliver a scroll of mail!).

I'm not familiar with the LED subsystem, but my concerns would be that
this would add a whole load of code to implement.

Most of concern is the fact that the Logitech wheels are transitioned from
a 'simple' wheel when they are plugged in to an 'advanced' wheel, they
release/re-register with a different USB ID.

The port (for a lack of a better word) for controlling the LEDs is mixed
in with the FF and wheel configuration commands. All this is handled
within the hid-lg4ff driver (if FF is not enabled you just get the
'simple' wheel behaviour).

If you can point me at some (simple) implementations of the LED subsystem,
possibly with some guidance, we can look to see if the added complexity is
actually worth it.

I'd be pretty sure that other gaming controllers don't abstract their LEDs
via this subsystem,
Simon.

PS. I don't really care about email when screaming around Laguna Seca ;-)



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-12 18:19       ` simon
@ 2012-03-12 18:32         ` Mark Brown
  2012-03-12 19:05           ` simon
  0 siblings, 1 reply; 25+ messages in thread
From: Mark Brown @ 2012-03-12 18:32 UTC (permalink / raw)
  To: simon; +Cc: Jiri Kosina, linux-input, linux-kernel, Michael Bauer, Michal Maly

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

On Mon, Mar 12, 2012 at 02:19:25PM -0400, simon@mungewell.org wrote:
> > On Mon, Mar 12, 2012 at 02:04:14PM -0400, simon@mungewell.org wrote:

> >> At the moment you'd just drive the LEDs simply, ie:
> >> $ echo 31 > /sys/bus/hid/devices/.../leds

> > Shouldn't this be using the LED subsystem the kernel already has?

> The LED subsystem appears to be aim at controller chips (ie. lp3944) which
> are connected to a bus (I2C) in a generalised form. I guess people might
> be doing 'blinken light' type systems.

Not really, it's aimed at supporting LEDs in general.  Obviously most of
the drivers are for specific controller chips which makes them more
visible but there's also things like the triggers.

> The LEDs on the G27 are very much more specific, mounted on the yoke of
> the gaming wheel.

> Would this require the use/complexity of the LED subsystem?

Well, it'd mean that applications that know how to drive LEDs will be
able to use the LEDs on this wheel with less per device knowledge which
seems like a useful thing and if someone wants to remap a LED to "new
mail" or whatever that'll be easier (nethack has the mailer daemon
deliver a scroll of mail!).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-12 18:07     ` Mark Brown
@ 2012-03-12 18:19       ` simon
  2012-03-12 18:32         ` Mark Brown
  0 siblings, 1 reply; 25+ messages in thread
From: simon @ 2012-03-12 18:19 UTC (permalink / raw)
  To: Mark Brown
  Cc: simon, Jiri Kosina, linux-input, linux-kernel, Michael Bauer,
	Michal Maly

> On Mon, Mar 12, 2012 at 02:04:14PM -0400, simon@mungewell.org wrote:
>
>> At the moment you'd just drive the LEDs simply, ie:
>> $ echo 31 > /sys/bus/hid/devices/.../leds
>
> Shouldn't this be using the LED subsystem the kernel already has?
>

The LED subsystem appears to be aim at controller chips (ie. lp3944) which
are connected to a bus (I2C) in a generalised form. I guess people might
be doing 'blinken light' type systems.

The LEDs on the G27 are very much more specific, mounted on the yoke of
the gaming wheel.

Would this require the use/complexity of the LED subsystem?
Simon



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-12 18:04   ` simon
@ 2012-03-12 18:07     ` Mark Brown
  2012-03-12 18:19       ` simon
  0 siblings, 1 reply; 25+ messages in thread
From: Mark Brown @ 2012-03-12 18:07 UTC (permalink / raw)
  To: simon; +Cc: Jiri Kosina, linux-input, linux-kernel, Michael Bauer, Michal Maly

On Mon, Mar 12, 2012 at 02:04:14PM -0400, simon@mungewell.org wrote:

> At the moment you'd just drive the LEDs simply, ie:
> $ echo 31 > /sys/bus/hid/devices/.../leds

Shouldn't this be using the LED subsystem the kernel already has?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-12 15:46 ` Jiri Kosina
@ 2012-03-12 18:04   ` simon
  2012-03-12 18:07     ` Mark Brown
  0 siblings, 1 reply; 25+ messages in thread
From: simon @ 2012-03-12 18:04 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Simon Wood, linux-input, linux-kernel, Michael Bauer, Michal Maly

> On Mon, 12 Mar 2012, Simon Wood wrote:
>
>> This patch adds support for the G27 LEDs. The LEDs are controlled
>> by a 5bit value (0-31) where bit0 is the right most LED, the LEDs
>> are mirrored to the left.
>>
>> Arrangement on wheel is:
>>    G G Y Y R R(bit4) Y Y G G(bit0)
>
> Simon,
>
> is there a usespace application operating on top of this?

At present no; I'd like to add this to Speed Dreams (www.speed-dreams.org)
racing simulator in the future, but this will likely take the form of a
3rd party script to read RPM/Redline and drive the sysfs interface
directly as very few user's setups will be alike.

At the moment you'd just drive the LEDs simply, ie:
$ echo 31 > /sys/bus/hid/devices/.../leds

>
> Also, it'd be necessary to document this properly in Documentation/ABI.

OK, I'll add a description to
'Documention/ABI/testing/sysfs-driver-hid-logitech-lg4ff'.

Should this move to 'stable' now as hid-lg4ff is in mainline now?


>> +	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;
>> +	}
>
> Please correct me if I am wrong, but I don't see nothing that'd prevent
> this racing with lg4ff_deinit().
>
> The same applies to already existing sysfs attributes actually.

I believe that Michal is working on some 'spin locking' in this area,
which might meet your requirements. But the patch he sent around last week
(off list) didn't lock on the deinit()...

Simon.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-12 15:17 Simon Wood
  2012-03-12 15:46 ` Jiri Kosina
@ 2012-03-12 16:09 ` Jiri Kosina
  1 sibling, 0 replies; 25+ messages in thread
From: Jiri Kosina @ 2012-03-12 16:09 UTC (permalink / raw)
  To: Simon Wood; +Cc: linux-input, linux-kernel, Michael Bauer, Michal Maly

On Mon, 12 Mar 2012, Simon Wood wrote:

> This patch adds support for the G27 LEDs. The LEDs are controlled
> by a 5bit value (0-31) where bit0 is the right most LED, the LEDs
> are mirrored to the left.
> 
> Arrangement on wheel is:
>    G G Y Y R R(bit4) Y Y G G(bit0)
> 
> Signed-off-by: Simon Wood <simon@mungewell.org>
> ---
>  drivers/hid/hid-lg4ff.c |   84 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
> index 6ecc9e2..4f3b744 100644
> --- a/drivers/hid/hid-lg4ff.c
> +++ b/drivers/hid/hid-lg4ff.c
> @@ -49,7 +49,12 @@ static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range);
>  static ssize_t lg4ff_range_show(struct device *dev, struct device_attribute *attr, char *buf);
>  static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count);
>  
> +static void hid_lg4ff_set_leds(struct hid_device *hid, __u8 leds);
> +static ssize_t lg4ff_leds_show(struct device *dev, struct device_attribute *attr, char *buf);
> +static ssize_t lg4ff_leds_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count);
> +
>  static DEVICE_ATTR(range, S_IRWXU | S_IRWXG | S_IRWXO, lg4ff_range_show, lg4ff_range_store);
> +static DEVICE_ATTR(leds, S_IRWXU | S_IRWXG | S_IRWXO, lg4ff_leds_show, lg4ff_leds_store);
>  
>  static bool list_inited;
>  
> @@ -336,6 +341,67 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
>  	return count;
>  }
>  
> +/* Read current state of leds and display it in terminal */
> +static ssize_t lg4ff_leds_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct lg4ff_device_entry *uninitialized_var(entry);
> +	struct list_head *h;
> +	struct hid_device *hid = to_hid_device(dev);
> +	size_t count;
> +
> +	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;
> +	}

Please correct me if I am wrong, but I don't see nothing that'd prevent 
this racing with lg4ff_deinit().

The same applies to already existing sysfs attributes actually.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
  2012-03-12 15:17 Simon Wood
@ 2012-03-12 15:46 ` Jiri Kosina
  2012-03-12 18:04   ` simon
  2012-03-12 16:09 ` Jiri Kosina
  1 sibling, 1 reply; 25+ messages in thread
From: Jiri Kosina @ 2012-03-12 15:46 UTC (permalink / raw)
  To: Simon Wood; +Cc: linux-input, linux-kernel, Michael Bauer, Michal Maly

On Mon, 12 Mar 2012, Simon Wood wrote:

> This patch adds support for the G27 LEDs. The LEDs are controlled
> by a 5bit value (0-31) where bit0 is the right most LED, the LEDs
> are mirrored to the left.
> 
> Arrangement on wheel is:
>    G G Y Y R R(bit4) Y Y G G(bit0)

Simon,

is there a usespace application operating on top of this?

Also, it'd be necessary to document this properly in Documentation/ABI.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] HID: hid-lg4ff add support for G27 LEDs
@ 2012-03-12 15:17 Simon Wood
  2012-03-12 15:46 ` Jiri Kosina
  2012-03-12 16:09 ` Jiri Kosina
  0 siblings, 2 replies; 25+ messages in thread
From: Simon Wood @ 2012-03-12 15:17 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Jiri Kosina, Michael Bauer, Michal Maly, Simon Wood

This patch adds support for the G27 LEDs. The LEDs are controlled
by a 5bit value (0-31) where bit0 is the right most LED, the LEDs
are mirrored to the left.

Arrangement on wheel is:
   G G Y Y R R(bit4) Y Y G G(bit0)

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-lg4ff.c |   84 +++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 6ecc9e2..4f3b744 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -49,7 +49,12 @@ static void hid_lg4ff_set_range_g25(struct hid_device *hid, u16 range);
 static ssize_t lg4ff_range_show(struct device *dev, struct device_attribute *attr, char *buf);
 static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count);
 
+static void hid_lg4ff_set_leds(struct hid_device *hid, __u8 leds);
+static ssize_t lg4ff_leds_show(struct device *dev, struct device_attribute *attr, char *buf);
+static ssize_t lg4ff_leds_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count);
+
 static DEVICE_ATTR(range, S_IRWXU | S_IRWXG | S_IRWXO, lg4ff_range_show, lg4ff_range_store);
+static DEVICE_ATTR(leds, S_IRWXU | S_IRWXG | S_IRWXO, lg4ff_leds_show, lg4ff_leds_store);
 
 static bool list_inited;
 
@@ -336,6 +341,67 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
 	return count;
 }
 
+/* Read current state of leds and display it in terminal */
+static ssize_t lg4ff_leds_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct lg4ff_device_entry *uninitialized_var(entry);
+	struct list_head *h;
+	struct hid_device *hid = to_hid_device(dev);
+	size_t count;
+
+	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 0;
+	}
+
+	count = scnprintf(buf, PAGE_SIZE, "%u\n", entry->leds);
+	return count;
+}
+
+static ssize_t lg4ff_leds_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct lg4ff_device_entry *uninitialized_var(entry);
+	struct list_head *h;
+	struct hid_device *hid = to_hid_device(dev);
+	__u8 leds = (__u8) simple_strtoul(buf, NULL, 10) & 0x1F; /* values 0 to 31 */
+
+	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 count;
+	}
+
+	/* Set leds to user specified value: 5 wide bit field for leds on right (mirrored to left) */
+	hid_lg4ff_set_leds(hid, leds);
+	entry->leds = leds;
+
+	return count;
+}
+
+static void hid_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);
+}
+
 int lg4ff_init(struct hid_device *hid)
 {
 	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
@@ -446,17 +512,30 @@ int lg4ff_init(struct hid_device *hid)
 	entry->set_range = lg4ff_devices[i].set_range;
 	list_add(&entry->list, &device_list.list);
 
-	/* Create sysfs interface */
+	/* Create sysfs interface for range */
 	error = device_create_file(&hid->dev, &dev_attr_range);
 	if (error)
 		return error;
-	dbg_hid("sysfs interface created\n");
+	dbg_hid("sysfs interface created for range\n");
 
 	/* Set the maximum range to start with */
 	entry->range = entry->max_range;
 	if (entry->set_range != NULL)
 		entry->set_range(hid, entry->range);
 
+	/* Create sysfs interface for leds - G27 only */
+	if (rev_maj == G27_REV_MAJ && rev_min == G27_REV_MIN) {
+		error = device_create_file(&hid->dev, &dev_attr_leds);
+		if (error)
+			return error;
+
+		dbg_hid("sysfs interface created for leds\n");
+
+		/* Turn off all leds */
+		entry->leds = 0;
+		hid_lg4ff_set_leds(hid, 0);
+	}
+
 	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by Simon Wood <simon@mungewell.org>\n");
 	return 0;
 }
@@ -483,6 +562,7 @@ int lg4ff_deinit(struct hid_device *hid)
 	}
 
 	device_remove_file(&hid->dev, &dev_attr_range);
+	device_remove_file(&hid->dev, &dev_attr_leds);
 	dbg_hid("Device successfully unregistered\n");
 	return 0;
 }
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2012-04-23 18:55 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1649210.pcO5IpzBEF@qosmio-x300>
2012-04-09 16:08 ` [PATCH] HID: hid-lg4ff: Add support for G27 LEDs Simon Wood
2012-04-13 12:59   ` Jiri Kosina
2012-04-18  7:03     ` [PATCH 1/2] " Simon Wood
2012-04-18  7:03       ` [PATCH 2/2] HID: hid-lg4ff: Updated Comments Simon Wood
2012-04-18 11:24       ` [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs Jiri Kosina
2012-04-18 14:58         ` simon
2012-04-20 10:05           ` Jiri Kosina
2012-04-21 12:41             ` Simon Wood
2012-04-21 12:41               ` [PATCH 2/2] HID: hid-lg4ff: Updated Comments Simon Wood
2012-04-23 18:55               ` [PATCH 1/2] HID: hid-lg4ff: Add support for G27 LEDs Jiri Kosina
2012-03-13 21:08 [PATCH 1/2] HID: hid-lg4ff add " simon
2012-03-13 21:11 ` simon
2012-03-13 21:24 ` Jiri Kosina
2012-03-13 21:44   ` simon
2012-03-14  1:51   ` simon
  -- strict thread matches above, loose matches on Subject: below --
2012-03-12 15:17 Simon Wood
2012-03-12 15:46 ` Jiri Kosina
2012-03-12 18:04   ` simon
2012-03-12 18:07     ` Mark Brown
2012-03-12 18:19       ` simon
2012-03-12 18:32         ` Mark Brown
2012-03-12 19:05           ` simon
2012-03-12 19:12             ` Mark Brown
2012-03-13  8:23             ` Jiri Kosina
2012-03-12 16:09 ` Jiri Kosina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).