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

On Mon May 11, 2026 at 5:52 PM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The debugfs write handler `adm1266_clear_blackbox_write` performs an I2C transaction without acquiring the required hardware monitoring locks, violating hwmon subsystem guidelines.
> --
>
> commit ab9c19373562d58415e28511b037545cfd278c2a
> Author: Abdurrahman Hussain <[email protected]>
> Subject: hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry
>
> This commit adds a debugfs entry to clear the ADM1266 blackbox buffer by
> issuing a block-write command to the device. This is needed when the
> device operates in single-recording mode to preserve fault history.
>
>> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
>> index 7b8433226176f..31aefa8fc75f5 100644
>> --- a/drivers/hwmon/pmbus/adm1266.c
>> +++ b/drivers/hwmon/pmbus/adm1266.c
>
> [ ... ]
>
>> +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;
>> +
>> +	ret = i2c_smbus_write_block_data(client, ADM1266_READ_BLACKBOX,
>> +					 sizeof(payload), payload);
>
> Does this I2C block write need to be protected by hwmon_lock() and
> hwmon_unlock()?
>
> The hardware monitoring subsystem guidelines require drivers to implement
> locking for attributes registered outside of the core registration functions,
> such as debugfs entries.
>
> Without these locks, could this write operation race with other multi-record
> transactions? For example, if this clear command is interleaved while
> adm1266_nvmem_read_blackbox() is iterating through multiple records, might
> it corrupt the read sequence?
>
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return count;
>> +}

Good catch!

Will use pmbus_lock_interruptible() in v3.