* [PATCH v3 0/3] staging: iio: ad7780: correct driver read
@ 2018-11-01 14:42 Renato Lui Geh
2018-11-01 14:43 ` [PATCH v3 1/3] staging: iio: ad7780: fix offset read value Renato Lui Geh
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Renato Lui Geh @ 2018-11-01 14:42 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
giuliano.belinassi
Cc: linux-iio, devel, linux-kernel, kernel-usp
The purpose of this series is to correct two issues in the driver's raw
read function and remove unnecessary struct fields.
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
Renato Lui Geh (3):
staging: iio: ad7780: fix offset read value
staging: iio: ad7780: update voltage on read
staging: iio: ad7780: remove unnecessary stashed voltage value
drivers/staging/iio/adc/ad7780.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v3 1/3] staging: iio: ad7780: fix offset read value 2018-11-01 14:42 [PATCH v3 0/3] staging: iio: ad7780: correct driver read Renato Lui Geh @ 2018-11-01 14:43 ` Renato Lui Geh 2018-11-01 15:02 ` Ardelean, Alexandru 2018-11-01 14:43 ` [PATCH v3 2/3] staging: iio: ad7780: update voltage on read Renato Lui Geh 2018-11-01 14:43 ` [PATCH v3 3/3] staging: iio: ad7780: remove unnecessary stashed voltage value Renato Lui Geh 2 siblings, 1 reply; 14+ messages in thread From: Renato Lui Geh @ 2018-11-01 14:43 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, giuliano.belinassi Cc: linux-iio, devel, linux-kernel, kernel-usp Variable val subtracted an uninitialized value on IIO_CHAN_INFO_OFFSET. This was fixed by assigning the correct value instead. Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> --- drivers/staging/iio/adc/ad7780.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c index b67412db0318..91e016d534ed 100644 --- a/drivers/staging/iio/adc/ad7780.c +++ b/drivers/staging/iio/adc/ad7780.c @@ -96,7 +96,7 @@ static int ad7780_read_raw(struct iio_dev *indio_dev, *val2 = chan->scan_type.realbits - 1; return IIO_VAL_FRACTIONAL_LOG2; case IIO_CHAN_INFO_OFFSET: - *val -= (1 << (chan->scan_type.realbits - 1)); + *val = -(1 << (chan->scan_type.realbits - 1)); return IIO_VAL_INT; } -- 2.19.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/3] staging: iio: ad7780: fix offset read value 2018-11-01 14:43 ` [PATCH v3 1/3] staging: iio: ad7780: fix offset read value Renato Lui Geh @ 2018-11-01 15:02 ` Ardelean, Alexandru 2018-11-03 13:07 ` Jonathan Cameron 0 siblings, 1 reply; 14+ messages in thread From: Ardelean, Alexandru @ 2018-11-01 15:02 UTC (permalink / raw) To: lars, knaack.h, jic23, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh Cc: linux-kernel, linux-iio, devel, kernel-usp Good catch. Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com> On Thu, 2018-11-01 at 11:43 -0300, Renato Lui Geh wrote: > Variable val subtracted an uninitialized value on IIO_CHAN_INFO_OFFSET. > This was fixed by assigning the correct value instead. > > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > --- > drivers/staging/iio/adc/ad7780.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/adc/ad7780.c > b/drivers/staging/iio/adc/ad7780.c > index b67412db0318..91e016d534ed 100644 > --- a/drivers/staging/iio/adc/ad7780.c > +++ b/drivers/staging/iio/adc/ad7780.c > @@ -96,7 +96,7 @@ static int ad7780_read_raw(struct iio_dev *indio_dev, > *val2 = chan->scan_type.realbits - 1; > return IIO_VAL_FRACTIONAL_LOG2; > case IIO_CHAN_INFO_OFFSET: > - *val -= (1 << (chan->scan_type.realbits - 1)); > + *val = -(1 << (chan->scan_type.realbits - 1)); > return IIO_VAL_INT; > } > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/3] staging: iio: ad7780: fix offset read value 2018-11-01 15:02 ` Ardelean, Alexandru @ 2018-11-03 13:07 ` Jonathan Cameron 2018-11-03 15:59 ` Renato Lui Geh 0 siblings, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2018-11-03 13:07 UTC (permalink / raw) To: Ardelean, Alexandru Cc: lars, knaack.h, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh, linux-kernel, linux-iio, devel, kernel-usp On Thu, 1 Nov 2018 15:02:32 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > Good catch. > > Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com> On the basis this has been broken for a long time, and you are clearly doing other nearby not fix work, I'm going to take this through the togreg tree rather than via the quicker fix path. It makes my life easier :) Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > > On Thu, 2018-11-01 at 11:43 -0300, Renato Lui Geh wrote: > > Variable val subtracted an uninitialized value on IIO_CHAN_INFO_OFFSET. > > This was fixed by assigning the correct value instead. > > > > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > > --- > > drivers/staging/iio/adc/ad7780.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/iio/adc/ad7780.c > > b/drivers/staging/iio/adc/ad7780.c > > index b67412db0318..91e016d534ed 100644 > > --- a/drivers/staging/iio/adc/ad7780.c > > +++ b/drivers/staging/iio/adc/ad7780.c > > @@ -96,7 +96,7 @@ static int ad7780_read_raw(struct iio_dev *indio_dev, > > *val2 = chan->scan_type.realbits - 1; > > return IIO_VAL_FRACTIONAL_LOG2; > > case IIO_CHAN_INFO_OFFSET: > > - *val -= (1 << (chan->scan_type.realbits - 1)); > > + *val = -(1 << (chan->scan_type.realbits - 1)); > > return IIO_VAL_INT; > > } > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/3] staging: iio: ad7780: fix offset read value 2018-11-03 13:07 ` Jonathan Cameron @ 2018-11-03 15:59 ` Renato Lui Geh 2018-11-03 17:23 ` Jonathan Cameron 0 siblings, 1 reply; 14+ messages in thread From: Renato Lui Geh @ 2018-11-03 15:59 UTC (permalink / raw) To: Jonathan Cameron Cc: Ardelean, Alexandru, lars, knaack.h, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh, linux-kernel, linux-iio, devel, kernel-usp Hi, >On Thu, 1 Nov 2018 15:02:32 +0000 >"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > >> Good catch. That was actually Jonathan's catch. :) >> Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com> I read up on Acked-by on the kernel docs, as I didn't know exactly what it meant, but I'm not so sure on how to proceed once the patch has been acked. For future patches, do I add this Acked-by line on the patch's message body? Or is this just an informal way of approval? >On the basis this has been broken for a long time, and you are clearly >doing other nearby not fix work, I'm going to take this through the togreg >tree rather than via the quicker fix path. It makes my life >easier :) > >Applied to the togreg branch of iio.git and pushed out as testing for >the autobuilders to play with it. So this means I should skip this patch on v4, right? Thanks, Renato ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/3] staging: iio: ad7780: fix offset read value 2018-11-03 15:59 ` Renato Lui Geh @ 2018-11-03 17:23 ` Jonathan Cameron 0 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2018-11-03 17:23 UTC (permalink / raw) To: Renato Lui Geh Cc: Ardelean, Alexandru, lars, knaack.h, Hennerich, Michael, giuliano.belinassi, pmeerw, gregkh, linux-kernel, linux-iio, devel, kernel-usp On Sat, 3 Nov 2018 12:59:16 -0300 Renato Lui Geh <renatogeh@gmail.com> wrote: > Hi, > > >On Thu, 1 Nov 2018 15:02:32 +0000 > >"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > > > >> Good catch. > > That was actually Jonathan's catch. :) > > >> Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > I read up on Acked-by on the kernel docs, as I didn't know exactly what > it meant, but I'm not so sure on how to proceed once the patch has been > acked. For future patches, do I add this Acked-by line on the patch's > message body? Or is this just an informal way of approval? It's formal. You put the line directly below your Signed-off-by: line if you are resending. If I pick up the patch I paste it in there whilst applying. > > >On the basis this has been broken for a long time, and you are clearly > >doing other nearby not fix work, I'm going to take this through the togreg > >tree rather than via the quicker fix path. It makes my life > >easier :) > > > >Applied to the togreg branch of iio.git and pushed out as testing for > >the autobuilders to play with it. > > So this means I should skip this patch on v4, right? Yes. Already in the tree so don't send it again. Thanks Jonathan > > Thanks, > Renato > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 2/3] staging: iio: ad7780: update voltage on read 2018-11-01 14:42 [PATCH v3 0/3] staging: iio: ad7780: correct driver read Renato Lui Geh 2018-11-01 14:43 ` [PATCH v3 1/3] staging: iio: ad7780: fix offset read value Renato Lui Geh @ 2018-11-01 14:43 ` Renato Lui Geh 2018-11-01 15:20 ` Ardelean, Alexandru 2018-11-01 14:43 ` [PATCH v3 3/3] staging: iio: ad7780: remove unnecessary stashed voltage value Renato Lui Geh 2 siblings, 1 reply; 14+ messages in thread From: Renato Lui Geh @ 2018-11-01 14:43 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, giuliano.belinassi Cc: linux-iio, devel, linux-kernel, kernel-usp 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 <renatogeh@gmail.com> --- Changes in v3: - removed initialization (int voltage_uv = 0) - returns error when voltage_uv is null 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 91e016d534ed..f2a11e9424cd 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) + return -EINVAL; + *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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] staging: iio: ad7780: update voltage on read 2018-11-01 14:43 ` [PATCH v3 2/3] staging: iio: ad7780: update voltage on read Renato Lui Geh @ 2018-11-01 15:20 ` Ardelean, Alexandru 2018-11-03 13:10 ` Jonathan Cameron 0 siblings, 1 reply; 14+ messages in thread From: Ardelean, Alexandru @ 2018-11-01 15:20 UTC (permalink / raw) To: lars, knaack.h, jic23, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh Cc: linux-kernel, linux-iio, devel, kernel-usp On Thu, 2018-11-01 at 11:43 -0300, 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. > > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > --- > Changes in v3: > - removed initialization (int voltage_uv = 0) > - returns error when voltage_uv is null > > 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 91e016d534ed..f2a11e9424cd 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) This looks wrong. I admit this was done in the same way in the probe function, but that looks a bit wrong as well. Typically, the return value of `regulator_get_voltage()` would get checked with: ret = regulator_get_voltage(st->reg); if (ret < 0) return ret; *val = ret / 1000; So, negative values are errors and zero & positive values are valid voltage values. > + return -EINVAL; > + *val = (voltage_uv / 1000) * st->gain; > *val2 = chan->scan_type.realbits - 1; > return IIO_VAL_FRACTIONAL_LOG2; > case IIO_CHAN_INFO_OFFSET: ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] staging: iio: ad7780: update voltage on read 2018-11-01 15:20 ` Ardelean, Alexandru @ 2018-11-03 13:10 ` Jonathan Cameron 2018-11-03 16:06 ` Renato Lui Geh 0 siblings, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2018-11-03 13:10 UTC (permalink / raw) To: Ardelean, Alexandru Cc: lars, knaack.h, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh, linux-kernel, linux-iio, devel, kernel-usp On Thu, 1 Nov 2018 15:20:55 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > On Thu, 2018-11-01 at 11:43 -0300, 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. > > > > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > > --- > > Changes in v3: > > - removed initialization (int voltage_uv = 0) > > - returns error when voltage_uv is null > > > > 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 91e016d534ed..f2a11e9424cd 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) > > This looks wrong. > I admit this was done in the same way in the probe function, but that looks > a bit wrong as well. > > Typically, the return value of `regulator_get_voltage()` would get checked > with: > > ret = regulator_get_voltage(st->reg); > if (ret < 0) > return ret; > *val = ret / 1000; > > So, negative values are errors and zero & positive values are valid voltage > values. This one is a stylistic choice for readability. I ever so slightly prefer how Alex has it but don't care enough that I'd have commented on it ;) However, nice to tidy up as you'll be respinning patch 3 anyway! Thanks, Jonathan > > > + return -EINVAL; > > + *val = (voltage_uv / 1000) * st->gain; > > *val2 = chan->scan_type.realbits - 1; > > return IIO_VAL_FRACTIONAL_LOG2; > > case IIO_CHAN_INFO_OFFSET: ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] staging: iio: ad7780: update voltage on read 2018-11-03 13:10 ` Jonathan Cameron @ 2018-11-03 16:06 ` Renato Lui Geh 2018-11-03 17:21 ` Jonathan Cameron 0 siblings, 1 reply; 14+ messages in thread From: Renato Lui Geh @ 2018-11-03 16:06 UTC (permalink / raw) To: Jonathan Cameron Cc: Ardelean, Alexandru, lars, knaack.h, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh, linux-kernel, linux-iio, devel, kernel-usp On Thu, 1 Nov 2018 15:20:55 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > > This looks wrong. > I admit this was done in the same way in the probe function, but that looks > a bit wrong as well. > > Typically, the return value of `regulator_get_voltage()` would get checked > with: > > ret = regulator_get_voltage(st->reg); > if (ret < 0) > return ret; > *val = ret / 1000; > > So, negative values are errors and zero & positive values are valid voltage > values. I see. So -EINVAL is only used when sent the wrong info type? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] staging: iio: ad7780: update voltage on read 2018-11-03 16:06 ` Renato Lui Geh @ 2018-11-03 17:21 ` Jonathan Cameron 0 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2018-11-03 17:21 UTC (permalink / raw) To: Renato Lui Geh Cc: Ardelean, Alexandru, lars, knaack.h, Hennerich, Michael, giuliano.belinassi, pmeerw, gregkh, linux-kernel, linux-iio, devel, kernel-usp On Sat, 3 Nov 2018 13:06:19 -0300 Renato Lui Geh <renatogeh@gmail.com> wrote: > On Thu, 1 Nov 2018 15:20:55 +0000 > "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > > > > This looks wrong. > > I admit this was done in the same way in the probe function, but that looks > > a bit wrong as well. > > > > Typically, the return value of `regulator_get_voltage()` would get checked > > with: > > > > ret = regulator_get_voltage(st->reg); > > if (ret < 0) > > return ret; > > *val = ret / 1000; > > > > So, negative values are errors and zero & positive values are valid voltage > > values. > > I see. So -EINVAL is only used when sent the wrong info type? yes. I actually misread what was there and thought we were just talking about using a ret variable rather than returning the error via your local variable. Definitely want to pass on the error from regulator_get_voltage as it may have more meaning than a simple -EINVAL. Thanks, Jonathan ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/3] staging: iio: ad7780: remove unnecessary stashed voltage value 2018-11-01 14:42 [PATCH v3 0/3] staging: iio: ad7780: correct driver read Renato Lui Geh 2018-11-01 14:43 ` [PATCH v3 1/3] staging: iio: ad7780: fix offset read value Renato Lui Geh 2018-11-01 14:43 ` [PATCH v3 2/3] staging: iio: ad7780: update voltage on read Renato Lui Geh @ 2018-11-01 14:43 ` Renato Lui Geh 2018-11-01 15:28 ` Ardelean, Alexandru 2 siblings, 1 reply; 14+ messages in thread From: Renato Lui Geh @ 2018-11-01 14:43 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, giuliano.belinassi Cc: linux-iio, devel, linux-kernel, kernel-usp This patch removes the unnecessary field int_vref_mv in ad7780_state referring to the device's voltage. Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> --- Changes in v3: - removed unnecessary int_vref_mv from ad7780_state drivers/staging/iio/adc/ad7780.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c index f2a11e9424cd..f250370efcf7 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; }; @@ -190,9 +189,7 @@ static int ad7780_probe(struct spi_device *spi) 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 + if (!voltage_uv) dev_warn(&spi->dev, "Reference voltage unspecified\n"); spi_set_drvdata(spi, indio_dev); -- 2.19.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] staging: iio: ad7780: remove unnecessary stashed voltage value 2018-11-01 14:43 ` [PATCH v3 3/3] staging: iio: ad7780: remove unnecessary stashed voltage value Renato Lui Geh @ 2018-11-01 15:28 ` Ardelean, Alexandru 2018-11-03 13:10 ` Jonathan Cameron 0 siblings, 1 reply; 14+ messages in thread From: Ardelean, Alexandru @ 2018-11-01 15:28 UTC (permalink / raw) To: lars, knaack.h, jic23, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh Cc: linux-kernel, linux-iio, devel, kernel-usp On Thu, 2018-11-01 at 11:43 -0300, Renato Lui Geh wrote: > This patch removes the unnecessary field int_vref_mv in ad7780_state > referring to the device's voltage. > > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > --- > Changes in v3: > - removed unnecessary int_vref_mv from ad7780_state > > drivers/staging/iio/adc/ad7780.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/staging/iio/adc/ad7780.c > b/drivers/staging/iio/adc/ad7780.c > index f2a11e9424cd..f250370efcf7 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; > }; > @@ -190,9 +189,7 @@ static int ad7780_probe(struct spi_device *spi) > 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 > + if (!voltage_uv) > dev_warn(&spi->dev, "Reference voltage unspecified\n"); I think you could remove this altogether, and also remove the entire `voltage_uv = regulator_get_voltage(st->reg);` assignment. It doesn't make much sense to read the voltage here, since it won't be used in the probe part anymore. > > spi_set_drvdata(spi, indio_dev); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] staging: iio: ad7780: remove unnecessary stashed voltage value 2018-11-01 15:28 ` Ardelean, Alexandru @ 2018-11-03 13:10 ` Jonathan Cameron 0 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2018-11-03 13:10 UTC (permalink / raw) To: Ardelean, Alexandru Cc: lars, knaack.h, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh, linux-kernel, linux-iio, devel, kernel-usp On Thu, 1 Nov 2018 15:28:19 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > On Thu, 2018-11-01 at 11:43 -0300, Renato Lui Geh wrote: > > This patch removes the unnecessary field int_vref_mv in ad7780_state > > referring to the device's voltage. > > > > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > > --- > > Changes in v3: > > - removed unnecessary int_vref_mv from ad7780_state > > > > drivers/staging/iio/adc/ad7780.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/staging/iio/adc/ad7780.c > > b/drivers/staging/iio/adc/ad7780.c > > index f2a11e9424cd..f250370efcf7 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; > > }; > > @@ -190,9 +189,7 @@ static int ad7780_probe(struct spi_device *spi) > > 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 > > + if (!voltage_uv) > > dev_warn(&spi->dev, "Reference voltage unspecified\n"); > > I think you could remove this altogether, and also remove the entire > `voltage_uv = regulator_get_voltage(st->reg);` assignment. > > It doesn't make much sense to read the voltage here, since it won't be used > in the probe part anymore. > Absolutely agree on this. There is no point in reading here at all. Jonathan > > > > spi_set_drvdata(spi, indio_dev); ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-11-03 17:23 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-01 14:42 [PATCH v3 0/3] staging: iio: ad7780: correct driver read Renato Lui Geh 2018-11-01 14:43 ` [PATCH v3 1/3] staging: iio: ad7780: fix offset read value Renato Lui Geh 2018-11-01 15:02 ` Ardelean, Alexandru 2018-11-03 13:07 ` Jonathan Cameron 2018-11-03 15:59 ` Renato Lui Geh 2018-11-03 17:23 ` Jonathan Cameron 2018-11-01 14:43 ` [PATCH v3 2/3] staging: iio: ad7780: update voltage on read Renato Lui Geh 2018-11-01 15:20 ` Ardelean, Alexandru 2018-11-03 13:10 ` Jonathan Cameron 2018-11-03 16:06 ` Renato Lui Geh 2018-11-03 17:21 ` Jonathan Cameron 2018-11-01 14:43 ` [PATCH v3 3/3] staging: iio: ad7780: remove unnecessary stashed voltage value Renato Lui Geh 2018-11-01 15:28 ` Ardelean, Alexandru 2018-11-03 13:10 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).