Re: [PATCH v9 8/8] iio: adc: ad4851: add ad485x driver

[David Lechner]

On 12/23/24 6:00 AM, Jonathan Cameron wrote:
> On Fri, 20 Dec 2024 14:01:34 +0200
> Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> wrote:
> 

...

>> +	ret = devm_regulator_get_enable_optional(dev, "vddh");
>> +	if (ret < 0 && ret != -ENODEV)
>> +		return dev_err_probe(dev, ret, "failed to enable vddh voltage\n");
>> +
>> +	ret = devm_regulator_get_enable_optional(dev, "vddl");
>> +	if (ret < 0 && ret != -ENODEV)
>> +		return dev_err_probe(dev, ret, "failed to enable vddl voltage\n");
>> +
>> +	ret = devm_regulator_get_enable_optional(dev, "vrefbuf");
>> +	if (ret < 0 && ret != -ENODEV)
>> +		return dev_err_probe(dev, ret, "failed to enable vrefbuf voltage\n");
>> +
>> +	if (ret > 0)
> 
> I'm fairly sure that call never returns a positive.  Will return 0 for success so I think this
> should be if (ret == 0)

Even better, make it 1 line instead of 4 and use the specific error code we are
checking for:

		st->vrefbuf_en = ret != -ENODEV;
> 
>> +		st->vrefbuf_en = true;
>> +	else
>> +		st->vrefbuf_en = false;
>> +
>> +	ret = devm_regulator_get_enable_optional(dev, "vrefio");
>> +	if (ret < 0 && ret != -ENODEV)
>> +		return dev_err_probe(dev, ret, "failed to enable vrefio voltage\n");
>> +
>> +	if (ret > 0)
> 
> Same here.
  ^
> 
>> +		st->vrefio_en = true;
>> +	else
>> +		st->vrefio_en = false;
>> +
> ...
> 
>> +}
>