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

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

[Victor Duicu]

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

Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
---
 drivers/hwmon/mcp9982.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/mcp9982.c b/drivers/hwmon/mcp9982.c
index 26c69e3388ab..2e75ade20118 100644
--- a/drivers/hwmon/mcp9982.c
+++ b/drivers/hwmon/mcp9982.c
@@ -92,19 +92,19 @@ static const struct hwmon_channel_info * const mcp9985_info[] = {
 			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_MIN |
 			   HWMON_T_MIN_ALARM | HWMON_T_MAX | HWMON_T_MAX_ALARM |
 			   HWMON_T_MAX_HYST | HWMON_T_CRIT | HWMON_T_CRIT_ALARM |
-			   HWMON_T_CRIT_HYST,
+			   HWMON_T_CRIT_HYST | HWMON_T_FAULT,
 			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_MIN |
 			   HWMON_T_MIN_ALARM | HWMON_T_MAX | HWMON_T_MAX_ALARM |
 			   HWMON_T_MAX_HYST | HWMON_T_CRIT | HWMON_T_CRIT_ALARM |
-			   HWMON_T_CRIT_HYST,
+			   HWMON_T_CRIT_HYST | HWMON_T_FAULT,
 			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_MIN |
 			   HWMON_T_MIN_ALARM | HWMON_T_MAX | HWMON_T_MAX_ALARM |
 			   HWMON_T_MAX_HYST | HWMON_T_CRIT | HWMON_T_CRIT_ALARM |
-			   HWMON_T_CRIT_HYST,
+			   HWMON_T_CRIT_HYST | HWMON_T_FAULT,
 			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_MIN |
 			   HWMON_T_MIN_ALARM | HWMON_T_MAX | HWMON_T_MAX_ALARM |
 			   HWMON_T_MAX_HYST | HWMON_T_CRIT | HWMON_T_CRIT_ALARM |
-			   HWMON_T_CRIT_HYST),
+			   HWMON_T_CRIT_HYST | HWMON_T_FAULT),
 	HWMON_CHANNEL_INFO(chip,
 			   HWMON_C_UPDATE_INTERVAL),
 	NULL
@@ -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));
+			if (*val < 0)
+				return *val;
+
 			return 0;
 		default:
 			return -EINVAL;
@@ -681,6 +693,7 @@ static umode_t mcp9982_is_visible(const void *_data, enum hwmon_sensor_types typ
 		case hwmon_temp_max_alarm:
 		case hwmon_temp_max_hyst:
 		case hwmon_temp_crit_alarm:
+		case hwmon_temp_fault:
 			return 0444;
 		case hwmon_temp_min:
 		case hwmon_temp_max:

---
base-commit: fb447217c59a13b2fff22d94de2498c185cd9032
change-id: 20260422-add-external-diode-fault-read-a0c3cd1a4761

Best regards,
-- 
Victor Duicu <victor.duicu@microchip.com>

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

[Guenter Roeck]

On 4/22/26 01:43, Victor Duicu wrote:
> This patch adds external diode fault read capability
> to the MCP998X/33 and MCP998XD/33D driver.
> 
> Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
> ---
>   drivers/hwmon/mcp9982.c | 21 +++++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/mcp9982.c b/drivers/hwmon/mcp9982.c
> index 26c69e3388ab..2e75ade20118 100644
> --- a/drivers/hwmon/mcp9982.c
> +++ b/drivers/hwmon/mcp9982.c
> @@ -92,19 +92,19 @@ static const struct hwmon_channel_info * const mcp9985_info[] = {
>   			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_MIN |
>   			   HWMON_T_MIN_ALARM | HWMON_T_MAX | HWMON_T_MAX_ALARM |
>   			   HWMON_T_MAX_HYST | HWMON_T_CRIT | HWMON_T_CRIT_ALARM |
> -			   HWMON_T_CRIT_HYST,
> +			   HWMON_T_CRIT_HYST | HWMON_T_FAULT,
>   			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_MIN |
>   			   HWMON_T_MIN_ALARM | HWMON_T_MAX | HWMON_T_MAX_ALARM |
>   			   HWMON_T_MAX_HYST | HWMON_T_CRIT | HWMON_T_CRIT_ALARM |
> -			   HWMON_T_CRIT_HYST,
> +			   HWMON_T_CRIT_HYST | HWMON_T_FAULT,
>   			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_MIN |
>   			   HWMON_T_MIN_ALARM | HWMON_T_MAX | HWMON_T_MAX_ALARM |
>   			   HWMON_T_MAX_HYST | HWMON_T_CRIT | HWMON_T_CRIT_ALARM |
> -			   HWMON_T_CRIT_HYST,
> +			   HWMON_T_CRIT_HYST | HWMON_T_FAULT,
>   			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_MIN |
>   			   HWMON_T_MIN_ALARM | HWMON_T_MAX | HWMON_T_MAX_ALARM |
>   			   HWMON_T_MAX_HYST | HWMON_T_CRIT | HWMON_T_CRIT_ALARM |
> -			   HWMON_T_CRIT_HYST),
> +			   HWMON_T_CRIT_HYST | HWMON_T_FAULT),
>   	HWMON_CHANNEL_INFO(chip,
>   			   HWMON_C_UPDATE_INTERVAL),
>   	NULL
> @@ -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.
> +			 */


What does that mean ? How does this behavior differ from other alarm
bits to warrant that comment ?

Thanks,
Guenter

> +			*val = regmap_test_bits(priv->regmap, MCP9982_EXT_FAULT_STATUS_ADDR,
> +						BIT(channel));
> +			if (*val < 0)
> +				return *val;
> +
>   			return 0;
>   		default:
>   			return -EINVAL;
> @@ -681,6 +693,7 @@ static umode_t mcp9982_is_visible(const void *_data, enum hwmon_sensor_types typ
>   		case hwmon_temp_max_alarm:
>   		case hwmon_temp_max_hyst:
>   		case hwmon_temp_crit_alarm:
> +		case hwmon_temp_fault:
>   			return 0444;
>   		case hwmon_temp_min:
>   		case hwmon_temp_max:
> 
> ---
> base-commit: fb447217c59a13b2fff22d94de2498c185cd9032
> change-id: 20260422-add-external-diode-fault-read-a0c3cd1a4761
> 
> Best regards,

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

[sashiko-bot]

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