[PATCH] hwmon: (asus_rog_ryujin) Fix timeout when reading 0% duty cycle

[PATCH] hwmon: (asus_rog_ryujin) Fix timeout when reading 0% duty cycle

[Guenter Roeck]

The device uses the same zero-duty report format for both legitimate 0%
duty GET responses and SET confirmation ACKs on cooler and controller
duty channels. To distinguish them, rog_ryujin_raw_event() checks
!completion_done() on cooler_duty_set and controller_duty_set, assuming
this is only true while a SET command is awaiting confirmation.

However, init_completion() initializes the completion count to zero, so
!completion_done() evaluates to true from probe until the first SET
command completes. Likewise, if rog_ryujin_execute_cmd() fails to send a
command, times out, or is interrupted after reinit_completion(), the
completion remains uncompleted. When a valid 0% duty GET response then
arrives, rog_ryujin_raw_event() consumes it as a SET confirmation and
leaves the pending *_duty_received completion uncompleted, causing
rog_ryujin_execute_cmd() to wait 1.5 seconds and fail with -ETIMEDOUT.

Mark all completions as done with complete_all() after initialization in
rog_ryujin_probe(), and complete status_completion on error in
rog_ryujin_execute_cmd(), so !completion_done() is only true while a
command is actively awaiting a response.

Fixes: ed3e03790c5c ("hwmon: Add driver for ASUS ROG RYUJIN II 360 AIO cooler")
Cc: Aleksa Savic <savicaleksa83@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/asus_rog_ryujin.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/asus_rog_ryujin.c b/drivers/hwmon/asus_rog_ryujin.c
index ba188e867934..111e0099fd9c 100644
--- a/drivers/hwmon/asus_rog_ryujin.c
+++ b/drivers/hwmon/asus_rog_ryujin.c
@@ -207,16 +207,23 @@ static int rog_ryujin_execute_cmd(struct rog_ryujin_data *priv, const u8 *cmd, i
 	/* Send command for getting data */
 	ret = rog_ryujin_write_expanded(priv, cmd, cmd_length);
 	if (ret < 0)
-		return ret;
+		goto out_complete;
 
 	ret = wait_for_completion_interruptible_timeout(status_completion,
 							msecs_to_jiffies(STATUS_VALIDITY));
 	if (ret == 0)
-		return -ETIMEDOUT;
-	else if (ret < 0)
-		return ret;
+		ret = -ETIMEDOUT;
+	if (ret < 0)
+		goto out_complete;
 
 	return 0;
+
+out_complete:
+	spin_lock_irqsave(&priv->status_report_request_lock, flags);
+	if (!completion_done(status_completion))
+		complete_all(status_completion);
+	spin_unlock_irqrestore(&priv->status_report_request_lock, flags);
+	return ret;
 }
 
 static int rog_ryujin_get_status(struct rog_ryujin_data *priv)
@@ -630,6 +637,18 @@ static int rog_ryujin_probe(struct hid_device *hdev, const struct hid_device_id
 	init_completion(&priv->cooler_duty_set);
 	init_completion(&priv->controller_duty_set);
 
+	/*
+	 * Start completions in the completed state so rog_ryujin_raw_event()
+	 * only sees !completion_done() while rog_ryujin_execute_cmd() is
+	 * awaiting a response.
+	 */
+	complete_all(&priv->cooler_status_received);
+	complete_all(&priv->controller_status_received);
+	complete_all(&priv->cooler_duty_received);
+	complete_all(&priv->controller_duty_received);
+	complete_all(&priv->cooler_duty_set);
+	complete_all(&priv->controller_duty_set);
+
 	priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "rog_ryujin",
 							  priv, &rog_ryujin_chip_info, NULL);
 	if (IS_ERR(priv->hwmon_dev)) {
-- 
2.45.2

Re: [PATCH] hwmon: (asus_rog_ryujin) Fix timeout when reading 0% duty cycle

[sashiko-bot]

> The device uses the same zero-duty report format for both legitimate 0%
> duty GET responses and SET confirmation ACKs on cooler and controller
> duty channels. To distinguish them, rog_ryujin_raw_event() checks
> !completion_done() on cooler_duty_set and controller_duty_set, assuming
> this is only true while a SET command is awaiting confirmation.
> 
> However, init_completion() initializes the completion count to zero, so
> !completion_done() evaluates to true from probe until the first SET
> command completes. Likewise, if rog_ryujin_execute_cmd() fails to send a
> command, times out, or is interrupted after reinit_completion(), the
> completion remains uncompleted. When a valid 0% duty GET response then
> arrives, rog_ryujin_raw_event() consumes it as a SET confirmation and
> leaves the pending *_duty_received completion uncompleted, causing
> rog_ryujin_execute_cmd() to wait 1.5 seconds and fail with -ETIMEDOUT.
> 
> Mark all completions as done with complete_all() after initialization in
> rog_ryujin_probe(), and complete status_completion on error in
> rog_ryujin_execute_cmd(), so !completion_done() is only true while a
> command is actively awaiting a response.
> 
> Fixes: ed3e03790c5c ("hwmon: Add driver for ASUS ROG RYUJIN II 360 AIO cooler")
> Cc: Aleksa Savic <savicaleksa83@gmail.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104144432.3199070-1-linux@roeck-us.net?part=1