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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `adm1266_clear_blackbox_write` debugfs attribute modifies hardware state without holding the required subsystem locks.
- [Low] A zero-byte write to the `clear_blackbox` debugfs file unconditionally triggers the destructive blackbox clear command.
--

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

hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry

This commit exposes a write-only debugfs file to issue the clear-blackbox
sub-command for deployments running in single recording mode. Writing any
data to the file triggers a 2-byte block-write to READ_BLACKBOX.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
[ ... ]
> +/*
> + * Clearing the blackbox is required when the device is configured in
> + * single-recording mode (BLACKBOX_CONFIG[0] = 0): once the 32-record
> + * buffer is full the device stops recording until cleared.
> + *
> + * The clear is issued as a 2-byte block-write to READ_BLACKBOX with
> + * payload {0xFE, 0x00} per the datasheet.
> + */
> +static ssize_t adm1266_clear_blackbox_write(struct file *file, const char __user *ubuf,
> +					    size_t count, loff_t *ppos)
> +{
> +	struct i2c_client *client = file->private_data;
> +	u8 payload[2] = { 0xFE, 0x00 };
> +	int ret;

Does this code trigger the hardware clear even for a zero-byte write?
The VFS layer can invoke the write handler with count == 0. Should we
verify that count > 0 before issuing the I2C command?

Is locking required here to prevent races with other device accesses?

The hardware monitoring subsystem guidelines state that drivers must
implement locking (e.g., using hwmon_lock()) for attributes registered
outside the core, such as debugfs files.

If a write to this file occurs concurrently with the multi-step read loop
in adm1266_nvmem_read_blackbox():

drivers/hwmon/pmbus/adm1266.c:adm1266_nvmem_read_blackbox() {
    ...
    for (index = 0; index < record_count; index++) {
        ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, ...);
    ...
}

could it clear the hardware buffer mid-read and cause corrupted data or
I2C errors?

> +
> +	ret = i2c_smbus_write_block_data(client, ADM1266_READ_BLACKBOX,
> +					 sizeof(payload), payload);
> +	if (ret < 0)
> +		return ret;
> +
> +	return count;
> +}

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