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@xxxxxxxxxxxxx>
> ---
>   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,