Re: [PATCH] hwmon: mcp9982: Add external diode fault read

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Reading the `hwmon_temp_fault` attribute in Standby mode returns stale data because it fails to trigger a one-shot conversion.
--

commit 50b760776977e57ad65e2996d0bfd4a793fe2276
Author: Victor Duicu <victor.duicu@microchip.com>

hwmon: mcp9982: Add external diode fault read

This patch adds external diode fault read capability to the MCP998X/33
and MCP998XD/33D driver.

> diff --git a/drivers/hwmon/mcp9982.c b/drivers/hwmon/mcp9982.c
> index 26c69e3388ab8..2e75ade20118b 100644
> --- a/drivers/hwmon/mcp9982.c
> +++ b/drivers/hwmon/mcp9982.c

[ ... ]

> @@ -510,6 +510,18 @@ static int mcp9982_read(struct device *dev, enum hwmon_sensor_types type, u32 at
>  
>  			*val -= hyst * 1000;
>  
> +			return 0;
> +		case hwmon_temp_fault:
> +			/*
> +			 * Because the ALERT/THERM pin is set in Therm(Comparator)
> +			 * mode, the external diode fault status register
> +			 * does not clear the bits after reading.
> +			 */
> +			*val = regmap_test_bits(priv->regmap, MCP9982_EXT_FAULT_STATUS_ADDR,
> +						BIT(channel));

Does this return stale data when the device is in Standby mode?

When the device is in Standby mode (!priv->run_state), automatic measurements
are stopped, and the driver triggers a manual one-shot conversion before
reading attributes:

	if (!priv->run_state) {
		switch (type) {
		case hwmon_temp:
			switch (attr) {
			case hwmon_temp_input:
			case hwmon_temp_max_alarm:
			case hwmon_temp_min_alarm:
			case hwmon_temp_crit_alarm:
				ret = regmap_write(priv->regmap, MCP9982_ONE_SHOT_ADDR, 1);
				...

Since hwmon_temp_fault is not included in this wake-up list, it looks like
reading the fault status bypasses the wake-up logic and returns the cached
hardware state from the last active cycle instead of triggering a new
measurement.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260422-add-external-diode-fault-read-v1-1-bb28e03a0766@microchip.com?part=1