The purpose of this series is to correct an issue in the driver's raw
read function and remove an unnecessary struct field.
Changelog:
*v2
- separated original patch into two patches
(https://marc.info/?l=linux-iio&m=154047435605492)
*v3
- reordered patches so that fixes go first
- removed unnecessary initialization
- removed unnecessary voltage field variable
- dropped reading voltage on probe
- returns -EINVAL error on null voltage
*v4
- removed voltage reading from probe
- fixed voltage error handling
Renato Lui Geh (2):
staging: iio: ad7780: update voltage on read
staging: iio: ad7780: remove unnecessary stashed voltage value
drivers/staging/iio/adc/ad7780.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
--
2.19.1
The ad7780 driver previously did not read the correct device output, as
it read an outdated value set at initialization. It now updates its
voltage on read.
Signed-off-by: Renato Lui Geh <[email protected]>
---
Changes in v3:
- removed initialization (int voltage_uv = 0)
- returns error when voltage_uv is null
Changes in v4:
- returns error when voltage_uv is negative
drivers/staging/iio/adc/ad7780.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
index b67412db0318..c7cb05cedbbc 100644
--- a/drivers/staging/iio/adc/ad7780.c
+++ b/drivers/staging/iio/adc/ad7780.c
@@ -87,12 +87,16 @@ static int ad7780_read_raw(struct iio_dev *indio_dev,
long m)
{
struct ad7780_state *st = iio_priv(indio_dev);
+ int voltage_uv;
switch (m) {
case IIO_CHAN_INFO_RAW:
return ad_sigma_delta_single_conversion(indio_dev, chan, val);
case IIO_CHAN_INFO_SCALE:
- *val = st->int_vref_mv * st->gain;
+ voltage_uv = regulator_get_voltage(st->reg);
+ if (voltage_uv < 0)
+ return voltage_uv;
+ *val = (voltage_uv / 1000) * st->gain;
*val2 = chan->scan_type.realbits - 1;
return IIO_VAL_FRACTIONAL_LOG2;
case IIO_CHAN_INFO_OFFSET:
--
2.19.1
This patch removes the unnecessary field int_vref_mv in ad7780_state
referring to the device's voltage.
Signed-off-by: Renato Lui Geh <[email protected]>
---
Changes in v3:
- removed unnecessary int_vref_mv from ad7780_state
Changes in v4:
- removed voltage reading on probe
drivers/staging/iio/adc/ad7780.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
index c7cb05cedbbc..52a914360574 100644
--- a/drivers/staging/iio/adc/ad7780.c
+++ b/drivers/staging/iio/adc/ad7780.c
@@ -42,7 +42,6 @@ struct ad7780_state {
struct regulator *reg;
struct gpio_desc *powerdown_gpio;
unsigned int gain;
- u16 int_vref_mv;
struct ad_sigma_delta sd;
};
@@ -165,7 +164,7 @@ static int ad7780_probe(struct spi_device *spi)
{
struct ad7780_state *st;
struct iio_dev *indio_dev;
- int ret, voltage_uv = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
if (!indio_dev)
@@ -185,16 +184,10 @@ static int ad7780_probe(struct spi_device *spi)
dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
return ret;
}
- voltage_uv = regulator_get_voltage(st->reg);
st->chip_info =
&ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data];
- if (voltage_uv)
- st->int_vref_mv = voltage_uv / 1000;
- else
- dev_warn(&spi->dev, "Reference voltage unspecified\n");
-
spi_set_drvdata(spi, indio_dev);
indio_dev->dev.parent = &spi->dev;
--
2.19.1
On Mon, 2018-11-05 at 17:16 -0200, Renato Lui Geh wrote:
> This patch removes the unnecessary field int_vref_mv in ad7780_state
> referring to the device's voltage.
>
Looks good from my side.
Alex
> Signed-off-by: Renato Lui Geh <[email protected]>
> ---
> Changes in v3:
> - removed unnecessary int_vref_mv from ad7780_state
> Changes in v4:
> - removed voltage reading on probe
>
> drivers/staging/iio/adc/ad7780.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index c7cb05cedbbc..52a914360574 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -42,7 +42,6 @@ struct ad7780_state {
> struct regulator *reg;
> struct gpio_desc *powerdown_gpio;
> unsigned int gain;
> - u16 int_vref_mv;
>
> struct ad_sigma_delta sd;
> };
> @@ -165,7 +164,7 @@ static int ad7780_probe(struct spi_device *spi)
> {
> struct ad7780_state *st;
> struct iio_dev *indio_dev;
> - int ret, voltage_uv = 0;
> + int ret;
>
> indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> if (!indio_dev)
> @@ -185,16 +184,10 @@ static int ad7780_probe(struct spi_device *spi)
> dev_err(&spi->dev, "Failed to enable specified AVdd
> supply\n");
> return ret;
> }
> - voltage_uv = regulator_get_voltage(st->reg);
>
> st->chip_info =
> &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data];
>
> - if (voltage_uv)
> - st->int_vref_mv = voltage_uv / 1000;
> - else
> - dev_warn(&spi->dev, "Reference voltage unspecified\n");
> -
> spi_set_drvdata(spi, indio_dev);
>
> indio_dev->dev.parent = &spi->dev;
On Mon, 2018-11-05 at 17:14 -0200, Renato Lui Geh wrote:
> The ad7780 driver previously did not read the correct device output, as
> it read an outdated value set at initialization. It now updates its
> voltage on read.
>
Looks good from my side.
Alex
> Signed-off-by: Renato Lui Geh <[email protected]>
> ---
> Changes in v3:
> - removed initialization (int voltage_uv = 0)
> - returns error when voltage_uv is null
> Changes in v4:
> - returns error when voltage_uv is negative
>
> drivers/staging/iio/adc/ad7780.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index b67412db0318..c7cb05cedbbc 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -87,12 +87,16 @@ static int ad7780_read_raw(struct iio_dev *indio_dev,
> long m)
> {
> struct ad7780_state *st = iio_priv(indio_dev);
> + int voltage_uv;
>
> switch (m) {
> case IIO_CHAN_INFO_RAW:
> return ad_sigma_delta_single_conversion(indio_dev, chan,
> val);
> case IIO_CHAN_INFO_SCALE:
> - *val = st->int_vref_mv * st->gain;
> + voltage_uv = regulator_get_voltage(st->reg);
> + if (voltage_uv < 0)
> + return voltage_uv;
> + *val = (voltage_uv / 1000) * st->gain;
> *val2 = chan->scan_type.realbits - 1;
> return IIO_VAL_FRACTIONAL_LOG2;
> case IIO_CHAN_INFO_OFFSET:
On Tue, 6 Nov 2018 09:24:44 +0000
"Ardelean, Alexandru" <[email protected]> wrote:
> On Mon, 2018-11-05 at 17:14 -0200, Renato Lui Geh wrote:
> > The ad7780 driver previously did not read the correct device output, as
> > it read an outdated value set at initialization. It now updates its
> > voltage on read.
> >
>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to check it. I'm not that fussed about pushing this
one as a fix, as most of the time these things are on fixed regulators
anyway, but it is nice to do it right.
> Looks good from my side.
Ack?
Much preferred if you are willing to give a formal acknowledgement.
Thanks,
Jonathan
>
> Alex
>
> > Signed-off-by: Renato Lui Geh <[email protected]>
> > ---
> > Changes in v3:
> > - removed initialization (int voltage_uv = 0)
> > - returns error when voltage_uv is null
> > Changes in v4:
> > - returns error when voltage_uv is negative
> >
> > drivers/staging/iio/adc/ad7780.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7780.c
> > b/drivers/staging/iio/adc/ad7780.c
> > index b67412db0318..c7cb05cedbbc 100644
> > --- a/drivers/staging/iio/adc/ad7780.c
> > +++ b/drivers/staging/iio/adc/ad7780.c
> > @@ -87,12 +87,16 @@ static int ad7780_read_raw(struct iio_dev *indio_dev,
> > long m)
> > {
> > struct ad7780_state *st = iio_priv(indio_dev);
> > + int voltage_uv;
> >
> > switch (m) {
> > case IIO_CHAN_INFO_RAW:
> > return ad_sigma_delta_single_conversion(indio_dev, chan,
> > val);
> > case IIO_CHAN_INFO_SCALE:
> > - *val = st->int_vref_mv * st->gain;
> > + voltage_uv = regulator_get_voltage(st->reg);
> > + if (voltage_uv < 0)
> > + return voltage_uv;
> > + *val = (voltage_uv / 1000) * st->gain;
> > *val2 = chan->scan_type.realbits - 1;
> > return IIO_VAL_FRACTIONAL_LOG2;
> > case IIO_CHAN_INFO_OFFSET:
On Tue, 6 Nov 2018 09:25:27 +0000
"Ardelean, Alexandru" <[email protected]> wrote:
> On Mon, 2018-11-05 at 17:16 -0200, Renato Lui Geh wrote:
> > This patch removes the unnecessary field int_vref_mv in ad7780_state
> > referring to the device's voltage.
> >
>
> Looks good from my side.
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
>
> Alex
>
> > Signed-off-by: Renato Lui Geh <[email protected]>
> > ---
> > Changes in v3:
> > - removed unnecessary int_vref_mv from ad7780_state
> > Changes in v4:
> > - removed voltage reading on probe
> >
> > drivers/staging/iio/adc/ad7780.c | 9 +--------
> > 1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7780.c
> > b/drivers/staging/iio/adc/ad7780.c
> > index c7cb05cedbbc..52a914360574 100644
> > --- a/drivers/staging/iio/adc/ad7780.c
> > +++ b/drivers/staging/iio/adc/ad7780.c
> > @@ -42,7 +42,6 @@ struct ad7780_state {
> > struct regulator *reg;
> > struct gpio_desc *powerdown_gpio;
> > unsigned int gain;
> > - u16 int_vref_mv;
> >
> > struct ad_sigma_delta sd;
> > };
> > @@ -165,7 +164,7 @@ static int ad7780_probe(struct spi_device *spi)
> > {
> > struct ad7780_state *st;
> > struct iio_dev *indio_dev;
> > - int ret, voltage_uv = 0;
> > + int ret;
> >
> > indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > if (!indio_dev)
> > @@ -185,16 +184,10 @@ static int ad7780_probe(struct spi_device *spi)
> > dev_err(&spi->dev, "Failed to enable specified AVdd
> > supply\n");
> > return ret;
> > }
> > - voltage_uv = regulator_get_voltage(st->reg);
> >
> > st->chip_info =
> > &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data];
> >
> > - if (voltage_uv)
> > - st->int_vref_mv = voltage_uv / 1000;
> > - else
> > - dev_warn(&spi->dev, "Reference voltage unspecified\n");
> > -
> > spi_set_drvdata(spi, indio_dev);
> >
> > indio_dev->dev.parent = &spi->dev;
On Sun, 2018-11-11 at 14:30 +0000, Jonathan Cameron wrote:
> On Tue, 6 Nov 2018 09:24:44 +0000
> "Ardelean, Alexandru" <[email protected]> wrote:
>
> > On Mon, 2018-11-05 at 17:14 -0200, Renato Lui Geh wrote:
> > > The ad7780 driver previously did not read the correct device output,
> > > as
> > > it read an outdated value set at initialization. It now updates its
> > > voltage on read.
> > >
>
> Applied to the togreg branch of iio.git and pushed out as testing for
> the autobuilders to check it. I'm not that fussed about pushing this
> one as a fix, as most of the time these things are on fixed regulators
> anyway, but it is nice to do it right.
>
> > Looks good from my side.
>
> Ack?
Acked-by: Alexandru Ardelean <[email protected]>
>
> Much preferred if you are willing to give a formal acknowledgement.
>
> Thanks,
>
> Jonathan
>
>
> >
> > Alex
> >
> > > Signed-off-by: Renato Lui Geh <[email protected]>
> > > ---
> > > Changes in v3:
> > > - removed initialization (int voltage_uv = 0)
> > > - returns error when voltage_uv is null
> > > Changes in v4:
> > > - returns error when voltage_uv is negative
> > >
> > > drivers/staging/iio/adc/ad7780.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7780.c
> > > b/drivers/staging/iio/adc/ad7780.c
> > > index b67412db0318..c7cb05cedbbc 100644
> > > --- a/drivers/staging/iio/adc/ad7780.c
> > > +++ b/drivers/staging/iio/adc/ad7780.c
> > > @@ -87,12 +87,16 @@ static int ad7780_read_raw(struct iio_dev
> > > *indio_dev,
> > > long m)
> > > {
> > > struct ad7780_state *st = iio_priv(indio_dev);
> > > + int voltage_uv;
> > >
> > > switch (m) {
> > > case IIO_CHAN_INFO_RAW:
> > > return ad_sigma_delta_single_conversion(indio_dev, chan,
> > > val);
> > > case IIO_CHAN_INFO_SCALE:
> > > - *val = st->int_vref_mv * st->gain;
> > > + voltage_uv = regulator_get_voltage(st->reg);
> > > + if (voltage_uv < 0)
> > > + return voltage_uv;
> > > + *val = (voltage_uv / 1000) * st->gain;
> > > *val2 = chan->scan_type.realbits - 1;
> > > return IIO_VAL_FRACTIONAL_LOG2;
> > > case IIO_CHAN_INFO_OFFSET:
>
>
On Mon, 12 Nov 2018 07:57:24 +0000
"Ardelean, Alexandru" <[email protected]> wrote:
> On Sun, 2018-11-11 at 14:30 +0000, Jonathan Cameron wrote:
> > On Tue, 6 Nov 2018 09:24:44 +0000
> > "Ardelean, Alexandru" <[email protected]> wrote:
> >
> > > On Mon, 2018-11-05 at 17:14 -0200, Renato Lui Geh wrote:
> > > > The ad7780 driver previously did not read the correct device output,
> > > > as
> > > > it read an outdated value set at initialization. It now updates its
> > > > voltage on read.
> > > >
> >
> > Applied to the togreg branch of iio.git and pushed out as testing for
> > the autobuilders to check it. I'm not that fussed about pushing this
> > one as a fix, as most of the time these things are on fixed regulators
> > anyway, but it is nice to do it right.
> >
> > > Looks good from my side.
> >
> > Ack?
>
> Acked-by: Alexandru Ardelean <[email protected]>
Added to both in this series.
Thanks,
Jonathan
>
> >
> > Much preferred if you are willing to give a formal acknowledgement.
> >
> > Thanks,
> >
> > Jonathan
> >
> >
> > >
> > > Alex
> > >
> > > > Signed-off-by: Renato Lui Geh <[email protected]>
> > > > ---
> > > > Changes in v3:
> > > > - removed initialization (int voltage_uv = 0)
> > > > - returns error when voltage_uv is null
> > > > Changes in v4:
> > > > - returns error when voltage_uv is negative
> > > >
> > > > drivers/staging/iio/adc/ad7780.c | 6 +++++-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/staging/iio/adc/ad7780.c
> > > > b/drivers/staging/iio/adc/ad7780.c
> > > > index b67412db0318..c7cb05cedbbc 100644
> > > > --- a/drivers/staging/iio/adc/ad7780.c
> > > > +++ b/drivers/staging/iio/adc/ad7780.c
> > > > @@ -87,12 +87,16 @@ static int ad7780_read_raw(struct iio_dev
> > > > *indio_dev,
> > > > long m)
> > > > {
> > > > struct ad7780_state *st = iio_priv(indio_dev);
> > > > + int voltage_uv;
> > > >
> > > > switch (m) {
> > > > case IIO_CHAN_INFO_RAW:
> > > > return ad_sigma_delta_single_conversion(indio_dev, chan,
> > > > val);
> > > > case IIO_CHAN_INFO_SCALE:
> > > > - *val = st->int_vref_mv * st->gain;
> > > > + voltage_uv = regulator_get_voltage(st->reg);
> > > > + if (voltage_uv < 0)
> > > > + return voltage_uv;
> > > > + *val = (voltage_uv / 1000) * st->gain;
> > > > *val2 = chan->scan_type.realbits - 1;
> > > > return IIO_VAL_FRACTIONAL_LOG2;
> > > > case IIO_CHAN_INFO_OFFSET:
> >
> >