[PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Przemo Firszt]

Add sysfs attribute to control LED selector on Wacom Intuos4. There are 4
different LEDs on the tablet and they can be turned on by something like:

echo 50 > /sys/class/leds/(device # here)\:selector\:1/brightness

Only one can be lit at a time. The brightness range is 0 to 127. This patch
also contains short ABI description.

Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
 Documentation/ABI/testing/sysfs-driver-wacom |    8 ++
 drivers/hid/Kconfig                          |    1 +
 drivers/hid/hid-wacom.c                      |  126 ++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-wacom b/Documentation/ABI/testing/sysfs-driver-wacom
index 0130d66..56c5455 100644
--- a/Documentation/ABI/testing/sysfs-driver-wacom
+++ b/Documentation/ABI/testing/sysfs-driver-wacom
@@ -9,6 +9,14 @@ Description:
 		or 0 otherwise. Writing to this file one of these values
 		switches reporting speed.
 
+What:		/sys/class/leds/0005\:056A\:00BD.0001\:selector\:*/
+Date:		May 2012
+Kernel Version:	3.5
+Contact:	linux-bluetooth@vger.kernel.org
+Description:
+		LED selector for Intuos4 WL. There are 4 leds, but only one LED
+		can be lit at a time. Max brightness is 127.
+
 What:		/sys/bus/usb/devices/<busnum>-<devnum>:<cfg>.<intf>/wacom_led/led
 Date:		August 2011
 Contact:	linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 9092ae1..a9647dd 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -596,6 +596,7 @@ config HID_WACOM
 	tristate "Wacom Bluetooth devices support"
 	depends on BT_HIDP
 	select POWER_SUPPLY
+	select LEDS_CLASS
 	---help---
 	Support for Wacom Graphire Bluetooth and Intuos4 WL tablets.
 
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index a66e1aa..29372ed 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -24,6 +24,7 @@
 #include <linux/device.h>
 #include <linux/hid.h>
 #include <linux/module.h>
+#include <linux/leds.h>
 #include <linux/slab.h>
 #include <linux/power_supply.h>
 
@@ -31,6 +32,8 @@
 
 #define PAD_DEVICE_ID	0x0F
 
+#define WAC_CMD_LED_CONTROL     0x20
+
 struct wacom_data {
 	__u16 tool;
 	__u16 butstate;
@@ -44,6 +47,8 @@ struct wacom_data {
 	__u8 ps_connected;
 	struct power_supply battery;
 	struct power_supply ac;
+	__u8 led_selector;
+	struct led_classdev *leds[4];
 };
 
 /*percent of battery capacity for Graphire
@@ -64,6 +69,117 @@ static enum power_supply_property wacom_ac_props[] = {
 	POWER_SUPPLY_PROP_SCOPE,
 };
 
+static void wacom_leds_set_brightness(struct led_classdev *led_dev,
+						enum led_brightness value)
+{
+	struct device *dev = led_dev->dev->parent;
+	struct hid_device *hdev;
+	struct wacom_data *wdata;
+	unsigned char *buf;
+	__u8 led = 0;
+	int i;
+
+	hdev = container_of(dev, struct hid_device, dev);
+	wdata = hid_get_drvdata(hdev);
+	for (i = 0; i < 4; ++i) {
+		if (wdata->leds[i] == led_dev)
+			wdata->led_selector = i;
+	}
+
+	led = wdata->led_selector | 0x04;
+	buf = kzalloc(9, GFP_KERNEL);
+	if (buf) {
+		buf[0] = WAC_CMD_LED_CONTROL;
+		buf[1] = led;
+		buf[2] = value;
+		hdev->hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
+		kfree(buf);
+	}
+
+	return;
+}
+
+static enum led_brightness wacom_leds_get_brightness(struct led_classdev *led_dev)
+{
+	struct wacom_data *wdata;
+	struct device *dev = led_dev->dev->parent;
+	int value = 0;
+	int i;
+
+	wdata = hid_get_drvdata(container_of(dev, struct hid_device, dev));
+
+	for (i = 0; i < 4; ++i) {
+		if (wdata->leds[i] == led_dev) {
+			value = wdata->leds[i]->brightness;
+			break;
+		}
+	}
+
+	return value;
+}
+
+
+static int wacom_initialize_leds(struct hid_device *hdev)
+{
+	struct wacom_data *wdata = hid_get_drvdata(hdev);
+	struct led_classdev *led;
+	struct device *dev = &hdev->dev;
+	size_t namesz = strlen(dev_name(dev)) + 12;
+	char *name;
+	int i, ret;
+
+	wdata->led_selector = 0;
+
+	for (i = 0; i < 4; i++) {
+		led = kzalloc(sizeof(struct led_classdev) + namesz, GFP_KERNEL);
+		if (!led) {
+			hid_warn(hdev,
+				 "can't allocate memory for LED selector\n");
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		name = (void *)&led[1];
+		snprintf(name, namesz, "%s:selector:%d", dev_name(dev), i);
+		led->name = name;
+		led->brightness = 0;
+		led->max_brightness = 127;
+		led->brightness_get = wacom_leds_get_brightness;
+		led->brightness_set = wacom_leds_set_brightness;
+
+		wdata->leds[i] = led;
+
+		ret = led_classdev_register(dev, wdata->leds[i]);
+
+		if (ret) {
+			wdata->leds[i] = NULL;
+			kfree(led);
+			hid_warn(hdev, "can't register LED\n");
+			goto err;
+		}
+	}
+
+err:
+	return ret;
+}
+
+static void wacom_destroy_leds(struct hid_device *hdev)
+{
+	struct wacom_data *wdata = hid_get_drvdata(hdev);
+	struct led_classdev *led;
+	int i;
+
+	for (i = 0; i < 4; ++i) {
+		if (wdata->leds[i]) {
+			led = wdata->leds[i];
+			wdata->leds[i] = NULL;
+			led_classdev_unregister(led);
+			kfree(led);
+		}
+	}
+
+}
+
 static int wacom_battery_get_property(struct power_supply *psy,
 				enum power_supply_property psp,
 				union power_supply_propval *val)
@@ -602,6 +718,12 @@ static int wacom_probe(struct hid_device *hdev,
 		sprintf(hdev->name, "%s", "Wacom Intuos4 WL");
 		wdata->features = 0;
 		wacom_set_features(hdev);
+		ret = wacom_initialize_leds(hdev);
+		if (ret) {
+			hid_warn(hdev,
+				 "can't create led attribute, err: %d\n", ret);
+			goto destroy_leds;
+		}
 		break;
 	}
 
@@ -644,6 +766,8 @@ err_ac:
 err_battery:
 	device_remove_file(&hdev->dev, &dev_attr_speed);
 	hid_hw_stop(hdev);
+destroy_leds:
+	wacom_destroy_leds(hdev);
 err_free:
 	kfree(wdata);
 	return ret;
@@ -652,6 +776,8 @@ err_free:
 static void wacom_remove(struct hid_device *hdev)
 {
 	struct wacom_data *wdata = hid_get_drvdata(hdev);
+
+	wacom_destroy_leds(hdev);
 	device_remove_file(&hdev->dev, &dev_attr_speed);
 	hid_hw_stop(hdev);
 
-- 
1.7.10

Re: [PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Jiri Kosina]

On Thu, 10 May 2012, Przemo Firszt wrote:

> Add sysfs attribute to control LED selector on Wacom Intuos4. 

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[David Rientjes]

On Fri, 11 May 2012, Jiri Kosina wrote:

> > Add sysfs attribute to control LED selector on Wacom Intuos4. 
> 
> Applied, thanks.
> 

This results in a build failure if you've enabled CONFIG_HID_WACOM without 
CONFIG_NEW_LEDS, which is required for CONFIG_LEDS_CLASS.

drivers/built-in.o: In function `led_classdev_unregister':
(.text+0x3ff685): undefined reference to `led_brightness_set'
drivers/built-in.o: In function `led_classdev_unregister':
(.text+0x3ff695): undefined reference to `leds_list_lock'
drivers/built-in.o: In function `led_classdev_unregister':
(.text+0x3ff6ab): undefined reference to `leds_list_lock'
drivers/built-in.o: In function `led_classdev_register':
(.text+0x3ff6e9): undefined reference to `leds_list_lock'
drivers/built-in.o: In function `led_classdev_register':
(.text+0x3ff6f5): undefined reference to `leds_list'
drivers/built-in.o: In function `led_classdev_register':
(.text+0x3ff700): undefined reference to `leds_list'
drivers/built-in.o: In function `led_classdev_register':
(.text+0x3ff70c): undefined reference to `leds_list_lock'

Please apply the patch below.


hid, wacom: fix build breakage without CONFIG_LEDS_CLASS

CONFIG_HID_WACOM must depend on CONFIG_LEDS_CLASS, otherwise 
CONFIG_NEW_LEDS may be disabled.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 drivers/hid/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -613,8 +613,8 @@ config THRUSTMASTER_FF
 config HID_WACOM
 	tristate "Wacom Bluetooth devices support"
 	depends on BT_HIDP
+	depends on LEDS_CLASS
 	select POWER_SUPPLY
-	select LEDS_CLASS
 	---help---
 	Support for Wacom Graphire Bluetooth and Intuos4 WL tablets.
 

Re: [PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Jiri Kosina]

On Sat, 19 May 2012, David Rientjes wrote:

> On Fri, 11 May 2012, Jiri Kosina wrote:
> 
> > > Add sysfs attribute to control LED selector on Wacom Intuos4. 
> > 
> > Applied, thanks.
> > 
> 
> This results in a build failure if you've enabled CONFIG_HID_WACOM without 
> CONFIG_NEW_LEDS, which is required for CONFIG_LEDS_CLASS.
> 
> drivers/built-in.o: In function `led_classdev_unregister':
> (.text+0x3ff685): undefined reference to `led_brightness_set'
> drivers/built-in.o: In function `led_classdev_unregister':
> (.text+0x3ff695): undefined reference to `leds_list_lock'
> drivers/built-in.o: In function `led_classdev_unregister':
> (.text+0x3ff6ab): undefined reference to `leds_list_lock'
> drivers/built-in.o: In function `led_classdev_register':
> (.text+0x3ff6e9): undefined reference to `leds_list_lock'
> drivers/built-in.o: In function `led_classdev_register':
> (.text+0x3ff6f5): undefined reference to `leds_list'
> drivers/built-in.o: In function `led_classdev_register':
> (.text+0x3ff700): undefined reference to `leds_list'
> drivers/built-in.o: In function `led_classdev_register':
> (.text+0x3ff70c): undefined reference to `leds_list_lock'
> 
> Please apply the patch below.
> 
> 
> hid, wacom: fix build breakage without CONFIG_LEDS_CLASS
> 
> CONFIG_HID_WACOM must depend on CONFIG_LEDS_CLASS, otherwise 
> CONFIG_NEW_LEDS may be disabled.
> 
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  drivers/hid/Kconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -613,8 +613,8 @@ config THRUSTMASTER_FF
>  config HID_WACOM
>  	tristate "Wacom Bluetooth devices support"
>  	depends on BT_HIDP
> +	depends on LEDS_CLASS
>  	select POWER_SUPPLY
> -	select LEDS_CLASS
>  	---help---
>  	Support for Wacom Graphire Bluetooth and Intuos4 WL tablets.

Right, thanks for your report. Now queued.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Przemo Firszt]

Dnia 20 Maja 2012, 3:11 am, N, David Rientjes napisał(a):
> On Fri, 11 May 2012, Jiri Kosina wrote:
>
>> > Add sysfs attribute to control LED selector on Wacom Intuos4.
>>
>> Applied, thanks.
>>
>
> This results in a build failure if you've enabled CONFIG_HID_WACOM without
> CONFIG_NEW_LEDS, which is required for CONFIG_LEDS_CLASS.
>
[..]
Thanks for catching that!
-- 
Regards,
Przemo Firszt

Re: [PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Przemo Firszt]

Dnia 20 Maja 2012, 3:11 am, N, David Rientjes napisał(a):
> On Fri, 11 May 2012, Jiri Kosina wrote:
>
>> > Add sysfs attribute to control LED selector on Wacom Intuos4.
>>
>> Applied, thanks.
>>
>
> This results in a build failure if you've enabled CONFIG_HID_WACOM without
> CONFIG_NEW_LEDS, which is required for CONFIG_LEDS_CLASS.
>
[..]
Thanks for catching that!
-- 
Regards,
Przemo Firszt

--
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

Re: [PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Przemo Firszt]

Dnia 2012-05-10, czw o godzinie 12:09 +0200, Jiri Kosina pisze:
> On Mon, 7 May 2012, Przemo Firszt wrote:
> 
> > Add sysfs attribute to control LED selector on Wacom Intuos4. There are 4
[..]
> > +
> > +	for (i = 0; i < 4; ++i) {
> 
> And here as well.
> 
> Otherwise the patch looks fine, and once you fix this, I will apply it. 
> Thanks,
> 
Hi Jiri,
Thanks for catching it! New patch is on the way...
-- 
regards,
Przemo Firszt <przemo@firszt.eu>

Re: [PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Jiri Kosina]

On Mon, 7 May 2012, Przemo Firszt wrote:

> Add sysfs attribute to control LED selector on Wacom Intuos4. There are 4
> different LEDs on the tablet and they can be turned on by something like:
> 
> echo 50 > /sys/class/leds/(device # here)\:selector\:1/brightness
> 
> Only one can be lit at a time. The brightness range is 0 to 127. This patch
> also contains short ABI description.
> 
> Signed-off-by: Przemo Firszt <przemo@firszt.eu>
[ ... snip ... ]
> @@ -44,6 +47,8 @@ struct wacom_data {
>  	__u8 ps_connected;
>  	struct power_supply battery;
>  	struct power_supply ac;
> +	__u8 led_selector;
> +	struct led_classdev *leds[3];

Here you define the leds[] array to have 3 members.

>  /*percent of battery capacity for Graphire
> @@ -64,6 +69,117 @@ static enum power_supply_property wacom_ac_props[] = {
>  	POWER_SUPPLY_PROP_SCOPE,
>  };
>  
> +static void wacom_leds_set_brightness(struct led_classdev *led_dev,
> +						enum led_brightness value)
> +{
> +	struct device *dev = led_dev->dev->parent;
> +	struct hid_device *hdev;
> +	struct wacom_data *wdata;
> +	unsigned char *buf;
> +	__u8 led = 0;
> +	int i;
> +
> +	hdev = container_of(dev, struct hid_device, dev);
> +	wdata = hid_get_drvdata(hdev);
> +	for (i = 0; i < 4; ++i) {

And here you are going off-by-one.

> +		if (wdata->leds[i] == led_dev)
> +			wdata->led_selector = i;
> +	}
> +
> +	led = wdata->led_selector | 0x04;
> +	buf = kzalloc(9, GFP_KERNEL);
> +	if (buf) {
> +		buf[0] = WAC_CMD_LED_CONTROL;
> +		buf[1] = led;
> +		buf[2] = value;
> +		hdev->hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
> +		kfree(buf);
> +	}
> +
> +	return;
> +}
> +
> +static enum led_brightness wacom_leds_get_brightness(struct led_classdev *led_dev)
> +{
> +	struct wacom_data *wdata;
> +	struct device *dev = led_dev->dev->parent;
> +	int value = 0;
> +	int i;
> +
> +	wdata = hid_get_drvdata(container_of(dev, struct hid_device, dev));
> +
> +	for (i = 0; i < 4; ++i) {

And here as well.

Otherwise the patch looks fine, and once you fix this, I will apply it. 
Thanks,

-- 
Jiri Kosina
SUSE Labs

[PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Przemo Firszt]

Add sysfs attribute to control LED selector on Wacom Intuos4. There are 4
different LEDs on the tablet and they can be turned on by something like:

echo 50 > /sys/class/leds/(device # here)\:selector\:1/brightness

Only one can be lit at a time. The brightness range is 0 to 127. This patch
also contains short ABI description.

Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
 Documentation/ABI/testing/sysfs-driver-wacom |    8 ++
 drivers/hid/Kconfig                          |    1 +
 drivers/hid/hid-wacom.c                      |  126 ++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-wacom b/Documentation/ABI/testing/sysfs-driver-wacom
index 0130d66..56c5455 100644
--- a/Documentation/ABI/testing/sysfs-driver-wacom
+++ b/Documentation/ABI/testing/sysfs-driver-wacom
@@ -9,6 +9,14 @@ Description:
 		or 0 otherwise. Writing to this file one of these values
 		switches reporting speed.
 
+What:		/sys/class/leds/0005\:056A\:00BD.0001\:selector\:*/
+Date:		May 2012
+Kernel Version:	3.5
+Contact:	linux-bluetooth@vger.kernel.org
+Description:
+		LED selector for Intuos4 WL. There are 4 leds, but only one LED
+		can be lit at a time. Max brightness is 127.
+
 What:		/sys/bus/usb/devices/<busnum>-<devnum>:<cfg>.<intf>/wacom_led/led
 Date:		August 2011
 Contact:	linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 9092ae1..a9647dd 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -596,6 +596,7 @@ config HID_WACOM
 	tristate "Wacom Bluetooth devices support"
 	depends on BT_HIDP
 	select POWER_SUPPLY
+	select LEDS_CLASS
 	---help---
 	Support for Wacom Graphire Bluetooth and Intuos4 WL tablets.
 
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index a66e1aa..336ddc8 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -24,6 +24,7 @@
 #include <linux/device.h>
 #include <linux/hid.h>
 #include <linux/module.h>
+#include <linux/leds.h>
 #include <linux/slab.h>
 #include <linux/power_supply.h>
 
@@ -31,6 +32,8 @@
 
 #define PAD_DEVICE_ID	0x0F
 
+#define WAC_CMD_LED_CONTROL     0x20
+
 struct wacom_data {
 	__u16 tool;
 	__u16 butstate;
@@ -44,6 +47,8 @@ struct wacom_data {
 	__u8 ps_connected;
 	struct power_supply battery;
 	struct power_supply ac;
+	__u8 led_selector;
+	struct led_classdev *leds[3];
 };
 
 /*percent of battery capacity for Graphire
@@ -64,6 +69,117 @@ static enum power_supply_property wacom_ac_props[] = {
 	POWER_SUPPLY_PROP_SCOPE,
 };
 
+static void wacom_leds_set_brightness(struct led_classdev *led_dev,
+						enum led_brightness value)
+{
+	struct device *dev = led_dev->dev->parent;
+	struct hid_device *hdev;
+	struct wacom_data *wdata;
+	unsigned char *buf;
+	__u8 led = 0;
+	int i;
+
+	hdev = container_of(dev, struct hid_device, dev);
+	wdata = hid_get_drvdata(hdev);
+	for (i = 0; i < 4; ++i) {
+		if (wdata->leds[i] == led_dev)
+			wdata->led_selector = i;
+	}
+
+	led = wdata->led_selector | 0x04;
+	buf = kzalloc(9, GFP_KERNEL);
+	if (buf) {
+		buf[0] = WAC_CMD_LED_CONTROL;
+		buf[1] = led;
+		buf[2] = value;
+		hdev->hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
+		kfree(buf);
+	}
+
+	return;
+}
+
+static enum led_brightness wacom_leds_get_brightness(struct led_classdev *led_dev)
+{
+	struct wacom_data *wdata;
+	struct device *dev = led_dev->dev->parent;
+	int value = 0;
+	int i;
+
+	wdata = hid_get_drvdata(container_of(dev, struct hid_device, dev));
+
+	for (i = 0; i < 4; ++i) {
+		if (wdata->leds[i] == led_dev) {
+			value = wdata->leds[i]->brightness;
+			break;
+		}
+	}
+
+	return value;
+}
+
+
+static int wacom_initialize_leds(struct hid_device *hdev)
+{
+	struct wacom_data *wdata = hid_get_drvdata(hdev);
+	struct led_classdev *led;
+	struct device *dev = &hdev->dev;
+	size_t namesz = strlen(dev_name(dev)) + 12;
+	char *name;
+	int i, ret;
+
+	wdata->led_selector = 0;
+
+	for (i = 0; i < 4; i++) {
+		led = kzalloc(sizeof(struct led_classdev) + namesz, GFP_KERNEL);
+		if (!led) {
+			hid_warn(hdev,
+				 "can't allocate memory for LED selector\n");
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		name = (void *)&led[1];
+		snprintf(name, namesz, "%s:selector:%d", dev_name(dev), i);
+		led->name = name;
+		led->brightness = 0;
+		led->max_brightness = 127;
+		led->brightness_get = wacom_leds_get_brightness;
+		led->brightness_set = wacom_leds_set_brightness;
+
+		wdata->leds[i] = led;
+
+		ret = led_classdev_register(dev, wdata->leds[i]);
+
+		if (ret) {
+			wdata->leds[i] = NULL;
+			kfree(led);
+			hid_warn(hdev, "can't register LED\n");
+			goto err;
+		}
+	}
+
+err:
+	return ret;
+}
+
+static void wacom_destroy_leds(struct hid_device *hdev)
+{
+	struct wacom_data *wdata = hid_get_drvdata(hdev);
+	struct led_classdev *led;
+	int i;
+
+	for (i = 0; i < 4; ++i) {
+		if (wdata->leds[i]) {
+			led = wdata->leds[i];
+			wdata->leds[i] = NULL;
+			led_classdev_unregister(led);
+			kfree(led);
+		}
+	}
+
+}
+
 static int wacom_battery_get_property(struct power_supply *psy,
 				enum power_supply_property psp,
 				union power_supply_propval *val)
@@ -602,6 +718,12 @@ static int wacom_probe(struct hid_device *hdev,
 		sprintf(hdev->name, "%s", "Wacom Intuos4 WL");
 		wdata->features = 0;
 		wacom_set_features(hdev);
+		ret = wacom_initialize_leds(hdev);
+		if (ret) {
+			hid_warn(hdev,
+				 "can't create led attribute, err: %d\n", ret);
+			goto destroy_leds;
+		}
 		break;
 	}
 
@@ -644,6 +766,8 @@ err_ac:
 err_battery:
 	device_remove_file(&hdev->dev, &dev_attr_speed);
 	hid_hw_stop(hdev);
+destroy_leds:
+	wacom_destroy_leds(hdev);
 err_free:
 	kfree(wdata);
 	return ret;
@@ -652,6 +776,8 @@ err_free:
 static void wacom_remove(struct hid_device *hdev)
 {
 	struct wacom_data *wdata = hid_get_drvdata(hdev);
+
+	wacom_destroy_leds(hdev);
 	device_remove_file(&hdev->dev, &dev_attr_speed);
 	hid_hw_stop(hdev);
 
-- 
1.7.10

Re: [PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Przemo Firszt]

Dnia 2012-05-07, pon o godzinie 15:09 +0200, Jiri Kosina pisze:
> On Mon, 7 May 2012, Przemo Firszt wrote:
[..]
> Przemo,
> 
> is there a particular reason for not using LEDS_CLASS for this, please?
Hi Jiri,
No, I just used the same approach as wacom USB driver. I'll change that.
Thanks!
-- 
Przemo Firszt <przemo@firszt.eu>

Re: [PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Jiri Kosina]

On Mon, 7 May 2012, Przemo Firszt wrote:

> Add sysfs attribute to control LED selector on Wacom Intuos4. There are 4
> different LEDs on the tablet and they can be turned on by something like:
> 
> echo 1 > /sys/class/bluetooth/hci0:1/(dev no here)/wacom_led/status_led0_select
> 
> The status_led0_select range is 0 to 3. The brightness of the LED selector can
> be controlled as well, but this patch uses a fixed value (0x0F) and the
> selector is also permanently switched on. The naming of the attribute
> (wacom_led/status_led0_select) is the same is in USB driver for Intuos4 Wl.
> 
> Signed-off-by: Przemo Firszt <przemo@firszt.eu>
> ---
>  drivers/hid/hid-wacom.c |   93 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
> index a66e1aa..c5c6ec6 100644
> --- a/drivers/hid/hid-wacom.c
> +++ b/drivers/hid/hid-wacom.c
> @@ -31,6 +31,8 @@
>  
>  #define PAD_DEVICE_ID	0x0F
>  
> +#define WAC_CMD_LED_CONTROL     0x20
> +
>  struct wacom_data {
>  	__u16 tool;
>  	__u16 butstate;
> @@ -44,6 +46,7 @@ struct wacom_data {
>  	__u8 ps_connected;
>  	struct power_supply battery;
>  	struct power_supply ac;
> +	__u8 led_selector;
>  };
>  
>  /*percent of battery capacity for Graphire
> @@ -64,6 +67,88 @@ static enum power_supply_property wacom_ac_props[] = {
>  	POWER_SUPPLY_PROP_SCOPE,
>  };
>  
> +static void wacom_led_control(struct hid_device *hdev)
> +{
> +	struct wacom_data *wdata = hid_get_drvdata(hdev);
> +	unsigned char *buf;
> +		__u8 led = 0;
> +
> +	buf = kzalloc(9, GFP_KERNEL);
> +	if (buf) {
> +		led =  wdata->led_selector | 0x4;
> +
> +		buf[0] = WAC_CMD_LED_CONTROL;
> +		buf[1] = led;
> +		buf[2] = 0x0F;
> +		hdev->hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
> +
> +		kfree(buf);
> +	}
> +
> +	return;
> +}
> +
> +static ssize_t wacom_led0_select_store(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> +	struct wacom_data *wdata = hid_get_drvdata(hdev);
> +	int new_led;
> +
> +	if (sscanf(buf, "%1d", &new_led) != 1)
> +		return -EINVAL;
> +
> +	wdata->led_selector = new_led & 0x03;
> +
> +	wacom_led_control(hdev);
> +	return strnlen(buf, PAGE_SIZE);
> +}
> +
> +static ssize_t wacom_led0_select_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> +	struct wacom_data *wdata = hid_get_drvdata(hdev);
> +
> +	return snprintf(buf, 2, "%d\n", wdata->led_selector);
> +}
> +
> +static DEVICE_ATTR(status_led0_select, S_IWUSR | S_IRUSR,
> +		    wacom_led0_select_show, wacom_led0_select_store);
> +
> +static struct attribute *intuos4_led_attrs[] = {
> +	&dev_attr_status_led0_select.attr,
> +	NULL
> +};
> +
> +static struct attribute_group intuos4_led_attr_group = {
> +	.name = "wacom_led",
> +	.attrs = intuos4_led_attrs,
> +};
> +
> +static int wacom_initialize_leds(struct hid_device *hdev)
> +{
> +	struct wacom_data *wdata = hid_get_drvdata(hdev);
> +	int ret;
> +
> +	wdata->led_selector = 0;
> +	ret = sysfs_create_group(&hdev->dev.kobj, &intuos4_led_attr_group);
> +
> +	if (ret) {
> +		dev_err(&hdev->dev,
> +			"cannot create sysfs group err: %d\n", ret);
> +		return ret;
> +	}
> +	wacom_led_control(hdev);
> +
> +	return 0;
> +}
> +
> +static void wacom_destroy_leds(struct hid_device *hdev)
> +{
> +	sysfs_remove_group(&hdev->dev.kobj, &intuos4_led_attr_group);
> +}
> +
>  static int wacom_battery_get_property(struct power_supply *psy,
>  				enum power_supply_property psp,
>  				union power_supply_propval *val)
> @@ -602,6 +687,12 @@ static int wacom_probe(struct hid_device *hdev,
>  		sprintf(hdev->name, "%s", "Wacom Intuos4 WL");
>  		wdata->features = 0;
>  		wacom_set_features(hdev);
> +		ret = wacom_initialize_leds(hdev);
> +		if (ret) {
> +			hid_warn(hdev,
> +				 "can't create led attribute, err: %d\n", ret);
> +			goto destroy_leds;
> +		}
>  		break;
>  	}
>  
> @@ -644,6 +735,8 @@ err_ac:
>  err_battery:
>  	device_remove_file(&hdev->dev, &dev_attr_speed);
>  	hid_hw_stop(hdev);
> +destroy_leds:
> +	wacom_destroy_leds(hdev);
>  err_free:
>  	kfree(wdata);
>  	return ret;

Przemo,

is there a particular reason for not using LEDS_CLASS for this, please?

-- 
Jiri Kosina
SUSE Labs

[PATCH] HID: wacom: Add LED selector control for Wacom Intuos4 WL

[Przemo Firszt]

Add sysfs attribute to control LED selector on Wacom Intuos4. There are 4
different LEDs on the tablet and they can be turned on by something like:

echo 1 > /sys/class/bluetooth/hci0:1/(dev no here)/wacom_led/status_led0_select

The status_led0_select range is 0 to 3. The brightness of the LED selector can
be controlled as well, but this patch uses a fixed value (0x0F) and the
selector is also permanently switched on. The naming of the attribute
(wacom_led/status_led0_select) is the same is in USB driver for Intuos4 Wl.

Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
 drivers/hid/hid-wacom.c |   93 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index a66e1aa..c5c6ec6 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -31,6 +31,8 @@
 
 #define PAD_DEVICE_ID	0x0F
 
+#define WAC_CMD_LED_CONTROL     0x20
+
 struct wacom_data {
 	__u16 tool;
 	__u16 butstate;
@@ -44,6 +46,7 @@ struct wacom_data {
 	__u8 ps_connected;
 	struct power_supply battery;
 	struct power_supply ac;
+	__u8 led_selector;
 };
 
 /*percent of battery capacity for Graphire
@@ -64,6 +67,88 @@ static enum power_supply_property wacom_ac_props[] = {
 	POWER_SUPPLY_PROP_SCOPE,
 };
 
+static void wacom_led_control(struct hid_device *hdev)
+{
+	struct wacom_data *wdata = hid_get_drvdata(hdev);
+	unsigned char *buf;
+		__u8 led = 0;
+
+	buf = kzalloc(9, GFP_KERNEL);
+	if (buf) {
+		led =  wdata->led_selector | 0x4;
+
+		buf[0] = WAC_CMD_LED_CONTROL;
+		buf[1] = led;
+		buf[2] = 0x0F;
+		hdev->hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
+
+		kfree(buf);
+	}
+
+	return;
+}
+
+static ssize_t wacom_led0_select_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+	struct wacom_data *wdata = hid_get_drvdata(hdev);
+	int new_led;
+
+	if (sscanf(buf, "%1d", &new_led) != 1)
+		return -EINVAL;
+
+	wdata->led_selector = new_led & 0x03;
+
+	wacom_led_control(hdev);
+	return strnlen(buf, PAGE_SIZE);
+}
+
+static ssize_t wacom_led0_select_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+	struct wacom_data *wdata = hid_get_drvdata(hdev);
+
+	return snprintf(buf, 2, "%d\n", wdata->led_selector);
+}
+
+static DEVICE_ATTR(status_led0_select, S_IWUSR | S_IRUSR,
+		    wacom_led0_select_show, wacom_led0_select_store);
+
+static struct attribute *intuos4_led_attrs[] = {
+	&dev_attr_status_led0_select.attr,
+	NULL
+};
+
+static struct attribute_group intuos4_led_attr_group = {
+	.name = "wacom_led",
+	.attrs = intuos4_led_attrs,
+};
+
+static int wacom_initialize_leds(struct hid_device *hdev)
+{
+	struct wacom_data *wdata = hid_get_drvdata(hdev);
+	int ret;
+
+	wdata->led_selector = 0;
+	ret = sysfs_create_group(&hdev->dev.kobj, &intuos4_led_attr_group);
+
+	if (ret) {
+		dev_err(&hdev->dev,
+			"cannot create sysfs group err: %d\n", ret);
+		return ret;
+	}
+	wacom_led_control(hdev);
+
+	return 0;
+}
+
+static void wacom_destroy_leds(struct hid_device *hdev)
+{
+	sysfs_remove_group(&hdev->dev.kobj, &intuos4_led_attr_group);
+}
+
 static int wacom_battery_get_property(struct power_supply *psy,
 				enum power_supply_property psp,
 				union power_supply_propval *val)
@@ -602,6 +687,12 @@ static int wacom_probe(struct hid_device *hdev,
 		sprintf(hdev->name, "%s", "Wacom Intuos4 WL");
 		wdata->features = 0;
 		wacom_set_features(hdev);
+		ret = wacom_initialize_leds(hdev);
+		if (ret) {
+			hid_warn(hdev,
+				 "can't create led attribute, err: %d\n", ret);
+			goto destroy_leds;
+		}
 		break;
 	}
 
@@ -644,6 +735,8 @@ err_ac:
 err_battery:
 	device_remove_file(&hdev->dev, &dev_attr_speed);
 	hid_hw_stop(hdev);
+destroy_leds:
+	wacom_destroy_leds(hdev);
 err_free:
 	kfree(wdata);
 	return ret;
-- 
1.7.10