Re: [PATCH v2 3/5] hwmon: (pmbus/adm1266) add powerup_counter debugfs entry

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The debugfs read function adm1266_powerup_counter_read() lacks mandatory subsystem locking.
--

commit 7f6c699c3d3803ae6836691f51cc018599b04811
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) add powerup_counter debugfs entry

This commit adds a read-only debugfs file to expose the ADM1266's 16-bit
non-volatile powerup counter.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 31aefa8fc75f5..99e92a8365fb7 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c

[ ... ]

> +static int adm1266_powerup_counter_read(struct seq_file *s, void *pdata)
> +{
> +	struct device *dev = s->private;
> +	struct i2c_client *client = to_i2c_client(dev);
> +	u8 buf[I2C_SMBUS_BLOCK_MAX];
> +	int ret;
> +
> +	ret = i2c_smbus_read_block_data(client, ADM1266_POWERUP_COUNTER, buf);

Does this debugfs read need to be protected by a lock?

The hardware monitoring subsystem guidelines state that drivers must
implement locking for attributes registered outside of the core info
struct, typically using hwmon_lock() or pmbus_lock().

Without synchronization here, could this I2C read interleave with multi-step
PMBus operations from other sysfs attributes or interrupt handlers and break
command atomicity?

> +	if (ret < 0)
> +		return ret;
> +	if (ret != 2)
> +		return -EIO;
> +
> +	seq_printf(s, "%u\n", buf[0] | (buf[1] << 8));
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510-adm1266-v2-0-3a22b903c2f1@nexthop.ai?part=3