| Message ID | 20210908152525.2946785-1-florian.boor@kernelconcepts.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/2] iio: adc: ad799x: Implement selecting external reference voltage input on AD7991, AD7995 and AD7999. | expand |
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() > + } > if (IS_ERR(st->vref)) { > ret = PTR_ERR(st->vref); > goto error_disable_reg; > @@ -806,6 +816,13 @@ static int ad799x_probe(struct i2c_client *client, > if (ret) > goto error_disable_reg; > > + /* use external reference voltage, optional if regulator present */ > + if (vref_external && > + ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999))) { > + dev_info(&client->dev, "Using external reference voltage\n"); > + extra_config |= AD7991_REF_SEL; > + } > + > st->client = client; > > indio_dev->name = id->name; > @@ -815,7 +832,7 @@ static int ad799x_probe(struct i2c_client *client, > indio_dev->channels = st->chip_config->channel; > indio_dev->num_channels = chip_info->num_channels; > > - ret = ad799x_update_config(st, st->chip_config->default_config); > + ret = ad799x_update_config(st, st->chip_config->default_config | extra_config); > if (ret) > goto error_disable_vref; >
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
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"); + } if (IS_ERR(st->vref)) { ret = PTR_ERR(st->vref); goto error_disable_reg; @@ -806,6 +816,13 @@ static int ad799x_probe(struct i2c_client *client, if (ret) goto error_disable_reg; + /* use external reference voltage, optional if regulator present */ + if (vref_external && + ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999))) { + dev_info(&client->dev, "Using external reference voltage\n"); + extra_config |= AD7991_REF_SEL; + } + st->client = client; indio_dev->name = id->name; @@ -815,7 +832,7 @@ static int ad799x_probe(struct i2c_client *client, indio_dev->channels = st->chip_config->channel; indio_dev->num_channels = chip_info->num_channels; - ret = ad799x_update_config(st, st->chip_config->default_config); + ret = ad799x_update_config(st, st->chip_config->default_config | extra_config); if (ret) goto error_disable_vref;
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> --- 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(-)