Re: [PATCH 2/2] hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The GPIO attribute callback performs I2C bus operations without acquiring the required hardware monitoring subsystem lock.
--

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

hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple

This commit removes a redundant clearing of the bitmask in the GPIO
get_multiple callback, ensuring that both GPIO and PDIO bits are preserved
in the result.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 11f9a44f4361a..4dd67c02b412e 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -210,7 +210,6 @@ static int adm1266_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask

Does this GPIO attribute callback need to acquire the hardware monitoring
subsystem lock?

The hardware monitoring subsystem guidelines require that drivers implement
locking for attributes registered outside the core, to serialize with thermal
and sysfs operations.

Looking at adm1266_gpio_get_multiple(), it issues i2c_smbus_read_block_data()
without acquiring a lock. Could this cause a race condition against concurrent
PMBus accesses from the core?

>  
>  	status = read_buf[0] + (read_buf[1] << 8);
>  
> -	*bits = 0;
>  	for_each_set_bit_from(gpio_nr, mask, ADM1266_GPIO_NR + ADM1266_PDIO_NR) {
>  		if (test_bit(gpio_nr - ADM1266_GPIO_NR, &status))
>  			set_bit(gpio_nr, bits);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260516-adm1266-gpio-fixes-v1-0-38d9dd39b905@nexthop.ai?part=2