Re: [PATCH v2 1/1] hwmon: Add Minisforum UM780 XTX EC monitoring and fan control

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `um780xtx_suspend` PM callback propagates errors from `um780xtx_cache_state()`, which will abort the entire system suspend if an EC read fails or the profile is unknown.
--

--- Patch [1]: [PATCH v2 1/1] hwmon: Add Minisforum UM780 XTX EC monitoring and fan control ---
commit 7277194b4d85f9b2916cb9ff030086a488cff81b
Author: Sebastián Peyrott <[email protected]>

hwmon: Add Minisforum UM780 XTX EC monitoring and fan control

This commit adds a DMI-gated hwmon driver for the embedded controller used by
the Minisforum UM780 XTX. It exposes CPU and system fan control temperatures
and tachometers, and allows selecting OEM CPU fan profiles and changing system
fan transition temperatures. It also caches and restores settings across
suspend/resume.

> diff --git a/drivers/hwmon/minisforum-um780xtx.c b/drivers/hwmon/minisforum-um780xtx.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hwmon/minisforum-um780xtx.c

[ ... ]

> +static int um780xtx_suspend(struct device *dev)
> +{
> +	struct um780xtx_data *data = dev_get_drvdata(dev);
> +
> +	guard(hwmon_lock)(data->hwmon_dev);
> +	return um780xtx_cache_state(data);

[Severity: High]
Does returning this error abort the entire system suspend?

In um780xtx_suspend(), if communicating with the embedded controller fails
momentarily or returns an unknown profile, um780xtx_cache_state() returns
an error code like -ENODEV or -EIO. Propagating this error to the PM core
causes the system suspend operation to be aborted.

Should this code log a warning and return 0 instead of aborting suspend for
a fan state save failure?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1