linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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	[thread overview]
Message-ID: <20210703220202.5637-6-maxtram95@gmail.com> (raw)
In-Reply-To: <20210703220202.5637-1-maxtram95@gmail.com>

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


  parent reply	other threads:[~2021-07-03 22:02 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-03 22:01 [PATCH 0/6] Add support for common USB HID headset features Maxim Mikityanskiy
2021-07-03 22:01 ` [PATCH 1/6] HID: hid-input: Add offhook and ring LEDs for headsets Maxim Mikityanskiy
2021-07-06  8:02   ` Benjamin Tissoires
2021-07-15 18:57     ` Jiri Kosina
2021-07-15 20:39       ` Dmitry Torokhov
2021-07-15 22:49         ` Pavel Machek
2021-07-16 17:23           ` Maxim Mikityanskiy
2021-08-09 18:30             ` Maxim Mikityanskiy
2021-08-31 19:11               ` Jiri Kosina
2021-09-07  6:30             ` Dmitry Torokhov
2021-07-03 22:01 ` [PATCH 2/6] HID: hid-input: Add phone hook and mic mute buttons " Maxim Mikityanskiy
2021-07-03 22:01 ` [PATCH 3/6] HID: plantronics: Expose headset LEDs Maxim Mikityanskiy
2021-07-03 22:02 ` [PATCH 4/6] HID: plantronics: Expose headset telephony buttons Maxim Mikityanskiy
2021-07-03 22:02 ` Maxim Mikityanskiy [this message]
2021-07-03 22:02 ` [PATCH 6/6] HID: jabra: Change mute LED state to avoid missing key press events Maxim Mikityanskiy

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=20210703220202.5637-6-maxtram95@gmail.com \
    --to=maxtram95@gmail.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=djkurtz@chromium.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oneukum@suse.de \
    /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).