Re: [PATCH v2 1/2] iio: adc: ad799x: Implement selecting external reference voltage input on AD7991, AD7995 and AD7999.

[Florian Boor <florian.boor@kernelconcepts.de>]

Hi Jonathan,

On 11.09.21 18:43, Jonathan Cameron wrote:
> On Wed,  8 Sep 2021 17:25:24 +0200
> Florian Boor <florian.boor@kernelconcepts.de> wrote:
> 
>> Make use of the AD7991_REF_SEL bit and support using the external
>> reference voltage if 'vref-supply' is present.
>>
>> Signed-off-by: Florian Boor <florian.boor@kernelconcepts.de>
> 
> Hi Florian,
> 
>> ---
>>
>> Changes in v2:
>> - Check if a provided external vref regulator is provided.
>> - Drop unused setting
>> - Add ad79xx documentation (second patch)
>>
>>  drivers/iio/adc/ad799x.c | 21 +++++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
>> index 18bf8386d50a..2ff926a4e9b3 100644
>> --- a/drivers/iio/adc/ad799x.c
>> +++ b/drivers/iio/adc/ad799x.c
>> @@ -770,6 +770,8 @@ static int ad799x_probe(struct i2c_client *client,
>>  				   const struct i2c_device_id *id)
>>  {
>>  	int ret;
>> +	int extra_config = 0;
>> +	bool vref_external = true;
>>  	struct ad799x_state *st;
>>  	struct iio_dev *indio_dev;
>>  	const struct ad799x_chip_info *chip_info =
>> @@ -797,7 +799,15 @@ static int ad799x_probe(struct i2c_client *client,
>>  	ret = regulator_enable(st->reg);
>>  	if (ret)
>>  		return ret;
>> -	st->vref = devm_regulator_get(&client->dev, "vref");
>> +		
>> +	/* check if an external reference is supplied */
>> +	st->vref = devm_regulator_get_optional(&client->dev, "vref");
>> +
>> +	if (PTR_ERR(st->vref) == -ENODEV) {
>> +		vref_external = false;
>> +		/* get dummy */
>> +		st->vref = devm_regulator_get(&client->dev, "vref");
> 
> Why?  Instead of doing this add if (st->vref) around the regulator
> enable and disable.  We don't want to pretend there is a regulator when
> there isn't one connected and we are using VDD as the reference.
> 
> If we are in that mode, we need to change which regulator is read in
> read_raw()

I have to admit I wondered about how this is handled as well. My idea was not to
change it at this point and improve this later in a separate patch.
But I see the point - I'll add this change as well.

Greetings

Florian

-- 
The dream of yesterday                  Florian Boor
is the hope of today                    Tel: +49(0) 271-338857-15
and the reality of tomorrow.		Fax: +49(0) 271-338857-29
[Robert Hutchings Goddard, 1904]        florian.boor@kernelconcepts.de
                                        http://www.kernelconcepts.de/en

kernel concepts GmbH
Hauptstraße 16
57074 Siegen
Deutschland
Geschäftsführer: Ole Reinhardt
HR Siegen, HR B 9613