[PATCH] hwmon: (asus_rog_ryujin) Update cached duty cycle after cooler PWM write

[PATCH] hwmon: (asus_rog_ryujin) Update cached duty cycle after cooler PWM write

[Guenter Roeck]

In rog_ryujin_write_fixed_duty(), writing a PWM duty cycle to channel 0
(pump) or channel 1 (internal fan) returns directly from
rog_ryujin_write_cooler_duty(), skipping the priv->duty_input[channel]
update at the end of the function. Because the function first fetches
the pre-write cooler duty via get_cooler_duty_cmd and SET confirmation
reports do not update priv->duty_input, subsequent sysfs reads within
the STATUS_VALIDITY (1500 ms) cache window return stale duty values (or,
on Ryujin III when switching pump source at the reported duty, the
temporary neighbouring duty written by rog_ryujin_select_pump_source()).

Furthermore, rog_ryujin_write_fixed_duty() overwrote val in place with
rog_ryujin_pwm_to_percent(val), so even before the early return was
introduced, priv->duty_input[channel] stored a 0-100 percentage rather
than the 0-255 PWM value expected by rog_ryujin_read().

Keep the 0-100 percentage in a separate local variable and fall through
on success so priv->duty_input[channel] caches the written 0-255 PWM
value until the next status refresh.

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

diff --git a/drivers/hwmon/asus_rog_ryujin.c b/drivers/hwmon/asus_rog_ryujin.c
index 695e74cf71e4..7b80c46a0930 100644
--- a/drivers/hwmon/asus_rog_ryujin.c
+++ b/drivers/hwmon/asus_rog_ryujin.c
@@ -420,6 +420,7 @@ static int rog_ryujin_select_pump_source(struct rog_ryujin_data *priv, u8 *set_c
 static int rog_ryujin_write_fixed_duty(struct rog_ryujin_data *priv, int channel, int val)
 {
 	u8 set_cmd[SET_CMD_LENGTH];
+	int duty;
 	int ret;
 
 	if (channel < 2) {
@@ -437,18 +438,18 @@ static int rog_ryujin_write_fixed_duty(struct rog_ryujin_data *priv, int channel
 		set_cmd[RYUJIN_SET_COOLER_CHANNEL_OFFSET] = priv->info->duty_channel;
 
 		/* Cooler duties are set as 0-100% */
-		val = rog_ryujin_pwm_to_percent(val);
+		duty = rog_ryujin_pwm_to_percent(val);
 
 		if (channel == 0) {
 			/* Cooler pump duty */
-			set_cmd[RYUJIN_SET_COOLER_PUMP_DUTY_OFFSET] = val;
+			set_cmd[RYUJIN_SET_COOLER_PUMP_DUTY_OFFSET] = duty;
 			set_cmd[RYUJIN_SET_COOLER_FAN_DUTY_OFFSET] =
 			    rog_ryujin_pwm_to_percent(priv->duty_input[1]);
 		} else if (channel == 1) {
 			/* Cooler internal fan duty */
 			set_cmd[RYUJIN_SET_COOLER_PUMP_DUTY_OFFSET] =
 			    rog_ryujin_pwm_to_percent(priv->duty_input[0]);
-			set_cmd[RYUJIN_SET_COOLER_FAN_DUTY_OFFSET] = val;
+			set_cmd[RYUJIN_SET_COOLER_FAN_DUTY_OFFSET] = duty;
 		}
 
 		if (priv->info->duty_channel) {
@@ -457,7 +458,9 @@ static int rog_ryujin_write_fixed_duty(struct rog_ryujin_data *priv, int channel
 				return ret;
 		}
 
-		return rog_ryujin_write_cooler_duty(priv, set_cmd);
+		ret = rog_ryujin_write_cooler_duty(priv, set_cmd);
+		if (ret < 0)
+			return ret;
 	} else {
 		/*
 		 * Controller fan duty (channel == 2). No need to retrieve current
-- 
2.45.2

Re: [PATCH] hwmon: (asus_rog_ryujin) Update cached duty cycle after cooler PWM write

[sashiko-bot]

> In rog_ryujin_write_fixed_duty(), writing a PWM duty cycle to channel 0
> (pump) or channel 1 (internal fan) returns directly from
> rog_ryujin_write_cooler_duty(), skipping the priv->duty_input[channel]
> update at the end of the function. Because the function first fetches
> the pre-write cooler duty via get_cooler_duty_cmd and SET confirmation
> reports do not update priv->duty_input, subsequent sysfs reads within
> the STATUS_VALIDITY (1500 ms) cache window return stale duty values (or,
> on Ryujin III when switching pump source at the reported duty, the
> temporary neighbouring duty written by rog_ryujin_select_pump_source()).
> 
> Furthermore, rog_ryujin_write_fixed_duty() overwrote val in place with
> rog_ryujin_pwm_to_percent(val), so even before the early return was
> introduced, priv->duty_input[channel] stored a 0-100 percentage rather
> than the 0-255 PWM value expected by rog_ryujin_read().
> 
> Keep the 0-100 percentage in a separate local variable and fall through
> on success so priv->duty_input[channel] caches the written 0-255 PWM
> value until the next status refresh.
> 
> Fixes: ed3e03790c5c ("hwmon: Add driver for ASUS ROG RYUJIN II 360 AIO cooler")
> Fixes: b4306c0c8e24 ("hwmon: (asus_rog_ryujin) Rely on subsystem locking")
> 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/20261105044843.3262037-1-linux@roeck-us.net?part=1