Re: [PATCH] hwmon: (pmbus/ltc4286) Add runtime shunt_resistor control

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The shared variable `data->rsense` is read without lock protection in the sysfs show function, causing a data race with concurrent store operations.
--

commit ba83e29a82adf5c7ae93ecbb0408db51473aa280
Author: Carl Lee <carl.lee@amd.com>

hwmon: (pmbus/ltc4286) Add runtime shunt_resistor control
    
This commit exposes a writable shunt_resistor sysfs attribute for the
ltc4286/ltc4287 drivers. It allows userspace to update the shunt resistor
value at runtime and updates the corresponding direct-format scaling
coefficients.

> diff --git a/drivers/hwmon/pmbus/ltc4286.c b/drivers/hwmon/pmbus/ltc4286.c
> index 8715d380784a0..e80c9ecea0997 100644
> --- a/drivers/hwmon/pmbus/ltc4286.c
> +++ b/drivers/hwmon/pmbus/ltc4286.c
[ ... ]
> @@ -16,6 +17,75 @@
>  
>  #define LTC4286_MFR_ID_SIZE	3
>  
> +struct ltc4286_data {
> +	struct pmbus_driver_info info;
> +	u32 rsense;
> +	bool vrange_low_enable;
> +};
> +
> +#define to_ltc4286_data(x) container_of((x), struct ltc4286_data, info)
> +
> +static ssize_t ltc4286_rsense_show(struct device *dev,
> +				   struct device_attribute *devattr,
> +				   char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev->parent);
> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> +	struct ltc4286_data *data = to_ltc4286_data(info);
> +
> +	return sysfs_emit(buf, "%u\n", data->rsense);

[Severity: Low]
Is it safe to read data->rsense here without holding a lock?

Since ltc4286_rsense_store() updates data->rsense under the protection of
pmbus_lock_interruptible(), a concurrent sysfs read and write could trigger
a KCSAN data race warning.

> +}
> +
> +static ssize_t ltc4286_rsense_store(struct device *dev,
> +				    struct device_attribute *devattr,
> +				    const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev->parent);
> +	const struct pmbus_driver_info *info_ro = pmbus_get_driver_info(client);
> +	struct ltc4286_data *data = to_ltc4286_data(info_ro);
> +	struct pmbus_driver_info *info = &data->info;
> +	u32 rsense;
> +	int ret;
> +
> +	ret = kstrtou32(buf, 10, &rsense);
> +	if (ret)
> +		return ret;
> +
> +	if (rsense == 0)
> +		return -EINVAL;
> +
> +	if (rsense > (INT_MAX / 1024))
> +		return -EINVAL;
> +
> +	ret = pmbus_lock_interruptible(client);
> +	if (ret)
> +		return ret;
> +
> +	data->rsense = rsense;
> +	info->m[PSC_CURRENT_OUT] = 1024 * rsense;
> +	info->m[PSC_POWER] = data->vrange_low_enable ? 4 * rsense : rsense;
> +
> +	pmbus_unlock(client);
> +
> +	return count;
> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-hwmon-pmbus-ltc4286-add-runtime-shunt-resistor-control-v1-1-aca2d49a1418@amd.com?part=1