Re: [PATCH v13 2/3] hwmon: ltc4283: Add support for the LTC4283 Swap Controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Read-modify-write operations on hardware alarm and fault log registers will cause concurrent events to be lost.
--

commit 768d24f774b79f50e1444b93b756d7bda5873159
Author: Nuno Sá <nuno.sa@analog.com>

hwmon: ltc4283: Add support for the LTC4283 Swap Controller

This commit introduces driver support for the Analog Devices LTC4283
Negative Voltage Hot Swap Controller. It adds functionality to monitor
voltage, current, power, and various hardware faults via an I2C interface.

> diff --git a/drivers/hwmon/ltc4283.c b/drivers/hwmon/ltc4283.c
> new file mode 100644
> index 000000000000..d8931c9a4685
> --- /dev/null
> +++ b/drivers/hwmon/ltc4283.c

[ ... ]

> +static int ltc4283_read_alarm(struct ltc4283_hwmon *st, u32 reg,
> +			      u32 mask, long *val)
> +{
> +	u32 alarm;
> +	int ret;
> +
> +	ret = regmap_read(st->map, reg, &alarm);
> +	if (ret)
> +		return ret;
> +
> +	*val = !!(alarm & mask);
> +
> +	/* If not status/fault logs, clear the alarm after reading it. */
> +	if (reg != LTC4283_FAULT_STATUS && reg != LTC4283_FAULT_LOG)
> +		return regmap_write(st->map, reg, alarm & ~mask);

Could this read-modify-write sequence cause concurrent hardware events to be
lost?

If these alarm registers use write-0-to-clear semantics, writing back
alarm & ~mask will inadvertently write a 0 to any new alarm bits that were
asserted by the hardware between the regmap_read() and regmap_write() calls.
Usually, a direct atomic write of ~mask without reading first is preferred
to avoid clearing unrelated active alarms.

[ ... ]

> +static int ltc4283_reset_power_hist(struct ltc4283_hwmon *st)
> +{

[ ... ]

> +	/* Clear possible power faults. */
> +	return regmap_clear_bits(st->map, LTC4283_FAULT_LOG,
> +				 LTC4283_PWR_FAIL_FAULT_MASK | LTC4283_PGI_FAULT_MASK);
> +}

Will using regmap_clear_bits() here and in other history reset functions
also risk losing concurrent fault logs?

Since regmap_clear_bits() performs a read-modify-write under the hood, any
new faults asserting between the read and write phases might be cleared
silently. Direct writes using regmap_write() might be safer if the register
behaves as write-0-to-clear.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260502-ltc4283-support-v13-0-1c206542e652@analog.com?part=2