Re: [PATCH v3] iio: dac: mcp47feb02: Fix passing uninitialized vref1_uV for no Vref1 case

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, Jun 03, 2026 at 05:44:48PM +0300, Ariana Lazar wrote:
> Ensure that if a device has Vref1 but reading the regulator returns an
> error, mcp47feb02_init_ctrl_regs() is not called with an uninitialized
> vref1_uV value.
> 
> Also add a device_property_present() check for the Vref1 supply before
> reading the regulator.

...

> +static void mcp47feb02_regulator_disable(void *d)
> +{
> +	struct regulator *reg = (struct regulator *)d;

Unneeded casting, then for the maintenance it's better to have assignment and
definition to be split as we have a validation check.

> +	if (reg)
> +		regulator_disable(reg);
> +}

With the above being said, there are two alternatives:

static void mcp47feb02_regulator_disable(void *d)
{
	struct regulator *reg;

	reg = d;
	if (reg)
		regulator_disable(reg);
}

OR (my preference)

static void mcp47feb02_regulator_disable(void *reg)
{
	if (reg)
		regulator_disable(reg);
}

...

> +static bool mcp47feb02_ref_mismatch(struct mcp47feb02_data *data, unsigned int ch)
> +{
> +	bool use_vref, use_bandgap;
> +
> +	if (data->chdata[ch].ref_mode == MCP47FEB02_VREF_VDD)
> +		return false;
> +
> +	use_vref = (data->phys_channels >= 4 && (ch % 2)) ?  data->use_vref1 : data->use_vref;

Too many spaces.

> +	use_bandgap = (data->chdata[ch].ref_mode == MCP47FEB02_INTERNAL_BAND_GAP);

(Unneeded parentheses.)

> +	return use_vref == use_bandgap;

I would rewrite this as

	bool use_bandgap;

	if (data->chdata[ch].ref_mode == MCP47FEB02_VREF_VDD)
		return false;

	use_bandgap = data->chdata[ch].ref_mode == MCP47FEB02_INTERNAL_BAND_GAP;

	if (data->phys_channels >= 4 && (ch % 2))
		return data->use_vref1 == use_bandgap;
	else // redundant, but left for the formatting purposes
		return data->use_vref == use_bandgap;

> +}

...

> +				data->phys_channels >= 4 && (i % 2)) {

This idiom is repeated so many times that I would rather see

static inline bool is_...(data, ch)
{
	return data->phys_channels >= 4 && (ch % 2);
}

and be used everywhere. If required, add also a comment on top to explain the logic,
why we need it to be done this way.

-- 
With Best Regards,
Andy Shevchenko