staging: iio: ad7780: correct driver read
Subsystems (4)
v4 cover staging: iio: ad7780: correct driver read
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(-)
Signature · 1 line
2.19.1
PATCH v4 1/2 staging: iio: ad7780: update voltage on read
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.
---
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(-)
drivers/staging/iio/adc/ad7780.c +0 -0
@@ -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:
Signature · 1 line
2.19.1
Re: [PATCH v4 1/2] staging: iio: ad7780: update voltage on read
T24gTW9uLCAyMDE4LTExLTA1IGF0IDE3OjE0IC0wMjAwLCBSZW5hdG8gTHVpIEdlaCB3cm90ZToN Cj4gVGhlIGFkNzc4MCBkcml2ZXIgcHJldmlvdXNseSBkaWQgbm90IHJlYWQgdGhlIGNvcnJlY3Qg ZGV2aWNlIG91dHB1dCwgYXMNCj4gaXQgcmVhZCBhbiBvdXRkYXRlZCB2YWx1ZSBzZXQgYXQgaW5p dGlhbGl6YXRpb24uIEl0IG5vdyB1cGRhdGVzIGl0cw0KPiB2b2x0YWdlIG9uIHJlYWQuDQo+IA0K DQpMb29rcyBnb29kIGZyb20gbXkgc2lkZS4NCg0KQWxleA0KDQo+IFNpZ25lZC1vZmYtYnk6IFJl bmF0byBMdWkgR2VoIDxyZW5hdG9nZWhAZ21haWwuY29tPg0KPiAtLS0NCj4gQ2hhbmdlcyBpbiB2 MzoNCj4gCS0gcmVtb3ZlZCBpbml0aWFsaXphdGlvbiAoaW50IHZvbHRhZ2VfdXYgPSAwKQ0KPiAJ LSByZXR1cm5zIGVycm9yIHdoZW4gdm9sdGFnZV91diBpcyBudWxsDQo+IENoYW5nZXMgaW4gdjQ6 DQo+IAktIHJldHVybnMgZXJyb3Igd2hlbiB2b2x0YWdlX3V2IGlzIG5lZ2F0aXZlDQo+IA0KPiAg ZHJpdmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3NzgwLmMgfCA2ICsrKysrLQ0KPiAgMSBmaWxlIGNo YW5nZWQsIDUgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KPiANCj4gZGlmZiAtLWdpdCBh L2RyaXZlcnMvc3RhZ2luZy9paW8vYWRjL2FkNzc4MC5jDQo+IGIvZHJpdmVycy9zdGFnaW5nL2lp by9hZGMvYWQ3NzgwLmMNCj4gaW5kZXggYjY3NDEyZGIwMzE4Li5jN2NiMDVjZWRiYmMgMTAwNjQ0 DQo+IC0tLSBhL2RyaXZlcnMvc3RhZ2luZy9paW8vYWRjL2FkNzc4MC5jDQo+ICsrKyBiL2RyaXZl cnMvc3RhZ2luZy9paW8vYWRjL2FkNzc4MC5jDQo+IEBAIC04NywxMiArODcsMTYgQEAgc3RhdGlj IGludCBhZDc3ODBfcmVhZF9yYXcoc3RydWN0IGlpb19kZXYgKmluZGlvX2RldiwNCj4gIAkJCSAg IGxvbmcgbSkNCj4gIHsNCj4gIAlzdHJ1Y3QgYWQ3NzgwX3N0YXRlICpzdCA9IGlpb19wcml2KGlu ZGlvX2Rldik7DQo+ICsJaW50IHZvbHRhZ2VfdXY7DQo+ICANCj4gIAlzd2l0Y2ggKG0pIHsNCj4g IAljYXNlIElJT19DSEFOX0lORk9fUkFXOg0KPiAgCQlyZXR1cm4gYWRfc2lnbWFfZGVsdGFfc2lu Z2xlX2NvbnZlcnNpb24oaW5kaW9fZGV2LCBjaGFuLA0KPiB2YWwpOw0KPiAgCWNhc2UgSUlPX0NI QU5fSU5GT19TQ0FMRToNCj4gLQkJKnZhbCA9IHN0LT5pbnRfdnJlZl9tdiAqIHN0LT5nYWluOw0K PiArCQl2b2x0YWdlX3V2ID0gcmVndWxhdG9yX2dldF92b2x0YWdlKHN0LT5yZWcpOw0KPiArCQlp ZiAodm9sdGFnZV91diA8IDApDQo+ICsJCQlyZXR1cm4gdm9sdGFnZV91djsNCj4gKwkJKnZhbCA9 ICh2b2x0YWdlX3V2IC8gMTAwMCkgKiBzdC0+Z2FpbjsNCj4gIAkJKnZhbDIgPSBjaGFuLT5zY2Fu X3R5cGUucmVhbGJpdHMgLSAxOw0KPiAgCQlyZXR1cm4gSUlPX1ZBTF9GUkFDVElPTkFMX0xPRzI7 DQo+ICAJY2FzZSBJSU9fQ0hBTl9JTkZPX09GRlNFVDoNCg==
Jonathan Cameron
maintainerRe: [PATCH v4 1/2] staging: iio: ad7780: update voltage on read
Quoted 6 lines · Alexandru
Quoted 4 lines · Renato Lui Geh
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.
Quoted 1 line
Looks good from my side.
Ack?
Much preferred if you are willing to give a formal acknowledgement.
Thanks,
Jonathan
Quoted 38 lines
Alex
Quoted 35 lines
Signed-off-by Renato Lui Geh <[email protected]>---
Changes in v3:
- removed initialization (intvoltage_uv= 0)
- returns error whenvoltage_uvis null
Changes in v4:
- returns error whenvoltage_uvis negativedrivers/staging/iio/adc/ad7780.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)drivers/staging/iio/adc/ad7780.c +5
-1@@ -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:
Ardelean, Alexandru
Acked-byRe: [PATCH v4 1/2] staging: iio: ad7780: update voltage on read
T24gU3VuLCAyMDE4LTExLTExIGF0IDE0OjMwICswMDAwLCBKb25hdGhhbiBDYW1lcm9uIHdyb3Rl Og0KPiBPbiBUdWUsIDYgTm92IDIwMTggMDk6MjQ6NDQgKzAwMDANCj4gIkFyZGVsZWFuLCBBbGV4 YW5kcnUiIDxhbGV4YW5kcnUuQXJkZWxlYW5AYW5hbG9nLmNvbT4gd3JvdGU6DQo+IA0KPiA+IE9u IE1vbiwgMjAxOC0xMS0wNSBhdCAxNzoxNCAtMDIwMCwgUmVuYXRvIEx1aSBHZWggd3JvdGU6DQo+ ID4gPiBUaGUgYWQ3NzgwIGRyaXZlciBwcmV2aW91c2x5IGRpZCBub3QgcmVhZCB0aGUgY29ycmVj dCBkZXZpY2Ugb3V0cHV0LA0KPiA+ID4gYXMNCj4gPiA+IGl0IHJlYWQgYW4gb3V0ZGF0ZWQgdmFs dWUgc2V0IGF0IGluaXRpYWxpemF0aW9uLiBJdCBub3cgdXBkYXRlcyBpdHMNCj4gPiA+IHZvbHRh Z2Ugb24gcmVhZC4NCj4gPiA+ICAgDQo+IA0KPiBBcHBsaWVkIHRvIHRoZSB0b2dyZWcgYnJhbmNo IG9mIGlpby5naXQgYW5kIHB1c2hlZCBvdXQgYXMgdGVzdGluZyBmb3INCj4gdGhlIGF1dG9idWls ZGVycyB0byBjaGVjayBpdC4gIEknbSBub3QgdGhhdCBmdXNzZWQgYWJvdXQgcHVzaGluZyB0aGlz DQo+IG9uZSBhcyBhIGZpeCwgYXMgbW9zdCBvZiB0aGUgdGltZSB0aGVzZSB0aGluZ3MgYXJlIG9u IGZpeGVkIHJlZ3VsYXRvcnMNCj4gYW55d2F5LCBidXQgaXQgaXMgbmljZSB0byBkbyBpdCByaWdo dC4NCj4gDQo+ID4gTG9va3MgZ29vZCBmcm9tIG15IHNpZGUuDQo+IA0KPiBBY2s/DQoNCkFja2Vk LWJ5OiBBbGV4YW5kcnUgQXJkZWxlYW4gPGFsZXhhbmRydS5hcmRlbGVhbkBhbmFsb2cuY29tPg0K DQo+IA0KPiBNdWNoIHByZWZlcnJlZCBpZiB5b3UgYXJlIHdpbGxpbmcgdG8gZ2l2ZSBhIGZvcm1h bCBhY2tub3dsZWRnZW1lbnQuDQo+IA0KPiBUaGFua3MsDQo+IA0KPiBKb25hdGhhbg0KPiANCj4g DQo+ID4gDQo+ID4gQWxleA0KPiA+IA0KPiA+ID4gU2lnbmVkLW9mZi1ieTogUmVuYXRvIEx1aSBH ZWggPHJlbmF0b2dlaEBnbWFpbC5jb20+DQo+ID4gPiAtLS0NCj4gPiA+IENoYW5nZXMgaW4gdjM6 DQo+ID4gPiAJLSByZW1vdmVkIGluaXRpYWxpemF0aW9uIChpbnQgdm9sdGFnZV91diA9IDApDQo+ ID4gPiAJLSByZXR1cm5zIGVycm9yIHdoZW4gdm9sdGFnZV91diBpcyBudWxsDQo+ID4gPiBDaGFu Z2VzIGluIHY0Og0KPiA+ID4gCS0gcmV0dXJucyBlcnJvciB3aGVuIHZvbHRhZ2VfdXYgaXMgbmVn YXRpdmUNCj4gPiA+IA0KPiA+ID4gIGRyaXZlcnMvc3RhZ2luZy9paW8vYWRjL2FkNzc4MC5jIHwg NiArKysrKy0NCj4gPiA+ICAxIGZpbGUgY2hhbmdlZCwgNSBpbnNlcnRpb25zKCspLCAxIGRlbGV0 aW9uKC0pDQo+ID4gPiANCj4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3N0YWdpbmcvaWlvL2Fk Yy9hZDc3ODAuYw0KPiA+ID4gYi9kcml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc3ODAuYw0KPiA+ ID4gaW5kZXggYjY3NDEyZGIwMzE4Li5jN2NiMDVjZWRiYmMgMTAwNjQ0DQo+ID4gPiAtLS0gYS9k cml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc3ODAuYw0KPiA+ID4gKysrIGIvZHJpdmVycy9zdGFn aW5nL2lpby9hZGMvYWQ3NzgwLmMNCj4gPiA+IEBAIC04NywxMiArODcsMTYgQEAgc3RhdGljIGlu dCBhZDc3ODBfcmVhZF9yYXcoc3RydWN0IGlpb19kZXYNCj4gPiA+ICppbmRpb19kZXYsDQo+ID4g PiAgCQkJICAgbG9uZyBtKQ0KPiA+ID4gIHsNCj4gPiA+ICAJc3RydWN0IGFkNzc4MF9zdGF0ZSAq c3QgPSBpaW9fcHJpdihpbmRpb19kZXYpOw0KPiA+ID4gKwlpbnQgdm9sdGFnZV91djsNCj4gPiA+ ICANCj4gPiA+ICAJc3dpdGNoIChtKSB7DQo+ID4gPiAgCWNhc2UgSUlPX0NIQU5fSU5GT19SQVc6 DQo+ID4gPiAgCQlyZXR1cm4gYWRfc2lnbWFfZGVsdGFfc2luZ2xlX2NvbnZlcnNpb24oaW5kaW9f ZGV2LCBjaGFuLA0KPiA+ID4gdmFsKTsNCj4gPiA+ICAJY2FzZSBJSU9fQ0hBTl9JTkZPX1NDQUxF Og0KPiA+ID4gLQkJKnZhbCA9IHN0LT5pbnRfdnJlZl9tdiAqIHN0LT5nYWluOw0KPiA+ID4gKwkJ dm9sdGFnZV91diA9IHJlZ3VsYXRvcl9nZXRfdm9sdGFnZShzdC0+cmVnKTsNCj4gPiA+ICsJCWlm ICh2b2x0YWdlX3V2IDwgMCkNCj4gPiA+ICsJCQlyZXR1cm4gdm9sdGFnZV91djsNCj4gPiA+ICsJ CSp2YWwgPSAodm9sdGFnZV91diAvIDEwMDApICogc3QtPmdhaW47DQo+ID4gPiAgCQkqdmFsMiA9 IGNoYW4tPnNjYW5fdHlwZS5yZWFsYml0cyAtIDE7DQo+ID4gPiAgCQlyZXR1cm4gSUlPX1ZBTF9G UkFDVElPTkFMX0xPRzI7DQo+ID4gPiAgCWNhc2UgSUlPX0NIQU5fSU5GT19PRkZTRVQ6ICANCj4g DQo+IA0K
Jonathan Cameron
maintainerRe: [PATCH v4 1/2] staging: iio: ad7780: update voltage on read
Quoted 16 lines · Alexandru
Quoted 15 lines · Jonathan Cameron
Quoted 6 lines · Alexandru
Quoted 5 lines · Renato Lui Geh
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.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Looks good from my side.
Ack?
Added to both in this series.
Thanks,
Jonathan
Quoted 50 lines
Quoted 50 lines
Much preferred if you are willing to give a formal acknowledgement.
Thanks,
Jonathan
Quoted 39 lines
Alex
Quoted 36 lines
Signed-off-by Renato Lui Geh <[email protected]>---
Changes in v3:
- removed initialization (intvoltage_uv= 0)
- returns error whenvoltage_uvis null
Changes in v4:
- returns error whenvoltage_uvis negativedrivers/staging/iio/adc/ad7780.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)drivers/staging/iio/adc/ad7780.c +5
-1@@ -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 ;drivers/staging/iio/adc/ad7780.c +0
-0return IIO_VAL_FRACTIONAL_LOG2 ;case IIO_CHAN_INFO_OFFSET :
PATCH v4 2/2 staging: iio: ad7780: remove unnecessary stashed voltage value
In reply to Renato Lui GehThis patch removes the unnecessary field int_vref_mv in ad7780_state referring to the device's voltage.
---
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(-)
drivers/staging/iio/adc/ad7780.c +0 -0
@@ -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;
Signature · 1 line
2.19.1
Re: [PATCH v4 2/2] staging: iio: ad7780: remove unnecessary stashed voltage value
Quoted 3 lines · Renato Lui Geh
This patch removes the unnecessary field
int_vref_mvinad7780_statereferring to the device's voltage.
Looks good from my side.
Alex
Quoted 50 lines
Signed-off-by Renato Lui Geh <[email protected]>---
Changes in v3:
- removed unnecessaryint_vref_mvfromad7780_state
Changes in v4:
- removed voltage reading on probedrivers/staging/iio/adc/ad7780.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)drivers/staging/iio/adc/ad7780.c +1
-8@@ -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;
Jonathan Cameron
maintainerRe: [PATCH v4 2/2] staging: iio: ad7780: remove unnecessary stashed voltage value
Quoted 6 lines · Alexandru
Quoted 3 lines · Renato Lui Geh
This patch removes the unnecessary field
int_vref_mvinad7780_statereferring 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
Quoted 53 lines
Alex
Quoted 50 lines
Signed-off-by Renato Lui Geh <[email protected]>---
Changes in v3:
- removed unnecessaryint_vref_mvfromad7780_state
Changes in v4:
- removed voltage reading on probedrivers/staging/iio/adc/ad7780.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)drivers/staging/iio/adc/ad7780.c +1
-8@@ -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;