Re: [PATCH 2/2] hwmon: (asus_rog_ryujin) Add pwm1_enable for the Ryujin III pump

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hwmon: asus_rog_ryujin: massive msleep() due to jiffies wrap-around
--

commit 9b0f4985a76174c3f4abba0e61bb854848b7c94c
Author: Zac Lanzon <lanzonza@gmail.com>

hwmon: (asus_rog_ryujin) Add pwm1_enable for the Ryujin III pump

This commit adds pwm1_enable support to the asus_rog_ryujin driver to expose 
and control the pump source. It also introduces delays to wait between source 
changes to ensure the cooler processes the commands correctly.

> diff --git a/drivers/hwmon/asus_rog_ryujin.c b/drivers/hwmon/asus_rog_ryujin.c
> index ba188e8679347..c89d854b2ae92 100644
> --- a/drivers/hwmon/asus_rog_ryujin.c
> +++ b/drivers/hwmon/asus_rog_ryujin.c

[ ... ]

> @@ -313,6 +330,46 @@ static int rog_ryujin_read_string(struct device *dev, enum hwmon_sensor_types ty
>  	return 0;
>  }
>  
> +/*
> + * Send a set cooler duty command. If it changes the pump control source, first
> + * wait until RYUJIN_SOURCE_CHANGE_INTERVAL has passed since the last change,
> + * then wait for the cooler duty report to show the new source, so that success
> + * means the change took effect.
> + */
> +static int rog_ryujin_write_cooler_duty(struct rog_ryujin_data *priv, const u8 *set_cmd)
> +{
> +	u8 source = set_cmd[RYUJIN_SET_COOLER_CHANNEL_OFFSET];
> +	unsigned long now, next, timeout;
> +	int ret;
> +
> +	if (!priv->info->duty_channel || source == priv->pump_source)
> +		return rog_ryujin_execute_cmd(priv, set_cmd, SET_CMD_LENGTH,
> +					      &priv->cooler_duty_set);
> +
> +	now = jiffies;
> +	next = priv->source_changed + msecs_to_jiffies(RYUJIN_SOURCE_CHANGE_INTERVAL);
> +	if (time_before(now, next))
> +		msleep(jiffies_to_msecs(next - now));

[Severity: Medium]
Is there a risk of a massive unintended sleep here due to a jiffies 
wrap-around? 

On 32-bit systems, if the elapsed time since priv->source_changed exceeds 
2^31 jiffies (for example, if there are no source changes for 24.8 days at 
HZ=1000), time_before() can wrap and return true. This would feed a very 
large wrapped value from next - now into msleep(), causing a thread hang 
when a write to the cooler duty attribute is attempted.

Checking the elapsed time via unsigned subtraction, such as 
now - priv->source_changed < msecs_to_jiffies(RYUJIN_SOURCE_CHANGE_INTERVAL), 
prevents this wrap-around issue.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927120927.72810-1-lanzonza@gmail.com?part=2