linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Jiri Kosina <jikos@kernel.org>, Ping Cheng <pinglinux@gmail.com>,
	Jason Gerecke <killertofu@gmail.com>,
	Aaron Skomra <skomra@gmail.com>,
	Peter Hutterer <peter.hutterer@who-t.net>
Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org
Subject: [PATCH v2 13/30] HID: wacom: leds: dynamically allocate LED groups
Date: Wed, 13 Jul 2016 18:06:00 +0200	[thread overview]
Message-ID: <1468425977-4819-14-git-send-email-benjamin.tissoires@redhat.com> (raw)
In-Reply-To: <1468425977-4819-1-git-send-email-benjamin.tissoires@redhat.com>

We need to add an action to ensure wacom->led.groups is null when
wacom_led_control() gets called after the resources has been freed.

This also prevents to send a LED command when there is no support
from the device.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

No changes in v2
---
 drivers/hid/wacom.h     |  8 +++--
 drivers/hid/wacom_sys.c | 85 +++++++++++++++++++++++++++++++++++++++++--------
 drivers/hid/wacom_wac.c |  2 +-
 3 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index bcfeb51..8ac1eb8 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -110,6 +110,10 @@ enum wacom_worker {
 	WACOM_WORKER_BATTERY,
 };
 
+struct wacom_group_leds {
+	u8 select; /* status led selector (0..3) */
+};
+
 struct wacom {
 	struct usb_device *usbdev;
 	struct usb_interface *intf;
@@ -118,8 +122,8 @@ struct wacom {
 	struct mutex lock;
 	struct work_struct wireless_work;
 	struct work_struct battery_work;
-	struct wacom_led {
-		u8 select[5]; /* status led selector (0..3) */
+	struct wacom_leds {
+		struct wacom_group_leds *groups;
 		u8 llv;       /* status led brightness no button (1..127) */
 		u8 hlv;       /* status led brightness button pressed (1..127) */
 		u8 img_lum;   /* OLED matrix display brightness */
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index d0f5764..4d4c737 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -647,6 +647,9 @@ static int wacom_led_control(struct wacom *wacom)
 	unsigned char report_id = WAC_CMD_LED_CONTROL;
 	int buf_size = 9;
 
+	if (!wacom->led.groups)
+		return -ENOTSUPP;
+
 	if (wacom->wacom_wac.pid) { /* wireless connected */
 		report_id = WAC_CMD_WL_LED_CONTROL;
 		buf_size = 13;
@@ -662,7 +665,7 @@ static int wacom_led_control(struct wacom *wacom)
 		 * one of four values:
 		 *    0 = Low; 1 = Medium; 2 = High; 3 = Off
 		 */
-		int ring_led = wacom->led.select[0] & 0x03;
+		int ring_led = wacom->led.groups[0].select & 0x03;
 		int ring_lum = (((wacom->led.llv & 0x60) >> 5) - 1) & 0x03;
 		int crop_lum = 0;
 		unsigned char led_bits = (crop_lum << 4) | (ring_lum << 2) | (ring_led);
@@ -677,11 +680,11 @@ static int wacom_led_control(struct wacom *wacom)
 			buf[1] = led_bits;
 	}
 	else {
-		int led = wacom->led.select[0] | 0x4;
+		int led = wacom->led.groups[0].select | 0x4;
 
 		if (wacom->wacom_wac.features.type == WACOM_21UX2 ||
 		    wacom->wacom_wac.features.type == WACOM_24HD)
-			led |= (wacom->led.select[1] << 4) | 0x40;
+			led |= (wacom->led.groups[1].select << 4) | 0x40;
 
 		buf[0] = report_id;
 		buf[1] = led;
@@ -753,7 +756,7 @@ static ssize_t wacom_led_select_store(struct device *dev, int set_id,
 
 	mutex_lock(&wacom->lock);
 
-	wacom->led.select[set_id] = id & 0x3;
+	wacom->led.groups[set_id].select = id & 0x3;
 	err = wacom_led_control(wacom);
 
 	mutex_unlock(&wacom->lock);
@@ -773,7 +776,7 @@ static ssize_t wacom_led##SET_ID##_select_show(struct device *dev,	\
 	struct hid_device *hdev = to_hid_device(dev);\
 	struct wacom *wacom = hid_get_drvdata(hdev);			\
 	return scnprintf(buf, PAGE_SIZE, "%d\n",			\
-			 wacom->led.select[SET_ID]);			\
+			 wacom->led.groups[SET_ID].select);		\
 }									\
 static DEVICE_ATTR(status_led##SET_ID##_select, DEV_ATTR_RW_PERM,	\
 		    wacom_led##SET_ID##_select_show,			\
@@ -955,6 +958,35 @@ static int wacom_devm_sysfs_create_group(struct wacom *wacom,
 	return 0;
 }
 
+static void wacom_led_groups_release(void *data)
+{
+	struct wacom *wacom = data;
+
+	wacom->led.groups = NULL;
+}
+
+static int wacom_led_groups_allocate(struct wacom *wacom, int count)
+{
+	struct wacom_group_leds *groups;
+	int error;
+
+	groups = devm_kzalloc(&wacom->hdev->dev,
+			      sizeof(struct wacom_group_leds) * count,
+			      GFP_KERNEL);
+	if (!groups)
+		return -ENOMEM;
+
+	error = devm_add_action_or_reset(&wacom->hdev->dev,
+					 wacom_led_groups_release,
+					 wacom);
+	if (error)
+		return error;
+
+	wacom->led.groups = groups;
+
+	return 0;
+}
+
 static int wacom_initialize_leds(struct wacom *wacom)
 {
 	int error;
@@ -968,23 +1000,34 @@ static int wacom_initialize_leds(struct wacom *wacom)
 	case INTUOS4:
 	case INTUOS4WL:
 	case INTUOS4L:
-		wacom->led.select[0] = 0;
-		wacom->led.select[1] = 0;
 		wacom->led.llv = 10;
 		wacom->led.hlv = 20;
 		wacom->led.img_lum = 10;
+
+		error = wacom_led_groups_allocate(wacom, 1);
+		if (error) {
+			hid_err(wacom->hdev,
+				"cannot create leds err: %d\n", error);
+			return error;
+		}
+
 		error = wacom_devm_sysfs_create_group(wacom,
 						      &intuos4_led_attr_group);
 		break;
 
 	case WACOM_24HD:
 	case WACOM_21UX2:
-		wacom->led.select[0] = 0;
-		wacom->led.select[1] = 0;
 		wacom->led.llv = 0;
 		wacom->led.hlv = 0;
 		wacom->led.img_lum = 0;
 
+		error = wacom_led_groups_allocate(wacom, 2);
+		if (error) {
+			hid_err(wacom->hdev,
+				"cannot create leds err: %d\n", error);
+			return error;
+		}
+
 		error = wacom_devm_sysfs_create_group(wacom,
 						      &cintiq_led_attr_group);
 		break;
@@ -995,16 +1038,30 @@ static int wacom_initialize_leds(struct wacom *wacom)
 	case INTUOSPS:
 	case INTUOSPM:
 	case INTUOSPL:
-		wacom->led.select[0] = 0;
-		wacom->led.select[1] = 0;
 		wacom->led.llv = 32;
 		wacom->led.hlv = 0;
 		wacom->led.img_lum = 0;
 
+		error = wacom_led_groups_allocate(wacom, 1);
+		if (error) {
+			hid_err(wacom->hdev,
+				"cannot create leds err: %d\n", error);
+			return error;
+		}
+
 		error = wacom_devm_sysfs_create_group(wacom,
 						      &intuos5_led_attr_group);
 		break;
 
+	case REMOTE:
+		error = wacom_led_groups_allocate(wacom, 5);
+		if (error) {
+			hid_err(wacom->hdev,
+				"cannot create leds err: %d\n", error);
+			return error;
+		}
+		return 0;
+
 	default:
 		return 0;
 	}
@@ -1204,7 +1261,7 @@ static ssize_t wacom_show_remote_mode(struct kobject *kobj,
 	struct wacom *wacom = hid_get_drvdata(hdev);
 	u8 mode;
 
-	mode = wacom->led.select[index];
+	mode = wacom->led.groups[index].select;
 	if (mode >= 0 && mode < 3)
 		return snprintf(buf, PAGE_SIZE, "%d\n", mode);
 	else
@@ -1272,7 +1329,7 @@ void wacom_remote_destroy_attr_group(struct wacom *wacom, __u32 serial)
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
 		if (wacom_wac->serial[i] == serial) {
 			wacom_wac->serial[i] = 0;
-			wacom->led.select[i] = WACOM_STATUS_UNKNOWN;
+			wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
 			if (wacom->remote_group[i].name) {
 				sysfs_remove_group(wacom->remote_dir,
 						   &wacom->remote_group[i]);
@@ -1369,7 +1426,7 @@ static int wacom_initialize_remote(struct wacom *wacom)
 	}
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
-		wacom->led.select[i] = WACOM_STATUS_UNKNOWN;
+		wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
 		wacom_wac->serial[i] = 0;
 	}
 
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 3e2c903..932d3ee 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -808,7 +808,7 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
 		if (wacom_wac->serial[i] == serial)
-			wacom->led.select[i] = touch_ring_mode;
+			wacom->led.groups[i].select = touch_ring_mode;
 	}
 
 	if (!wacom->battery &&
-- 
2.5.5

  parent reply	other threads:[~2016-07-13 16:07 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-13 16:05 [PATCH v2 00/30] HID: wacom: cleanup/EKR/LED Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 01/30] HID: wacom: actually report the battery level for wireless connected Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 02/30] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2 Benjamin Tissoires
2016-07-15  4:14   ` Ping Cheng
2016-07-18 14:54     ` Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 03/30] HID: wacom: remove cleanup of wacom->remote_dir from wacom_clean_inputs() Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 04/30] HID: wacom: untie leds from inputs Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 05/30] HID: wacom: use one work queue per task Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 06/30] HID: wacom: switch battery to devres Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 07/30] HID: wacom: switch inputs " Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 08/30] HID: wacom: put the managed resources in a group Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 09/30] HID: wacom: convert LEDs to devres Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 10/30] HID: wacom: use devm_kasprintf for allocating the name of the remote Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 11/30] HID: wacom: use devres to allocate driver data Benjamin Tissoires
2016-07-13 16:05 ` [PATCH v2 12/30] HID: wacom: devres manage the shared data too Benjamin Tissoires
2016-07-13 16:06 ` Benjamin Tissoires [this message]
2016-07-13 16:06 ` [PATCH v2 14/30] HID: wacom: EKR: add a worker to add/remove resources on addition/removal Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 15/30] HID: wacom: EKR: have the wacom resources dynamically allocated Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 16/30] HID: wacom: rework fail path in probe() and parse_and_register() Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 17/30] HID: wacom: EKR: have proper allocator and destructor Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 18/30] HID: wacom: EKR: use devres groups to manage resources Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 19/30] HID: wacom: EKR: have one array of struct remotes instead of many arrays Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 20/30] HID: wacom: EKR: allocate one input node per remote Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 21/30] HID: wacom: EKR: have one power_supply " Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 22/30] HID: wacom: EKR: attach the power_supply on first connection Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 23/30] HID: wacom: leds: use the ledclass instead of custom made sysfs files Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 24/30] HID: wacom: leds: actually release the LEDs on disconnect Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 25/30] HID: wacom: leds: handle the switch of the LEDs directly in the kernel Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 26/30] HID: wacom: leds: make sure Cintiq 21UX2 and 24HD control the right LEDs Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 27/30] HID: wacom: leds: handle Cintiq 24HD leds buttons Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 28/30] HID: wacom: power_supply: mark the type as USB Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 29/30] HID: wacom: power_supply: remove ac information Benjamin Tissoires
2016-07-13 16:06 ` [PATCH v2 30/30] HID: wacom: power_supply: provide the actual model_name Benjamin Tissoires
2016-07-13 21:36 ` [PATCH v2 00/30] HID: wacom: cleanup/EKR/LED Aaron Armstrong Skomra
2016-08-01 23:17   ` Ping Cheng
2016-08-02  8:48     ` Jiri Kosina
2016-08-05 11:41 ` Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1468425977-4819-14-git-send-email-benjamin.tissoires@redhat.com \
    --to=benjamin.tissoires@redhat.com \
    --cc=jikos@kernel.org \
    --cc=killertofu@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter.hutterer@who-t.net \
    --cc=pinglinux@gmail.com \
    --cc=skomra@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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).