Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Maxim Mikityanskiy <maxtram95@gmail.com>
To:     Jiri Kosina <jikos@kernel.org>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Daniel Kurtz <djkurtz@chromium.org>,
        Oliver Neukum <oneukum@suse.de>
Cc:     linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
        Maxim Mikityanskiy <maxtram95@gmail.com>
Subject: [PATCH 5/6] HID: hid-input: Update LEDs in all HID reports
Date:   Sun,  4 Jul 2021 01:02:01 +0300
Message-Id: <20210703220202.5637-6-maxtram95@gmail.com>
In-Reply-To: <20210703220202.5637-1-maxtram95@gmail.com>
References: <20210703220202.5637-1-maxtram95@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

hidinput_led_worker is scheduled on a work queue to update all LEDs in a
batch. However, it uses hidinput_get_led_field which gets the first LED
field found, and updates only the report this field belongs to. There
are devices that expose multiple LEDs in multiple reports. The current
implementation of the worker fails to update some LEDs on such devices.

Plantronics Blackwire 3220 Series (047f:c056) is an example of such
device. Only mute LED works, but offhook and ring LEDs don't work.

This commit fixes hidinput_led_worker by making it go over all reports
that contain at least one LED field.

Fixes: 4371ea8202e9 ("HID: usbhid: defer LED setting to a workqueue")
Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/hid/hid-input.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 533a7f429a5f..29f59208b34c 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1663,22 +1663,29 @@ unsigned int hidinput_count_leds(struct hid_device *hid)
 }
 EXPORT_SYMBOL_GPL(hidinput_count_leds);
 
-static void hidinput_led_worker(struct work_struct *work)
+static bool hidinput_is_led_report(struct hid_report *report)
 {
-	struct hid_device *hid = container_of(work, struct hid_device,
-					      led_work);
 	struct hid_field *field;
-	struct hid_report *report;
+	int i, j;
+
+	for (i = 0; i < report->maxfield; i++) {
+		field = report->field[i];
+		for (j = 0; j < field->maxusage; j++)
+			if (field->usage[j].type == EV_LED)
+				return true;
+	}
+
+	return false;
+}
+
+static void hidinput_led_update(struct hid_device *hid, struct hid_report *report)
+{
 	int ret;
 	u32 len;
 	__u8 *buf;
 
-	field = hidinput_get_led_field(hid);
-	if (!field)
-		return;
-
 	/*
-	 * field->report is accessed unlocked regarding HID core. So there might
+	 * report is accessed unlocked regarding HID core. So there might
 	 * be another incoming SET-LED request from user-space, which changes
 	 * the LED state while we assemble our outgoing buffer. However, this
 	 * doesn't matter as hid_output_report() correctly converts it into a
@@ -1690,8 +1697,6 @@ static void hidinput_led_worker(struct work_struct *work)
 	 * correct value, guaranteed!
 	 */
 
-	report = field->report;
-
 	/* use custom SET_REPORT request if possible (asynchronous) */
 	if (hid->ll_driver->request)
 		return hid->ll_driver->request(hid, report, HID_REQ_SET_REPORT);
@@ -1711,6 +1716,20 @@ static void hidinput_led_worker(struct work_struct *work)
 	kfree(buf);
 }
 
+static void hidinput_led_worker(struct work_struct *work)
+{
+	struct hid_device *hid = container_of(work, struct hid_device,
+					      led_work);
+	struct hid_report *report;
+
+	list_for_each_entry(report,
+			    &hid->report_enum[HID_OUTPUT_REPORT].report_list,
+			    list) {
+		if (hidinput_is_led_report(report))
+			hidinput_led_update(hid, report);
+	}
+}
+
 static int hidinput_input_event(struct input_dev *dev, unsigned int type,
 				unsigned int code, int value)
 {
-- 
2.32.0