Re: [PATCH v5 03/10] iio: dac: mcp47feb02: Return len when disabling EEPROM store

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but suspend and resume operations clobber the user-configured channel powerdown state.
--

commit 835e4538da01c3026ccf6237cb4357a864fa680c
Author: Ariana Lazar <ariana.lazar@microchip.com>

iio: dac: mcp47feb02: Return len when disabling EEPROM store

This commit modifies store_eeprom_store() to return the input buffer
length instead of 0 when state is false. This properly signals that the
buffer was fully consumed, preventing unnecessary write retries.

> diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c
> index bf78618ac2c89..2ee981393aa1a 100644
> --- a/drivers/iio/dac/mcp47feb02.c
> +++ b/drivers/iio/dac/mcp47feb02.c
> @@ -469,7 +469,7 @@ static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *a
>  		return ret;
>  
>  	if (!state)
> -		return 0;
> +		return len;
>  

[Severity: High]
This is a pre-existing issue, but suspend and resume operations clobber the
user-configured channel powerdown state.

Looking at mcp47feb02_suspend():

drivers/iio/dac/mcp47feb02.c:mcp47feb02_suspend() {
    ...
	for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) {
		u8 pd_mode;

		data->chdata[ch].powerdown = true;
    ...
}

Does this unconditionally overwrite the channel's powerdown state during
suspend, causing the user's prior configuration to be permanently lost?

Similarly, looking at mcp47feb02_resume():

drivers/iio/dac/mcp47feb02.c:mcp47feb02_resume() {
    ...
	for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) {
		u8 pd_mode;
		int ret;

		data->chdata[ch].powerdown = false;
    ...
		ret = regmap_update_bits(data->regmap, MCP47FEB02_POWER_DOWN_REG_ADDR,
					 DAC_CTRL_MASK(ch),
					 DAC_CTRL_VAL(ch, MCP47FEB02_NORMAL_OPERATION));
    ...
}

Could this force all channels out of powerdown during resume, ignoring any
user-configured disabled state? Waking up a DAC channel that was explicitly
powered down by the user via sysfs could drive unintended voltage to attached
hardware upon system resume.

>  	/*
>  	 * Verify DAC Wiper and DAC Configuration are unlocked. If both are disabled,
>  	 * writing to EEPROM is available.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-mcp47feb02_refactor-v5-0-8b67bcab93d1@microchip.com?part=3