Re: [PATCH v4 2/4] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs

[Marcelo Schmitt]

> > >>> +static int ltc2378_regulator_setup(struct device *dev, struct ltc2378_state *st)
> > >>> +{
> > >>> +	int ret;
> > >>> +
> > >>> +	ret = devm_regulator_get_enable_read_voltage(dev, "refin");
> > >>> +	if (ret < 0 && ret != -ENODEV) {
> > >>> +		return dev_err_probe(dev, ret, "failed to read refin regulator\n");
> > >>> +	} else if (ret > 0) {
> > >>
> > >> Else is not needed here.
> > > Why not?
> > 
> > The if returns unconditionally, so else is not needed. This is just a general
> > principal.
> 
> Unfortunately in this case ENODEV will go further, I don't see full context,
> but it may lead to something unexpected.
> 
> Personally I prefer the ladder of
> 
> 	if (ret == -ENODEV)
> 		...
> 	else if (ret < 0)
> 		...
> 	else
> 		...
> 
After splitting regulator availability check on a chip specific basis it will
probably simplify to something like

static int ltc2378_refin_setup(struct device *dev, struct ltc2378_state *st)
{
	int ret;

	ret = devm_regulator_get_enable_read_voltage(dev, "refin");
	if (ret == -ENODEV) /* refin is optional */
		st->ref_uV = st->info->internal_ref_uv * 2; /* ref buf amplifies REFIN by a factor of 2 */
	else if (ret < 0)
		return dev_err_probe(...);
	else
		st->ref_uV = ret * 2; /* ref buf amplifies REFIN by a factor of 2 */
	
	return 0;
}

For 'ref' only parts it will probably look like previous versions

static int ltc2378_ref_setup(struct device *dev, struct ltc2378_state *st)
{
	ret = devm_regulator_get_enable_read_voltage(dev, "ref"); /* ref is required */
	if (ret < 0)
		return dev_err_probe(dev, ret, "failed to read ref regulator\n");
	
	st->ref_uV = ret;

	return 0;
}

Calling the appropriate ref setup function for each part shall then (hopefully) 
error out incorrect device trees.