From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Jiri Kosina <jikos@kernel.org>,
". Benjamin Tissoires" <bentiss@kernel.org>
Cc: Douglas Anderson <dianders@chromium.org>,
Hans de Goede <hdegoede@redhat.com>, Kenny Levinsen <kl@kl.wtf>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] HID: i2c-hid: ensure various commands do not interfere with each other
Date: Mon, 9 Sep 2024 13:37:40 -0700 [thread overview]
Message-ID: <Zt9clAu04BinzIcd@google.com> (raw)
i2c-hid uses 2 shared buffers: command and "raw" input buffer for
sending requests to peripherals and read data from peripherals when
executing variety of commands. Such commands include reading of HID
registers, requesting particular power mode, getting and setting
reports and so on. Because all such requests use the same 2 buffers
they should not execute simultaneously.
Fix this by introducing "cmd_lock" mutex and acquire it whenever
we needs to access ihid->cmdbuf or idid->rawbuf.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 42 +++++++++++++++++++-----------
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 632eaf9e11a6..2f8a9d3f1e86 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -105,6 +105,7 @@ struct i2c_hid {
wait_queue_head_t wait; /* For waiting the interrupt */
+ struct mutex cmd_lock; /* protects cmdbuf and rawbuf */
struct mutex reset_lock;
struct i2chid_ops *ops;
@@ -220,6 +221,8 @@ static int i2c_hid_xfer(struct i2c_hid *ihid,
static int i2c_hid_read_register(struct i2c_hid *ihid, __le16 reg,
void *buf, size_t len)
{
+ guard(mutex)(&ihid->cmd_lock);
+
*(__le16 *)ihid->cmdbuf = reg;
return i2c_hid_xfer(ihid, ihid->cmdbuf, sizeof(__le16), buf, len);
@@ -252,6 +255,8 @@ static int i2c_hid_get_report(struct i2c_hid *ihid,
i2c_hid_dbg(ihid, "%s\n", __func__);
+ guard(mutex)(&ihid->cmd_lock);
+
/* Command register goes first */
*(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
length += sizeof(__le16);
@@ -342,6 +347,8 @@ static int i2c_hid_set_or_send_report(struct i2c_hid *ihid,
if (!do_set && le16_to_cpu(ihid->hdesc.wMaxOutputLength) == 0)
return -ENOSYS;
+ guard(mutex)(&ihid->cmd_lock);
+
if (do_set) {
/* Command register goes first */
*(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
@@ -384,6 +391,8 @@ static int i2c_hid_set_power_command(struct i2c_hid *ihid, int power_state)
{
size_t length;
+ guard(mutex)(&ihid->cmd_lock);
+
/* SET_POWER uses command register */
*(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
length = sizeof(__le16);
@@ -440,25 +449,27 @@ static int i2c_hid_start_hwreset(struct i2c_hid *ihid)
if (ret)
return ret;
- /* Prepare reset command. Command register goes first. */
- *(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
- length += sizeof(__le16);
- /* Next is RESET command itself */
- length += i2c_hid_encode_command(ihid->cmdbuf + length,
- I2C_HID_OPCODE_RESET, 0, 0);
+ scoped_guard(mutex, &ihid->cmd_lock) {
+ /* Prepare reset command. Command register goes first. */
+ *(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
+ length += sizeof(__le16);
+ /* Next is RESET command itself */
+ length += i2c_hid_encode_command(ihid->cmdbuf + length,
+ I2C_HID_OPCODE_RESET, 0, 0);
- set_bit(I2C_HID_RESET_PENDING, &ihid->flags);
+ set_bit(I2C_HID_RESET_PENDING, &ihid->flags);
- ret = i2c_hid_xfer(ihid, ihid->cmdbuf, length, NULL, 0);
- if (ret) {
- dev_err(&ihid->client->dev,
- "failed to reset device: %d\n", ret);
- goto err_clear_reset;
- }
+ ret = i2c_hid_xfer(ihid, ihid->cmdbuf, length, NULL, 0);
+ if (ret) {
+ dev_err(&ihid->client->dev,
+ "failed to reset device: %d\n", ret);
+ break;
+ }
- return 0;
+ return 0;
+ }
-err_clear_reset:
+ /* Clean up if sending reset command failed */
clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
return ret;
@@ -1200,6 +1211,7 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
ihid->is_panel_follower = drm_is_panel_follower(&client->dev);
init_waitqueue_head(&ihid->wait);
+ mutex_init(&ihid->cmd_lock);
mutex_init(&ihid->reset_lock);
INIT_WORK(&ihid->panel_follower_prepare_work, ihid_core_panel_prepare_work);
--
2.46.0.598.g6f2099f65c-goog
--
Dmitry
next reply other threads:[~2024-09-09 20:37 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-09 20:37 Dmitry Torokhov [this message]
2024-09-11 12:16 ` [PATCH] HID: i2c-hid: ensure various commands do not interfere with each other Hans de Goede
2024-09-11 13:27 ` Jiri Kosina
2024-09-11 18:18 ` Dmitry Torokhov
2024-09-11 18:20 ` Jiri Kosina
2024-09-12 12:12 ` 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=Zt9clAu04BinzIcd@google.com \
--to=dmitry.torokhov@gmail.com \
--cc=bentiss@kernel.org \
--cc=dianders@chromium.org \
--cc=hdegoede@redhat.com \
--cc=jikos@kernel.org \
--cc=kl@kl.wtf \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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).