Re: [PATCH v4 6/9] iio: dac: mcp47feb02: rename command mask define

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect initialization of the `powerdown` state when the device boots in `1kohm_to_gnd` power-down mode.
--

--- Patch [6]: [PATCH v4 6/9] iio: dac: mcp47feb02: rename command mask define ---
commit 5165e39a3476c86b84b827af722b000649c5b496
Author: Ariana Lazar <ariana.lazar@microchip.com>

iio: dac: mcp47feb02: rename command mask define

This commit replaces a generic mask definition with two new macros,
MCP47FEB02_GET_VREF_MODE and MCP47FEB02_GET_POWER_DOWN_MODE. These
improve code readability when extracting channel configurations.

> diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c
> index 6f084eb71240b..8ca67c4021a86 100644
> --- a/drivers/iio/dac/mcp47feb02.c
> +++ b/drivers/iio/dac/mcp47feb02.c

[ ... ]

> @@ -1070,7 +1071,7 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data)
>  			break;
>  		}
>  
> -		pd_tmp = (pd_ch >> (2 * i)) & MCP47FEB02_DAC_CTRL_MASK;
> +		pd_tmp = MCP47FEB02_GET_POWER_DOWN_MODE(pd_ch, i);
>  		data->chdata[i].powerdown_mode = pd_tmp ? (pd_tmp - 1) : pd_tmp;
>  		data->chdata[i].powerdown = !!(data->chdata[i].powerdown_mode);

[Severity: High]
This is a pre-existing issue, but does this logic incorrectly set the channel
to an active state when the device boots in 1kohm_to_gnd power-down mode?

When the device is configured in 1kohm_to_gnd mode in
mcp47feb02_init_ctrl_regs(), pd_tmp evaluates to 1. The preceding line sets
powerdown_mode to 0 (1 - 1 = 0). As a result, the check
!!(data->chdata[i].powerdown_mode) evaluates to false.

Does this cause the driver to report the channel as active, desynchronizing
the software state from the actual hardware state?

>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-mcp47feb02_refactor-v4-0-beb2aa3bfd09@microchip.com?part=6