[PATCH v2 0/2] pattern generation and gain update

[PATCH v2 0/2] pattern generation and gain update

[Giuliano Belinassi]

This series of patches fixes a bug in ad717x chips where the PAT2 bit
was wrongly read as a GAIN bit. It also refactors the pattern_mask
generation with the PAT bits.

Changelog:
* v2
	- Squashed is_add778x flag commit with the gain update fix
	- Changed u8 is_add778x to bool is_add778x
	- Set pattern and pattern_mask as macros
	- Aligned identation of macros

Giuliano Belinassi (2):
  staging: iio: ad7780: check if ad778x before gain update
  staging: iio: ad7780: generates pattern_mask from PAT bits

 drivers/staging/iio/adc/ad7780.c | 55 ++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 20 deletions(-)

-- 
2.19.1

[PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Giuliano Belinassi]

Only the ad778x have the 'gain' status bit. Check it before updating
through a new variable is_ad778x in chip_info.

Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
---
Changes in v2:
	- Squashed is_ad778x declaration commit with the ad778x checkage
	- Changed is_ad778x type to bool
 
 drivers/staging/iio/adc/ad7780.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
index 91e016d534ed..9ec2b002539e 100644
--- a/drivers/staging/iio/adc/ad7780.c
+++ b/drivers/staging/iio/adc/ad7780.c
@@ -35,6 +35,7 @@ struct ad7780_chip_info {
 	struct iio_chan_spec	channel;
 	unsigned int		pattern_mask;
 	unsigned int		pattern;
+	bool			is_ad778x;
 };
 
 struct ad7780_state {
@@ -113,10 +114,12 @@ static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta,
 	    ((raw_sample & chip_info->pattern_mask) != chip_info->pattern))
 		return -EIO;
 
-	if (raw_sample & AD7780_GAIN)
-		st->gain = 1;
-	else
-		st->gain = 128;
+	if (chip_info->is_ad778x) {
+		if (raw_sample & AD7780_GAIN)
+			st->gain = 1;
+		else
+			st->gain = 128;
+	}
 
 	return 0;
 }
@@ -135,21 +138,25 @@ static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
 		.channel = AD7780_CHANNEL(12, 24),
 		.pattern = 0x5,
 		.pattern_mask = 0x7,
+		.is_ad778x = false,
 	},
 	[ID_AD7171] = {
 		.channel = AD7780_CHANNEL(16, 24),
 		.pattern = 0x5,
 		.pattern_mask = 0x7,
+		.is_ad778x = false,
 	},
 	[ID_AD7780] = {
 		.channel = AD7780_CHANNEL(24, 32),
 		.pattern = 0x1,
 		.pattern_mask = 0x3,
+		.is_ad778x = true,
 	},
 	[ID_AD7781] = {
 		.channel = AD7780_CHANNEL(20, 32),
 		.pattern = 0x1,
 		.pattern_mask = 0x3,
+		.is_ad778x = true,
 	},
 };
 
-- 
2.19.1

[PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits

[Giuliano Belinassi]

Previously, all pattern_masks and patterns in the chip_info table were
hardcoded. Now they are generated using the PAT macros, as described in
the datasheets.

Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
---
Changes in v2:
	- Added the PATTERN and PATTERN_MASK macros
	- Update BIT macros alignment to match with PATTERN
	- Generate pattern mask with PATTERN_MASK macros.

drivers/staging/iio/adc/ad7780.c | 40 +++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
index 9ec2b002539e..ff8e3b2d0efc 100644
--- a/drivers/staging/iio/adc/ad7780.c
+++ b/drivers/staging/iio/adc/ad7780.c
@@ -22,14 +22,22 @@
 #include <linux/iio/sysfs.h>
 #include <linux/iio/adc/ad_sigma_delta.h>
 
-#define AD7780_RDY	BIT(7)
-#define AD7780_FILTER	BIT(6)
-#define AD7780_ERR	BIT(5)
-#define AD7780_ID1	BIT(4)
-#define AD7780_ID0	BIT(3)
-#define AD7780_GAIN	BIT(2)
-#define AD7780_PAT1	BIT(1)
-#define AD7780_PAT0	BIT(0)
+#define AD7780_RDY		BIT(7)
+#define AD7780_FILTER		BIT(6)
+#define AD7780_ERR		BIT(5)
+#define AD7780_ID1		BIT(4)
+#define AD7780_ID0		BIT(3)
+#define AD7780_GAIN		BIT(2)
+#define AD7780_PAT1		BIT(1)
+#define AD7780_PAT0		BIT(0)
+
+#define AD7780_PATTERN		(AD7780_PAT0)
+#define AD7780_PATTERN_MASK	(AD7780_PAT0 | AD7780_PAT1)
+
+#define AD7170_PAT2		BIT(2)
+
+#define AD7170_PATTERN		(AD7780_PAT0 | AD7170_PAT2)
+#define AD7170_PATTERN_MASK	(AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2)
 
 struct ad7780_chip_info {
 	struct iio_chan_spec	channel;
@@ -136,26 +144,26 @@ static const struct ad_sigma_delta_info ad7780_sigma_delta_info = {
 static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
 	[ID_AD7170] = {
 		.channel = AD7780_CHANNEL(12, 24),
-		.pattern = 0x5,
-		.pattern_mask = 0x7,
+		.pattern = AD7170_PATTERN,
+		.pattern_mask = AD7170_PATTERN_MASK,
 		.is_ad778x = false,
 	},
 	[ID_AD7171] = {
 		.channel = AD7780_CHANNEL(16, 24),
-		.pattern = 0x5,
-		.pattern_mask = 0x7,
+		.pattern = AD7170_PATTERN,
+		.pattern_mask = AD7170_PATTERN_MASK,
 		.is_ad778x = false,
 	},
 	[ID_AD7780] = {
 		.channel = AD7780_CHANNEL(24, 32),
-		.pattern = 0x1,
-		.pattern_mask = 0x3,
+		.pattern = AD7780_PATTERN,
+		.pattern_mask = AD7780_PATTERN_MASK,
 		.is_ad778x = true,
 	},
 	[ID_AD7781] = {
 		.channel = AD7780_CHANNEL(20, 32),
-		.pattern = 0x1,
-		.pattern_mask = 0x3,
+		.pattern = AD7780_PATTERN,
+		.pattern_mask = AD7780_PATTERN_MASK,
 		.is_ad778x = true,
 	},
 };
-- 
2.19.1

Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Ardelean, Alexandru]

On Thu, 2018-11-08 at 11:03 -0200, Giuliano Belinassi wrote:
> Only the ad778x have the 'gain' status bit. Check it before updating
> through a new variable is_ad778x in chip_info.
> 

Looks good.

Alex

> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> ---
> Changes in v2:
> 	- Squashed is_ad778x declaration commit with the ad778x checkage
> 	- Changed is_ad778x type to bool
>  
>  drivers/staging/iio/adc/ad7780.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index 91e016d534ed..9ec2b002539e 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -35,6 +35,7 @@ struct ad7780_chip_info {
>  	struct iio_chan_spec	channel;
>  	unsigned int		pattern_mask;
>  	unsigned int		pattern;
> +	bool			is_ad778x;
>  };
>  
>  struct ad7780_state {
> @@ -113,10 +114,12 @@ static int ad7780_postprocess_sample(struct
> ad_sigma_delta *sigma_delta,
>  	    ((raw_sample & chip_info->pattern_mask) != chip_info->pattern))
>  		return -EIO;
>  
> -	if (raw_sample & AD7780_GAIN)
> -		st->gain = 1;
> -	else
> -		st->gain = 128;
> +	if (chip_info->is_ad778x) {
> +		if (raw_sample & AD7780_GAIN)
> +			st->gain = 1;
> +		else
> +			st->gain = 128;
> +	}
>  
>  	return 0;
>  }
> @@ -135,21 +138,25 @@ static const struct ad7780_chip_info
> ad7780_chip_info_tbl[] = {
>  		.channel = AD7780_CHANNEL(12, 24),
>  		.pattern = 0x5,
>  		.pattern_mask = 0x7,
> +		.is_ad778x = false,
>  	},
>  	[ID_AD7171] = {
>  		.channel = AD7780_CHANNEL(16, 24),
>  		.pattern = 0x5,
>  		.pattern_mask = 0x7,
> +		.is_ad778x = false,
>  	},
>  	[ID_AD7780] = {
>  		.channel = AD7780_CHANNEL(24, 32),
>  		.pattern = 0x1,
>  		.pattern_mask = 0x3,
> +		.is_ad778x = true,
>  	},
>  	[ID_AD7781] = {
>  		.channel = AD7780_CHANNEL(20, 32),
>  		.pattern = 0x1,
>  		.pattern_mask = 0x3,
> +		.is_ad778x = true,
>  	},
>  };
>  

Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Ardelean, Alexandru]

T24gVGh1LCAyMDE4LTExLTA4IGF0IDExOjAzIC0wMjAwLCBHaXVsaWFubyBCZWxpbmFzc2kgd3Jv
dGU6DQo+IE9ubHkgdGhlIGFkNzc4eCBoYXZlIHRoZSAnZ2Fpbicgc3RhdHVzIGJpdC4gQ2hlY2sg
aXQgYmVmb3JlIHVwZGF0aW5nDQo+IHRocm91Z2ggYSBuZXcgdmFyaWFibGUgaXNfYWQ3Nzh4IGlu
IGNoaXBfaW5mby4NCj4gDQoNCkxvb2tzIGdvb2QuDQoNCkFsZXgNCg0KPiBTaWduZWQtb2ZmLWJ5
OiBHaXVsaWFubyBCZWxpbmFzc2kgPGdpdWxpYW5vLmJlbGluYXNzaUB1c3AuYnI+DQo+IC0tLQ0K
PiBDaGFuZ2VzIGluIHYyOg0KPiAJLSBTcXVhc2hlZCBpc19hZDc3OHggZGVjbGFyYXRpb24gY29t
bWl0IHdpdGggdGhlIGFkNzc4eCBjaGVja2FnZQ0KPiAJLSBDaGFuZ2VkIGlzX2FkNzc4eCB0eXBl
IHRvIGJvb2wNCj4gIA0KPiAgZHJpdmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3NzgwLmMgfCAxNSAr
KysrKysrKysrKy0tLS0NCj4gIDEgZmlsZSBjaGFuZ2VkLCAxMSBpbnNlcnRpb25zKCspLCA0IGRl
bGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvc3RhZ2luZy9paW8vYWRjL2Fk
Nzc4MC5jDQo+IGIvZHJpdmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3NzgwLmMNCj4gaW5kZXggOTFl
MDE2ZDUzNGVkLi45ZWMyYjAwMjUzOWUgMTAwNjQ0DQo+IC0tLSBhL2RyaXZlcnMvc3RhZ2luZy9p
aW8vYWRjL2FkNzc4MC5jDQo+ICsrKyBiL2RyaXZlcnMvc3RhZ2luZy9paW8vYWRjL2FkNzc4MC5j
DQo+IEBAIC0zNSw2ICszNSw3IEBAIHN0cnVjdCBhZDc3ODBfY2hpcF9pbmZvIHsNCj4gIAlzdHJ1
Y3QgaWlvX2NoYW5fc3BlYwljaGFubmVsOw0KPiAgCXVuc2lnbmVkIGludAkJcGF0dGVybl9tYXNr
Ow0KPiAgCXVuc2lnbmVkIGludAkJcGF0dGVybjsNCj4gKwlib29sCQkJaXNfYWQ3Nzh4Ow0KPiAg
fTsNCj4gIA0KPiAgc3RydWN0IGFkNzc4MF9zdGF0ZSB7DQo+IEBAIC0xMTMsMTAgKzExNCwxMiBA
QCBzdGF0aWMgaW50IGFkNzc4MF9wb3N0cHJvY2Vzc19zYW1wbGUoc3RydWN0DQo+IGFkX3NpZ21h
X2RlbHRhICpzaWdtYV9kZWx0YSwNCj4gIAkgICAgKChyYXdfc2FtcGxlICYgY2hpcF9pbmZvLT5w
YXR0ZXJuX21hc2spICE9IGNoaXBfaW5mby0+cGF0dGVybikpDQo+ICAJCXJldHVybiAtRUlPOw0K
PiAgDQo+IC0JaWYgKHJhd19zYW1wbGUgJiBBRDc3ODBfR0FJTikNCj4gLQkJc3QtPmdhaW4gPSAx
Ow0KPiAtCWVsc2UNCj4gLQkJc3QtPmdhaW4gPSAxMjg7DQo+ICsJaWYgKGNoaXBfaW5mby0+aXNf
YWQ3Nzh4KSB7DQo+ICsJCWlmIChyYXdfc2FtcGxlICYgQUQ3NzgwX0dBSU4pDQo+ICsJCQlzdC0+
Z2FpbiA9IDE7DQo+ICsJCWVsc2UNCj4gKwkJCXN0LT5nYWluID0gMTI4Ow0KPiArCX0NCj4gIA0K
PiAgCXJldHVybiAwOw0KPiAgfQ0KPiBAQCAtMTM1LDIxICsxMzgsMjUgQEAgc3RhdGljIGNvbnN0
IHN0cnVjdCBhZDc3ODBfY2hpcF9pbmZvDQo+IGFkNzc4MF9jaGlwX2luZm9fdGJsW10gPSB7DQo+
ICAJCS5jaGFubmVsID0gQUQ3NzgwX0NIQU5ORUwoMTIsIDI0KSwNCj4gIAkJLnBhdHRlcm4gPSAw
eDUsDQo+ICAJCS5wYXR0ZXJuX21hc2sgPSAweDcsDQo+ICsJCS5pc19hZDc3OHggPSBmYWxzZSwN
Cj4gIAl9LA0KPiAgCVtJRF9BRDcxNzFdID0gew0KPiAgCQkuY2hhbm5lbCA9IEFENzc4MF9DSEFO
TkVMKDE2LCAyNCksDQo+ICAJCS5wYXR0ZXJuID0gMHg1LA0KPiAgCQkucGF0dGVybl9tYXNrID0g
MHg3LA0KPiArCQkuaXNfYWQ3Nzh4ID0gZmFsc2UsDQo+ICAJfSwNCj4gIAlbSURfQUQ3NzgwXSA9
IHsNCj4gIAkJLmNoYW5uZWwgPSBBRDc3ODBfQ0hBTk5FTCgyNCwgMzIpLA0KPiAgCQkucGF0dGVy
biA9IDB4MSwNCj4gIAkJLnBhdHRlcm5fbWFzayA9IDB4MywNCj4gKwkJLmlzX2FkNzc4eCA9IHRy
dWUsDQo+ICAJfSwNCj4gIAlbSURfQUQ3NzgxXSA9IHsNCj4gIAkJLmNoYW5uZWwgPSBBRDc3ODBf
Q0hBTk5FTCgyMCwgMzIpLA0KPiAgCQkucGF0dGVybiA9IDB4MSwNCj4gIAkJLnBhdHRlcm5fbWFz
ayA9IDB4MywNCj4gKwkJLmlzX2FkNzc4eCA9IHRydWUsDQo+ICAJfSwNCj4gIH07DQo+ICANCg==

Re: [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits

[Ardelean, Alexandru]

On Thu, 2018-11-08 at 11:03 -0200, Giuliano Belinassi wrote:
> Previously, all pattern_masks and patterns in the chip_info table were
> hardcoded. Now they are generated using the PAT macros, as described in
> the datasheets.

One comment about indentation/whitespace.

Rest looks good.

Alex

> 
> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> ---
> Changes in v2:
> 	- Added the PATTERN and PATTERN_MASK macros
> 	- Update BIT macros alignment to match with PATTERN
> 	- Generate pattern mask with PATTERN_MASK macros.
> 
> drivers/staging/iio/adc/ad7780.c | 40 +++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index 9ec2b002539e..ff8e3b2d0efc 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -22,14 +22,22 @@
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/adc/ad_sigma_delta.h>
>  
> -#define AD7780_RDY	BIT(7)
> -#define AD7780_FILTER	BIT(6)
> -#define AD7780_ERR	BIT(5)
> -#define AD7780_ID1	BIT(4)
> -#define AD7780_ID0	BIT(3)
> -#define AD7780_GAIN	BIT(2)
> -#define AD7780_PAT1	BIT(1)
> -#define AD7780_PAT0	BIT(0)
> +#define AD7780_RDY		BIT(7)
> +#define AD7780_FILTER		BIT(6)
> +#define AD7780_ERR		BIT(5)
> +#define AD7780_ID1		BIT(4)
> +#define AD7780_ID0		BIT(3)
> +#define AD7780_GAIN		BIT(2)
> +#define AD7780_PAT1		BIT(1)
> +#define AD7780_PAT0		BIT(0)

Changing indentation here doesn't add much value; it's mostly
patch/whitespace noise.

While I agree that it looks nicer to indent all these to the same level,
you also need to think about the fact that the kernel git repo is already
pretty big as-is, so it's a good idea if a patch adds as much code/semantic
value [as possible] with as little change [as possible] and with as good of
readability [as possible].
[ Kind of sounds like a balance act between the 3 things ].


> +
> +#define AD7780_PATTERN		(AD7780_PAT0)
> +#define AD7780_PATTERN_MASK	(AD7780_PAT0 | AD7780_PAT1)
> +
> +#define AD7170_PAT2		BIT(2)
> +
> +#define AD7170_PATTERN		(AD7780_PAT0 | AD7170_PAT2)
> +#define AD7170_PATTERN_MASK	(AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2)
>  
>  struct ad7780_chip_info {
>  	struct iio_chan_spec	channel;
> @@ -136,26 +144,26 @@ static const struct ad_sigma_delta_info
> ad7780_sigma_delta_info = {
>  static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
>  	[ID_AD7170] = {
>  		.channel = AD7780_CHANNEL(12, 24),
> -		.pattern = 0x5,
> -		.pattern_mask = 0x7,
> +		.pattern = AD7170_PATTERN,
> +		.pattern_mask = AD7170_PATTERN_MASK,
>  		.is_ad778x = false,
>  	},
>  	[ID_AD7171] = {
>  		.channel = AD7780_CHANNEL(16, 24),
> -		.pattern = 0x5,
> -		.pattern_mask = 0x7,
> +		.pattern = AD7170_PATTERN,
> +		.pattern_mask = AD7170_PATTERN_MASK,
>  		.is_ad778x = false,
>  	},
>  	[ID_AD7780] = {
>  		.channel = AD7780_CHANNEL(24, 32),
> -		.pattern = 0x1,
> -		.pattern_mask = 0x3,
> +		.pattern = AD7780_PATTERN,
> +		.pattern_mask = AD7780_PATTERN_MASK,
>  		.is_ad778x = true,
>  	},
>  	[ID_AD7781] = {
>  		.channel = AD7780_CHANNEL(20, 32),
> -		.pattern = 0x1,
> -		.pattern_mask = 0x3,
> +		.pattern = AD7780_PATTERN,
> +		.pattern_mask = AD7780_PATTERN_MASK,
>  		.is_ad778x = true,
>  	},
>  };

Re: [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits

[Ardelean, Alexandru]

T24gVGh1LCAyMDE4LTExLTA4IGF0IDExOjAzIC0wMjAwLCBHaXVsaWFubyBCZWxpbmFzc2kgd3Jv
dGU6DQo+IFByZXZpb3VzbHksIGFsbCBwYXR0ZXJuX21hc2tzIGFuZCBwYXR0ZXJucyBpbiB0aGUg
Y2hpcF9pbmZvIHRhYmxlIHdlcmUNCj4gaGFyZGNvZGVkLiBOb3cgdGhleSBhcmUgZ2VuZXJhdGVk
IHVzaW5nIHRoZSBQQVQgbWFjcm9zLCBhcyBkZXNjcmliZWQgaW4NCj4gdGhlIGRhdGFzaGVldHMu
DQoNCk9uZSBjb21tZW50IGFib3V0IGluZGVudGF0aW9uL3doaXRlc3BhY2UuDQoNClJlc3QgbG9v
a3MgZ29vZC4NCg0KQWxleA0KDQo+IA0KPiBTaWduZWQtb2ZmLWJ5OiBHaXVsaWFubyBCZWxpbmFz
c2kgPGdpdWxpYW5vLmJlbGluYXNzaUB1c3AuYnI+DQo+IC0tLQ0KPiBDaGFuZ2VzIGluIHYyOg0K
PiAJLSBBZGRlZCB0aGUgUEFUVEVSTiBhbmQgUEFUVEVSTl9NQVNLIG1hY3Jvcw0KPiAJLSBVcGRh
dGUgQklUIG1hY3JvcyBhbGlnbm1lbnQgdG8gbWF0Y2ggd2l0aCBQQVRURVJODQo+IAktIEdlbmVy
YXRlIHBhdHRlcm4gbWFzayB3aXRoIFBBVFRFUk5fTUFTSyBtYWNyb3MuDQo+IA0KPiBkcml2ZXJz
L3N0YWdpbmcvaWlvL2FkYy9hZDc3ODAuYyB8IDQwICsrKysrKysrKysrKysrKysrKystLS0tLS0t
LS0tLS0tDQo+ICAxIGZpbGUgY2hhbmdlZCwgMjQgaW5zZXJ0aW9ucygrKSwgMTYgZGVsZXRpb25z
KC0pDQo+IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3NzgwLmMN
Cj4gYi9kcml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc3ODAuYw0KPiBpbmRleCA5ZWMyYjAwMjUz
OWUuLmZmOGUzYjJkMGVmYyAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy9zdGFnaW5nL2lpby9hZGMv
YWQ3NzgwLmMNCj4gKysrIGIvZHJpdmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3NzgwLmMNCj4gQEAg
LTIyLDE0ICsyMiwyMiBAQA0KPiAgI2luY2x1ZGUgPGxpbnV4L2lpby9zeXNmcy5oPg0KPiAgI2lu
Y2x1ZGUgPGxpbnV4L2lpby9hZGMvYWRfc2lnbWFfZGVsdGEuaD4NCj4gIA0KPiAtI2RlZmluZSBB
RDc3ODBfUkRZCUJJVCg3KQ0KPiAtI2RlZmluZSBBRDc3ODBfRklMVEVSCUJJVCg2KQ0KPiAtI2Rl
ZmluZSBBRDc3ODBfRVJSCUJJVCg1KQ0KPiAtI2RlZmluZSBBRDc3ODBfSUQxCUJJVCg0KQ0KPiAt
I2RlZmluZSBBRDc3ODBfSUQwCUJJVCgzKQ0KPiAtI2RlZmluZSBBRDc3ODBfR0FJTglCSVQoMikN
Cj4gLSNkZWZpbmUgQUQ3NzgwX1BBVDEJQklUKDEpDQo+IC0jZGVmaW5lIEFENzc4MF9QQVQwCUJJ
VCgwKQ0KPiArI2RlZmluZSBBRDc3ODBfUkRZCQlCSVQoNykNCj4gKyNkZWZpbmUgQUQ3NzgwX0ZJ
TFRFUgkJQklUKDYpDQo+ICsjZGVmaW5lIEFENzc4MF9FUlIJCUJJVCg1KQ0KPiArI2RlZmluZSBB
RDc3ODBfSUQxCQlCSVQoNCkNCj4gKyNkZWZpbmUgQUQ3NzgwX0lEMAkJQklUKDMpDQo+ICsjZGVm
aW5lIEFENzc4MF9HQUlOCQlCSVQoMikNCj4gKyNkZWZpbmUgQUQ3NzgwX1BBVDEJCUJJVCgxKQ0K
PiArI2RlZmluZSBBRDc3ODBfUEFUMAkJQklUKDApDQoNCkNoYW5naW5nIGluZGVudGF0aW9uIGhl
cmUgZG9lc24ndCBhZGQgbXVjaCB2YWx1ZTsgaXQncyBtb3N0bHkNCnBhdGNoL3doaXRlc3BhY2Ug
bm9pc2UuDQoNCldoaWxlIEkgYWdyZWUgdGhhdCBpdCBsb29rcyBuaWNlciB0byBpbmRlbnQgYWxs
IHRoZXNlIHRvIHRoZSBzYW1lIGxldmVsLA0KeW91IGFsc28gbmVlZCB0byB0aGluayBhYm91dCB0
aGUgZmFjdCB0aGF0IHRoZSBrZXJuZWwgZ2l0IHJlcG8gaXMgYWxyZWFkeQ0KcHJldHR5IGJpZyBh
cy1pcywgc28gaXQncyBhIGdvb2QgaWRlYSBpZiBhIHBhdGNoIGFkZHMgYXMgbXVjaCBjb2RlL3Nl
bWFudGljDQp2YWx1ZSBbYXMgcG9zc2libGVdIHdpdGggYXMgbGl0dGxlIGNoYW5nZSBbYXMgcG9z
c2libGVdIGFuZCB3aXRoIGFzIGdvb2Qgb2YNCnJlYWRhYmlsaXR5IFthcyBwb3NzaWJsZV0uDQpb
IEtpbmQgb2Ygc291bmRzIGxpa2UgYSBiYWxhbmNlIGFjdCBiZXR3ZWVuIHRoZSAzIHRoaW5ncyBd
Lg0KDQoNCj4gKw0KPiArI2RlZmluZSBBRDc3ODBfUEFUVEVSTgkJKEFENzc4MF9QQVQwKQ0KPiAr
I2RlZmluZSBBRDc3ODBfUEFUVEVSTl9NQVNLCShBRDc3ODBfUEFUMCB8IEFENzc4MF9QQVQxKQ0K
PiArDQo+ICsjZGVmaW5lIEFENzE3MF9QQVQyCQlCSVQoMikNCj4gKw0KPiArI2RlZmluZSBBRDcx
NzBfUEFUVEVSTgkJKEFENzc4MF9QQVQwIHwgQUQ3MTcwX1BBVDIpDQo+ICsjZGVmaW5lIEFENzE3
MF9QQVRURVJOX01BU0sJKEFENzc4MF9QQVQwIHwgQUQ3NzgwX1BBVDEgfCBBRDcxNzBfUEFUMikN
Cj4gIA0KPiAgc3RydWN0IGFkNzc4MF9jaGlwX2luZm8gew0KPiAgCXN0cnVjdCBpaW9fY2hhbl9z
cGVjCWNoYW5uZWw7DQo+IEBAIC0xMzYsMjYgKzE0NCwyNiBAQCBzdGF0aWMgY29uc3Qgc3RydWN0
IGFkX3NpZ21hX2RlbHRhX2luZm8NCj4gYWQ3NzgwX3NpZ21hX2RlbHRhX2luZm8gPSB7DQo+ICBz
dGF0aWMgY29uc3Qgc3RydWN0IGFkNzc4MF9jaGlwX2luZm8gYWQ3NzgwX2NoaXBfaW5mb190Ymxb
XSA9IHsNCj4gIAlbSURfQUQ3MTcwXSA9IHsNCj4gIAkJLmNoYW5uZWwgPSBBRDc3ODBfQ0hBTk5F
TCgxMiwgMjQpLA0KPiAtCQkucGF0dGVybiA9IDB4NSwNCj4gLQkJLnBhdHRlcm5fbWFzayA9IDB4
NywNCj4gKwkJLnBhdHRlcm4gPSBBRDcxNzBfUEFUVEVSTiwNCj4gKwkJLnBhdHRlcm5fbWFzayA9
IEFENzE3MF9QQVRURVJOX01BU0ssDQo+ICAJCS5pc19hZDc3OHggPSBmYWxzZSwNCj4gIAl9LA0K
PiAgCVtJRF9BRDcxNzFdID0gew0KPiAgCQkuY2hhbm5lbCA9IEFENzc4MF9DSEFOTkVMKDE2LCAy
NCksDQo+IC0JCS5wYXR0ZXJuID0gMHg1LA0KPiAtCQkucGF0dGVybl9tYXNrID0gMHg3LA0KPiAr
CQkucGF0dGVybiA9IEFENzE3MF9QQVRURVJOLA0KPiArCQkucGF0dGVybl9tYXNrID0gQUQ3MTcw
X1BBVFRFUk5fTUFTSywNCj4gIAkJLmlzX2FkNzc4eCA9IGZhbHNlLA0KPiAgCX0sDQo+ICAJW0lE
X0FENzc4MF0gPSB7DQo+ICAJCS5jaGFubmVsID0gQUQ3NzgwX0NIQU5ORUwoMjQsIDMyKSwNCj4g
LQkJLnBhdHRlcm4gPSAweDEsDQo+IC0JCS5wYXR0ZXJuX21hc2sgPSAweDMsDQo+ICsJCS5wYXR0
ZXJuID0gQUQ3NzgwX1BBVFRFUk4sDQo+ICsJCS5wYXR0ZXJuX21hc2sgPSBBRDc3ODBfUEFUVEVS
Tl9NQVNLLA0KPiAgCQkuaXNfYWQ3Nzh4ID0gdHJ1ZSwNCj4gIAl9LA0KPiAgCVtJRF9BRDc3ODFd
ID0gew0KPiAgCQkuY2hhbm5lbCA9IEFENzc4MF9DSEFOTkVMKDIwLCAzMiksDQo+IC0JCS5wYXR0
ZXJuID0gMHgxLA0KPiAtCQkucGF0dGVybl9tYXNrID0gMHgzLA0KPiArCQkucGF0dGVybiA9IEFE
Nzc4MF9QQVRURVJOLA0KPiArCQkucGF0dGVybl9tYXNrID0gQUQ3NzgwX1BBVFRFUk5fTUFTSywN
Cj4gIAkJLmlzX2FkNzc4eCA9IHRydWUsDQo+ICAJfSwNCj4gIH07DQo=

Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Tomasz Duszynski]

Hi Giuliano,

Comment inline.

On 11/8/18 2:03 PM, Giuliano Belinassi wrote:
> Only the ad778x have the 'gain' status bit. Check it before updating
> through a new variable is_ad778x in chip_info.
>
> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> ---
> Changes in v2:
> 	- Squashed is_ad778x declaration commit with the ad778x checkage
> 	- Changed is_ad778x type to bool
>  
>  drivers/staging/iio/adc/ad7780.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
> index 91e016d534ed..9ec2b002539e 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -35,6 +35,7 @@ struct ad7780_chip_info {
>  	struct iio_chan_spec	channel;
>  	unsigned int		pattern_mask;
>  	unsigned int		pattern;
> +	bool			is_ad778x;
>  };
>  
>  struct ad7780_state {
> @@ -113,10 +114,12 @@ static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta,
>  	    ((raw_sample & chip_info->pattern_mask) != chip_info->pattern))
>  		return -EIO;
>  
> -	if (raw_sample & AD7780_GAIN)
> -		st->gain = 1;
> -	else
> -		st->gain = 128;
> +	if (chip_info->is_ad778x) {
> +		if (raw_sample & AD7780_GAIN)
> +			st->gain = 1;
> +		else
> +			st->gain = 128;
> +	}

Just some random though. Instead of introducing extra level of indentation you
can simply check whether is_ad778x is asserted and simply return.

>  
>  	return 0;
>  }
> @@ -135,21 +138,25 @@ static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
>  		.channel = AD7780_CHANNEL(12, 24),
>  		.pattern = 0x5,
>  		.pattern_mask = 0x7,
> +		.is_ad778x = false,

Any reason for setting this explicitly? That's going to be false anyway.

>  	},
>  	[ID_AD7171] = {
>  		.channel = AD7780_CHANNEL(16, 24),
>  		.pattern = 0x5,
>  		.pattern_mask = 0x7,
> +		.is_ad778x = false,
>  	},
>  	[ID_AD7780] = {
>  		.channel = AD7780_CHANNEL(24, 32),
>  		.pattern = 0x1,
>  		.pattern_mask = 0x3,
> +		.is_ad778x = true,
>  	},
>  	[ID_AD7781] = {
>  		.channel = AD7780_CHANNEL(20, 32),
>  		.pattern = 0x1,
>  		.pattern_mask = 0x3,
> +		.is_ad778x = true,
>  	},
>  };
>  

Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Giuliano Augusto Faulin Belinassi]

> Just some random though. Instead of introducing extra level of indentation you
> can simply check whether is_ad778x is asserted and simply return.

I agree that the patch would be smaller if I do that, but is it really
an issue? If yes, then I will update the patch with this change

> Any reason for setting this explicitly? That's going to be false anyway

It seems clearer to me :-)
On Thu, Nov 8, 2018 at 4:26 PM Tomasz Duszynski <tduszyns@gmail.com> wrote:
>
> Hi Giuliano,
>
> Comment inline.
>
> On 11/8/18 2:03 PM, Giuliano Belinassi wrote:
> > Only the ad778x have the 'gain' status bit. Check it before updating
> > through a new variable is_ad778x in chip_info.
> >
> > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> > ---
> > Changes in v2:
> >       - Squashed is_ad778x declaration commit with the ad778x checkage
> >       - Changed is_ad778x type to bool
> >
> >  drivers/staging/iio/adc/ad7780.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
> > index 91e016d534ed..9ec2b002539e 100644
> > --- a/drivers/staging/iio/adc/ad7780.c
> > +++ b/drivers/staging/iio/adc/ad7780.c
> > @@ -35,6 +35,7 @@ struct ad7780_chip_info {
> >       struct iio_chan_spec    channel;
> >       unsigned int            pattern_mask;
> >       unsigned int            pattern;
> > +     bool                    is_ad778x;
> >  };
> >
> >  struct ad7780_state {
> > @@ -113,10 +114,12 @@ static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta,
> >           ((raw_sample & chip_info->pattern_mask) != chip_info->pattern))
> >               return -EIO;
> >
> > -     if (raw_sample & AD7780_GAIN)
> > -             st->gain = 1;
> > -     else
> > -             st->gain = 128;
> > +     if (chip_info->is_ad778x) {
> > +             if (raw_sample & AD7780_GAIN)
> > +                     st->gain = 1;
> > +             else
> > +                     st->gain = 128;
> > +     }
>
> Just some random though. Instead of introducing extra level of indentation you
> can simply check whether is_ad778x is asserted and simply return.
>
> >
> >       return 0;
> >  }
> > @@ -135,21 +138,25 @@ static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
> >               .channel = AD7780_CHANNEL(12, 24),
> >               .pattern = 0x5,
> >               .pattern_mask = 0x7,
> > +             .is_ad778x = false,
>
> Any reason for setting this explicitly? That's going to be false anyway.
>
> >       },
> >       [ID_AD7171] = {
> >               .channel = AD7780_CHANNEL(16, 24),
> >               .pattern = 0x5,
> >               .pattern_mask = 0x7,
> > +             .is_ad778x = false,
> >       },
> >       [ID_AD7780] = {
> >               .channel = AD7780_CHANNEL(24, 32),
> >               .pattern = 0x1,
> >               .pattern_mask = 0x3,
> > +             .is_ad778x = true,
> >       },
> >       [ID_AD7781] = {
> >               .channel = AD7780_CHANNEL(20, 32),
> >               .pattern = 0x1,
> >               .pattern_mask = 0x3,
> > +             .is_ad778x = true,
> >       },
> >  };
> >
>
> --
> You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> To post to this group, send email to kernel-usp@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/55b5de74-a607-94b9-8c85-40658e38fbb5%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits

[Giuliano Augusto Faulin Belinassi]

Hi

>While I agree that it looks nicer to indent all these to the same level,
>you also need to think about the fact that the kernel git repo is already
>pretty big as-is, so it's a good idea if a patch adds as much code/semantic
>value [as possible] with as little change [as possible] and with as good of
>readability [as possible].

Understood. But can't someone else submit another patch fixing these
indentation issues? That would be another commit and more stuff to the
repository.
On Thu, Nov 8, 2018 at 11:51 AM Ardelean, Alexandru
<alexandru.Ardelean@analog.com> wrote:
>
> On Thu, 2018-11-08 at 11:03 -0200, Giuliano Belinassi wrote:
> > Previously, all pattern_masks and patterns in the chip_info table were
> > hardcoded. Now they are generated using the PAT macros, as described in
> > the datasheets.
>
> One comment about indentation/whitespace.
>
> Rest looks good.
>
> Alex
>
> >
> > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> > ---
> > Changes in v2:
> >       - Added the PATTERN and PATTERN_MASK macros
> >       - Update BIT macros alignment to match with PATTERN
> >       - Generate pattern mask with PATTERN_MASK macros.
> >
> > drivers/staging/iio/adc/ad7780.c | 40 +++++++++++++++++++-------------
> >  1 file changed, 24 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7780.c
> > b/drivers/staging/iio/adc/ad7780.c
> > index 9ec2b002539e..ff8e3b2d0efc 100644
> > --- a/drivers/staging/iio/adc/ad7780.c
> > +++ b/drivers/staging/iio/adc/ad7780.c
> > @@ -22,14 +22,22 @@
> >  #include <linux/iio/sysfs.h>
> >  #include <linux/iio/adc/ad_sigma_delta.h>
> >
> > -#define AD7780_RDY   BIT(7)
> > -#define AD7780_FILTER        BIT(6)
> > -#define AD7780_ERR   BIT(5)
> > -#define AD7780_ID1   BIT(4)
> > -#define AD7780_ID0   BIT(3)
> > -#define AD7780_GAIN  BIT(2)
> > -#define AD7780_PAT1  BIT(1)
> > -#define AD7780_PAT0  BIT(0)
> > +#define AD7780_RDY           BIT(7)
> > +#define AD7780_FILTER                BIT(6)
> > +#define AD7780_ERR           BIT(5)
> > +#define AD7780_ID1           BIT(4)
> > +#define AD7780_ID0           BIT(3)
> > +#define AD7780_GAIN          BIT(2)
> > +#define AD7780_PAT1          BIT(1)
> > +#define AD7780_PAT0          BIT(0)
>
> Changing indentation here doesn't add much value; it's mostly
> patch/whitespace noise.
>
> While I agree that it looks nicer to indent all these to the same level,
> you also need to think about the fact that the kernel git repo is already
> pretty big as-is, so it's a good idea if a patch adds as much code/semantic
> value [as possible] with as little change [as possible] and with as good of
> readability [as possible].
> [ Kind of sounds like a balance act between the 3 things ].
>
>
> > +
> > +#define AD7780_PATTERN               (AD7780_PAT0)
> > +#define AD7780_PATTERN_MASK  (AD7780_PAT0 | AD7780_PAT1)
> > +
> > +#define AD7170_PAT2          BIT(2)
> > +
> > +#define AD7170_PATTERN               (AD7780_PAT0 | AD7170_PAT2)
> > +#define AD7170_PATTERN_MASK  (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2)
> >
> >  struct ad7780_chip_info {
> >       struct iio_chan_spec    channel;
> > @@ -136,26 +144,26 @@ static const struct ad_sigma_delta_info
> > ad7780_sigma_delta_info = {
> >  static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
> >       [ID_AD7170] = {
> >               .channel = AD7780_CHANNEL(12, 24),
> > -             .pattern = 0x5,
> > -             .pattern_mask = 0x7,
> > +             .pattern = AD7170_PATTERN,
> > +             .pattern_mask = AD7170_PATTERN_MASK,
> >               .is_ad778x = false,
> >       },
> >       [ID_AD7171] = {
> >               .channel = AD7780_CHANNEL(16, 24),
> > -             .pattern = 0x5,
> > -             .pattern_mask = 0x7,
> > +             .pattern = AD7170_PATTERN,
> > +             .pattern_mask = AD7170_PATTERN_MASK,
> >               .is_ad778x = false,
> >       },
> >       [ID_AD7780] = {
> >               .channel = AD7780_CHANNEL(24, 32),
> > -             .pattern = 0x1,
> > -             .pattern_mask = 0x3,
> > +             .pattern = AD7780_PATTERN,
> > +             .pattern_mask = AD7780_PATTERN_MASK,
> >               .is_ad778x = true,
> >       },
> >       [ID_AD7781] = {
> >               .channel = AD7780_CHANNEL(20, 32),
> > -             .pattern = 0x1,
> > -             .pattern_mask = 0x3,
> > +             .pattern = AD7780_PATTERN,
> > +             .pattern_mask = AD7780_PATTERN_MASK,
> >               .is_ad778x = true,
> >       },
> >  };
>
> --
> You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> To post to this group, send email to kernel-usp@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/43efc182fc7ab9aa1d2f1ca798e27d85c7132e1f.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Jonathan Cameron]

On Thu, 8 Nov 2018 13:44:17 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Thu, 2018-11-08 at 11:03 -0200, Giuliano Belinassi wrote:
> > Only the ad778x have the 'gain' status bit. Check it before updating
> > through a new variable is_ad778x in chip_info.
> >   
> 
> Looks good.
Alex, formal tags definitely preferred!  It's not as though a
looks good is any less of a review than an Ack, it's just better
hidden as people need to look at mailing list archives...

Jonathan

> 
> Alex
> 
> > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> > ---
> > Changes in v2:
> > 	- Squashed is_ad778x declaration commit with the ad778x checkage
> > 	- Changed is_ad778x type to bool
> >  
> >  drivers/staging/iio/adc/ad7780.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/adc/ad7780.c
> > b/drivers/staging/iio/adc/ad7780.c
> > index 91e016d534ed..9ec2b002539e 100644
> > --- a/drivers/staging/iio/adc/ad7780.c
> > +++ b/drivers/staging/iio/adc/ad7780.c
> > @@ -35,6 +35,7 @@ struct ad7780_chip_info {
> >  	struct iio_chan_spec	channel;
> >  	unsigned int		pattern_mask;
> >  	unsigned int		pattern;
> > +	bool			is_ad778x;
> >  };
> >  
> >  struct ad7780_state {
> > @@ -113,10 +114,12 @@ static int ad7780_postprocess_sample(struct
> > ad_sigma_delta *sigma_delta,
> >  	    ((raw_sample & chip_info->pattern_mask) != chip_info->pattern))
> >  		return -EIO;
> >  
> > -	if (raw_sample & AD7780_GAIN)
> > -		st->gain = 1;
> > -	else
> > -		st->gain = 128;
> > +	if (chip_info->is_ad778x) {
> > +		if (raw_sample & AD7780_GAIN)
> > +			st->gain = 1;
> > +		else
> > +			st->gain = 128;
> > +	}
> >  
> >  	return 0;
> >  }
> > @@ -135,21 +138,25 @@ static const struct ad7780_chip_info
> > ad7780_chip_info_tbl[] = {
> >  		.channel = AD7780_CHANNEL(12, 24),
> >  		.pattern = 0x5,
> >  		.pattern_mask = 0x7,
> > +		.is_ad778x = false,
> >  	},
> >  	[ID_AD7171] = {
> >  		.channel = AD7780_CHANNEL(16, 24),
> >  		.pattern = 0x5,
> >  		.pattern_mask = 0x7,
> > +		.is_ad778x = false,
> >  	},
> >  	[ID_AD7780] = {
> >  		.channel = AD7780_CHANNEL(24, 32),
> >  		.pattern = 0x1,
> >  		.pattern_mask = 0x3,
> > +		.is_ad778x = true,
> >  	},
> >  	[ID_AD7781] = {
> >  		.channel = AD7780_CHANNEL(20, 32),
> >  		.pattern = 0x1,
> >  		.pattern_mask = 0x3,
> > +		.is_ad778x = true,
> >  	},
> >  };
> >    

Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Jonathan Cameron]

On Fri, 9 Nov 2018 20:15:45 -0200
Giuliano Augusto Faulin Belinassi <giuliano.belinassi@usp.br> wrote:

> > Just some random though. Instead of introducing extra level of indentation you
> > can simply check whether is_ad778x is asserted and simply return.  
> 
> I agree that the patch would be smaller if I do that, but is it really
> an issue? If yes, then I will update the patch with this change
> 
> > Any reason for setting this explicitly? That's going to be false anyway  
> 
> It seems clearer to me :-)

Definitely marginal, but not a strong reason either way so I've
applied this as is.  If there were lots of instances of it I would
have agreed with Tomasz (both suggestions were good but Tomasz said,
minor!)

Jonathan

> On Thu, Nov 8, 2018 at 4:26 PM Tomasz Duszynski <tduszyns@gmail.com> wrote:
> >
> > Hi Giuliano,
> >
> > Comment inline.
> >
> > On 11/8/18 2:03 PM, Giuliano Belinassi wrote:  
> > > Only the ad778x have the 'gain' status bit. Check it before updating
> > > through a new variable is_ad778x in chip_info.
> > >
> > > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> > > ---
> > > Changes in v2:
> > >       - Squashed is_ad778x declaration commit with the ad778x checkage
> > >       - Changed is_ad778x type to bool
> > >
> > >  drivers/staging/iio/adc/ad7780.c | 15 +++++++++++----
> > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
> > > index 91e016d534ed..9ec2b002539e 100644
> > > --- a/drivers/staging/iio/adc/ad7780.c
> > > +++ b/drivers/staging/iio/adc/ad7780.c
> > > @@ -35,6 +35,7 @@ struct ad7780_chip_info {
> > >       struct iio_chan_spec    channel;
> > >       unsigned int            pattern_mask;
> > >       unsigned int            pattern;
> > > +     bool                    is_ad778x;
> > >  };
> > >
> > >  struct ad7780_state {
> > > @@ -113,10 +114,12 @@ static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta,
> > >           ((raw_sample & chip_info->pattern_mask) != chip_info->pattern))
> > >               return -EIO;
> > >
> > > -     if (raw_sample & AD7780_GAIN)
> > > -             st->gain = 1;
> > > -     else
> > > -             st->gain = 128;
> > > +     if (chip_info->is_ad778x) {
> > > +             if (raw_sample & AD7780_GAIN)
> > > +                     st->gain = 1;
> > > +             else
> > > +                     st->gain = 128;
> > > +     }  
> >
> > Just some random though. Instead of introducing extra level of indentation you
> > can simply check whether is_ad778x is asserted and simply return.
> >  
> > >
> > >       return 0;
> > >  }
> > > @@ -135,21 +138,25 @@ static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
> > >               .channel = AD7780_CHANNEL(12, 24),
> > >               .pattern = 0x5,
> > >               .pattern_mask = 0x7,
> > > +             .is_ad778x = false,  
> >
> > Any reason for setting this explicitly? That's going to be false anyway.
> >  
> > >       },
> > >       [ID_AD7171] = {
> > >               .channel = AD7780_CHANNEL(16, 24),
> > >               .pattern = 0x5,
> > >               .pattern_mask = 0x7,
> > > +             .is_ad778x = false,
> > >       },
> > >       [ID_AD7780] = {
> > >               .channel = AD7780_CHANNEL(24, 32),
> > >               .pattern = 0x1,
> > >               .pattern_mask = 0x3,
> > > +             .is_ad778x = true,
> > >       },
> > >       [ID_AD7781] = {
> > >               .channel = AD7780_CHANNEL(20, 32),
> > >               .pattern = 0x1,
> > >               .pattern_mask = 0x3,
> > > +             .is_ad778x = true,
> > >       },
> > >  };
> > >  
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> > To post to this group, send email to kernel-usp@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/55b5de74-a607-94b9-8c85-40658e38fbb5%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.  

Re: [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits

[Jonathan Cameron]

On Fri, 9 Nov 2018 20:18:58 -0200
Giuliano Augusto Faulin Belinassi <giuliano.belinassi@usp.br> wrote:

> Hi
> 
> >While I agree that it looks nicer to indent all these to the same level,
> >you also need to think about the fact that the kernel git repo is already
> >pretty big as-is, so it's a good idea if a patch adds as much code/semantic
> >value [as possible] with as little change [as possible] and with as good of
> >readability [as possible].  
> 
> Understood. But can't someone else submit another patch fixing these
> indentation issues? That would be another commit and more stuff to the
> repository.

Separating real changes from white space changes is much more important
from a reviewability point of view.  This patch is small enough that it
doesn't 'really matter' but I would have preferred it as a realignment patch
and a follow up patch making the real change.

Anyhow, it doesn't matter that much for such a small case, so applied to
the togreg branch of iio.git and pushed out as testing for the autobuilders
to play with it.

Thanks,

Jonathan


> On Thu, Nov 8, 2018 at 11:51 AM Ardelean, Alexandru
> <alexandru.Ardelean@analog.com> wrote:
> >
> > On Thu, 2018-11-08 at 11:03 -0200, Giuliano Belinassi wrote:  
> > > Previously, all pattern_masks and patterns in the chip_info table were
> > > hardcoded. Now they are generated using the PAT macros, as described in
> > > the datasheets.  
> >
> > One comment about indentation/whitespace.
> >
> > Rest looks good.
> >
> > Alex
> >  
> > >
> > > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> > > ---
> > > Changes in v2:
> > >       - Added the PATTERN and PATTERN_MASK macros
> > >       - Update BIT macros alignment to match with PATTERN
> > >       - Generate pattern mask with PATTERN_MASK macros.
> > >
> > > drivers/staging/iio/adc/ad7780.c | 40 +++++++++++++++++++-------------
> > >  1 file changed, 24 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7780.c
> > > b/drivers/staging/iio/adc/ad7780.c
> > > index 9ec2b002539e..ff8e3b2d0efc 100644
> > > --- a/drivers/staging/iio/adc/ad7780.c
> > > +++ b/drivers/staging/iio/adc/ad7780.c
> > > @@ -22,14 +22,22 @@
> > >  #include <linux/iio/sysfs.h>
> > >  #include <linux/iio/adc/ad_sigma_delta.h>
> > >
> > > -#define AD7780_RDY   BIT(7)
> > > -#define AD7780_FILTER        BIT(6)
> > > -#define AD7780_ERR   BIT(5)
> > > -#define AD7780_ID1   BIT(4)
> > > -#define AD7780_ID0   BIT(3)
> > > -#define AD7780_GAIN  BIT(2)
> > > -#define AD7780_PAT1  BIT(1)
> > > -#define AD7780_PAT0  BIT(0)
> > > +#define AD7780_RDY           BIT(7)
> > > +#define AD7780_FILTER                BIT(6)
> > > +#define AD7780_ERR           BIT(5)
> > > +#define AD7780_ID1           BIT(4)
> > > +#define AD7780_ID0           BIT(3)
> > > +#define AD7780_GAIN          BIT(2)
> > > +#define AD7780_PAT1          BIT(1)
> > > +#define AD7780_PAT0          BIT(0)  
> >
> > Changing indentation here doesn't add much value; it's mostly
> > patch/whitespace noise.
> >
> > While I agree that it looks nicer to indent all these to the same level,
> > you also need to think about the fact that the kernel git repo is already
> > pretty big as-is, so it's a good idea if a patch adds as much code/semantic
> > value [as possible] with as little change [as possible] and with as good of
> > readability [as possible].
> > [ Kind of sounds like a balance act between the 3 things ].
> >
> >  
> > > +
> > > +#define AD7780_PATTERN               (AD7780_PAT0)
> > > +#define AD7780_PATTERN_MASK  (AD7780_PAT0 | AD7780_PAT1)
> > > +
> > > +#define AD7170_PAT2          BIT(2)
> > > +
> > > +#define AD7170_PATTERN               (AD7780_PAT0 | AD7170_PAT2)
> > > +#define AD7170_PATTERN_MASK  (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2)
> > >
> > >  struct ad7780_chip_info {
> > >       struct iio_chan_spec    channel;
> > > @@ -136,26 +144,26 @@ static const struct ad_sigma_delta_info
> > > ad7780_sigma_delta_info = {
> > >  static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
> > >       [ID_AD7170] = {
> > >               .channel = AD7780_CHANNEL(12, 24),
> > > -             .pattern = 0x5,
> > > -             .pattern_mask = 0x7,
> > > +             .pattern = AD7170_PATTERN,
> > > +             .pattern_mask = AD7170_PATTERN_MASK,
> > >               .is_ad778x = false,
> > >       },
> > >       [ID_AD7171] = {
> > >               .channel = AD7780_CHANNEL(16, 24),
> > > -             .pattern = 0x5,
> > > -             .pattern_mask = 0x7,
> > > +             .pattern = AD7170_PATTERN,
> > > +             .pattern_mask = AD7170_PATTERN_MASK,
> > >               .is_ad778x = false,
> > >       },
> > >       [ID_AD7780] = {
> > >               .channel = AD7780_CHANNEL(24, 32),
> > > -             .pattern = 0x1,
> > > -             .pattern_mask = 0x3,
> > > +             .pattern = AD7780_PATTERN,
> > > +             .pattern_mask = AD7780_PATTERN_MASK,
> > >               .is_ad778x = true,
> > >       },
> > >       [ID_AD7781] = {
> > >               .channel = AD7780_CHANNEL(20, 32),
> > > -             .pattern = 0x1,
> > > -             .pattern_mask = 0x3,
> > > +             .pattern = AD7780_PATTERN,
> > > +             .pattern_mask = AD7780_PATTERN_MASK,
> > >               .is_ad778x = true,
> > >       },
> > >  };  
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> > To post to this group, send email to kernel-usp@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/43efc182fc7ab9aa1d2f1ca798e27d85c7132e1f.camel%40analog.com.
> > For more options, visit https://groups.google.com/d/optout.  

Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Ardelean, Alexandru]

On Sun, 2018-11-11 at 12:58 +0000, Jonathan Cameron wrote:
> On Thu, 8 Nov 2018 13:44:17 +0000
> "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:
> 
> > On Thu, 2018-11-08 at 11:03 -0200, Giuliano Belinassi wrote:
> > > Only the ad778x have the 'gain' status bit. Check it before updating
> > > through a new variable is_ad778x in chip_info.
> > >   
> > 
> > Looks good.
> 
> Alex, formal tags definitely preferred!  It's not as though a
> looks good is any less of a review than an Ack, it's just better
> hidden as people need to look at mailing list archives...
> 
> Jonathan
> 

Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

// Will remember that next time :)

Thanks
Alex

> > 
> > Alex
> > 
> > > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> > > ---
> > > Changes in v2:
> > > 	- Squashed is_ad778x declaration commit with the ad778x checkage
> > > 	- Changed is_ad778x type to bool
> > >  
> > >  drivers/staging/iio/adc/ad7780.c | 15 +++++++++++----
> > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/staging/iio/adc/ad7780.c
> > > b/drivers/staging/iio/adc/ad7780.c
> > > index 91e016d534ed..9ec2b002539e 100644
> > > --- a/drivers/staging/iio/adc/ad7780.c
> > > +++ b/drivers/staging/iio/adc/ad7780.c
> > > @@ -35,6 +35,7 @@ struct ad7780_chip_info {
> > >  	struct iio_chan_spec	channel;
> > >  	unsigned int		pattern_mask;
> > >  	unsigned int		pattern;
> > > +	bool			is_ad778x;
> > >  };
> > >  
> > >  struct ad7780_state {
> > > @@ -113,10 +114,12 @@ static int ad7780_postprocess_sample(struct
> > > ad_sigma_delta *sigma_delta,
> > >  	    ((raw_sample & chip_info->pattern_mask) != chip_info->pattern))
> > >  		return -EIO;
> > >  
> > > -	if (raw_sample & AD7780_GAIN)
> > > -		st->gain = 1;
> > > -	else
> > > -		st->gain = 128;
> > > +	if (chip_info->is_ad778x) {
> > > +		if (raw_sample & AD7780_GAIN)
> > > +			st->gain = 1;
> > > +		else
> > > +			st->gain = 128;
> > > +	}
> > >  
> > >  	return 0;
> > >  }
> > > @@ -135,21 +138,25 @@ static const struct ad7780_chip_info
> > > ad7780_chip_info_tbl[] = {
> > >  		.channel = AD7780_CHANNEL(12, 24),
> > >  		.pattern = 0x5,
> > >  		.pattern_mask = 0x7,
> > > +		.is_ad778x = false,
> > >  	},
> > >  	[ID_AD7171] = {
> > >  		.channel = AD7780_CHANNEL(16, 24),
> > >  		.pattern = 0x5,
> > >  		.pattern_mask = 0x7,
> > > +		.is_ad778x = false,
> > >  	},
> > >  	[ID_AD7780] = {
> > >  		.channel = AD7780_CHANNEL(24, 32),
> > >  		.pattern = 0x1,
> > >  		.pattern_mask = 0x3,
> > > +		.is_ad778x = true,
> > >  	},
> > >  	[ID_AD7781] = {
> > >  		.channel = AD7780_CHANNEL(20, 32),
> > >  		.pattern = 0x1,
> > >  		.pattern_mask = 0x3,
> > > +		.is_ad778x = true,
> > >  	},
> > >  };
> > >    
> 
> 

Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Ardelean, Alexandru]

T24gU3VuLCAyMDE4LTExLTExIGF0IDEyOjU4ICswMDAwLCBKb25hdGhhbiBDYW1lcm9uIHdyb3Rl
Og0KPiBPbiBUaHUsIDggTm92IDIwMTggMTM6NDQ6MTcgKzAwMDANCj4gIkFyZGVsZWFuLCBBbGV4
YW5kcnUiIDxhbGV4YW5kcnUuQXJkZWxlYW5AYW5hbG9nLmNvbT4gd3JvdGU6DQo+IA0KPiA+IE9u
IFRodSwgMjAxOC0xMS0wOCBhdCAxMTowMyAtMDIwMCwgR2l1bGlhbm8gQmVsaW5hc3NpIHdyb3Rl
Og0KPiA+ID4gT25seSB0aGUgYWQ3Nzh4IGhhdmUgdGhlICdnYWluJyBzdGF0dXMgYml0LiBDaGVj
ayBpdCBiZWZvcmUgdXBkYXRpbmcNCj4gPiA+IHRocm91Z2ggYSBuZXcgdmFyaWFibGUgaXNfYWQ3
Nzh4IGluIGNoaXBfaW5mby4NCj4gPiA+ICAgDQo+ID4gDQo+ID4gTG9va3MgZ29vZC4NCj4gDQo+
IEFsZXgsIGZvcm1hbCB0YWdzIGRlZmluaXRlbHkgcHJlZmVycmVkISAgSXQncyBub3QgYXMgdGhv
dWdoIGENCj4gbG9va3MgZ29vZCBpcyBhbnkgbGVzcyBvZiBhIHJldmlldyB0aGFuIGFuIEFjaywg
aXQncyBqdXN0IGJldHRlcg0KPiBoaWRkZW4gYXMgcGVvcGxlIG5lZWQgdG8gbG9vayBhdCBtYWls
aW5nIGxpc3QgYXJjaGl2ZXMuLi4NCj4gDQo+IEpvbmF0aGFuDQo+IA0KDQpBY2tlZC1ieTogQWxl
eGFuZHJ1IEFyZGVsZWFuIDxhbGV4YW5kcnUuYXJkZWxlYW5AYW5hbG9nLmNvbT4NCg0KLy8gV2ls
bCByZW1lbWJlciB0aGF0IG5leHQgdGltZSA6KQ0KDQpUaGFua3MNCkFsZXgNCg0KPiA+IA0KPiA+
IEFsZXgNCj4gPiANCj4gPiA+IFNpZ25lZC1vZmYtYnk6IEdpdWxpYW5vIEJlbGluYXNzaSA8Z2l1
bGlhbm8uYmVsaW5hc3NpQHVzcC5icj4NCj4gPiA+IC0tLQ0KPiA+ID4gQ2hhbmdlcyBpbiB2MjoN
Cj4gPiA+IAktIFNxdWFzaGVkIGlzX2FkNzc4eCBkZWNsYXJhdGlvbiBjb21taXQgd2l0aCB0aGUg
YWQ3Nzh4IGNoZWNrYWdlDQo+ID4gPiAJLSBDaGFuZ2VkIGlzX2FkNzc4eCB0eXBlIHRvIGJvb2wN
Cj4gPiA+ICANCj4gPiA+ICBkcml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc3ODAuYyB8IDE1ICsr
KysrKysrKysrLS0tLQ0KPiA+ID4gIDEgZmlsZSBjaGFuZ2VkLCAxMSBpbnNlcnRpb25zKCspLCA0
IGRlbGV0aW9ucygtKQ0KPiA+ID4gDQo+ID4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zdGFnaW5n
L2lpby9hZGMvYWQ3NzgwLmMNCj4gPiA+IGIvZHJpdmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3Nzgw
LmMNCj4gPiA+IGluZGV4IDkxZTAxNmQ1MzRlZC4uOWVjMmIwMDI1MzllIDEwMDY0NA0KPiA+ID4g
LS0tIGEvZHJpdmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3NzgwLmMNCj4gPiA+ICsrKyBiL2RyaXZl
cnMvc3RhZ2luZy9paW8vYWRjL2FkNzc4MC5jDQo+ID4gPiBAQCAtMzUsNiArMzUsNyBAQCBzdHJ1
Y3QgYWQ3NzgwX2NoaXBfaW5mbyB7DQo+ID4gPiAgCXN0cnVjdCBpaW9fY2hhbl9zcGVjCWNoYW5u
ZWw7DQo+ID4gPiAgCXVuc2lnbmVkIGludAkJcGF0dGVybl9tYXNrOw0KPiA+ID4gIAl1bnNpZ25l
ZCBpbnQJCXBhdHRlcm47DQo+ID4gPiArCWJvb2wJCQlpc19hZDc3OHg7DQo+ID4gPiAgfTsNCj4g
PiA+ICANCj4gPiA+ICBzdHJ1Y3QgYWQ3NzgwX3N0YXRlIHsNCj4gPiA+IEBAIC0xMTMsMTAgKzEx
NCwxMiBAQCBzdGF0aWMgaW50IGFkNzc4MF9wb3N0cHJvY2Vzc19zYW1wbGUoc3RydWN0DQo+ID4g
PiBhZF9zaWdtYV9kZWx0YSAqc2lnbWFfZGVsdGEsDQo+ID4gPiAgCSAgICAoKHJhd19zYW1wbGUg
JiBjaGlwX2luZm8tPnBhdHRlcm5fbWFzaykgIT0gY2hpcF9pbmZvLT5wYXR0ZXJuKSkNCj4gPiA+
ICAJCXJldHVybiAtRUlPOw0KPiA+ID4gIA0KPiA+ID4gLQlpZiAocmF3X3NhbXBsZSAmIEFENzc4
MF9HQUlOKQ0KPiA+ID4gLQkJc3QtPmdhaW4gPSAxOw0KPiA+ID4gLQllbHNlDQo+ID4gPiAtCQlz
dC0+Z2FpbiA9IDEyODsNCj4gPiA+ICsJaWYgKGNoaXBfaW5mby0+aXNfYWQ3Nzh4KSB7DQo+ID4g
PiArCQlpZiAocmF3X3NhbXBsZSAmIEFENzc4MF9HQUlOKQ0KPiA+ID4gKwkJCXN0LT5nYWluID0g
MTsNCj4gPiA+ICsJCWVsc2UNCj4gPiA+ICsJCQlzdC0+Z2FpbiA9IDEyODsNCj4gPiA+ICsJfQ0K
PiA+ID4gIA0KPiA+ID4gIAlyZXR1cm4gMDsNCj4gPiA+ICB9DQo+ID4gPiBAQCAtMTM1LDIxICsx
MzgsMjUgQEAgc3RhdGljIGNvbnN0IHN0cnVjdCBhZDc3ODBfY2hpcF9pbmZvDQo+ID4gPiBhZDc3
ODBfY2hpcF9pbmZvX3RibFtdID0gew0KPiA+ID4gIAkJLmNoYW5uZWwgPSBBRDc3ODBfQ0hBTk5F
TCgxMiwgMjQpLA0KPiA+ID4gIAkJLnBhdHRlcm4gPSAweDUsDQo+ID4gPiAgCQkucGF0dGVybl9t
YXNrID0gMHg3LA0KPiA+ID4gKwkJLmlzX2FkNzc4eCA9IGZhbHNlLA0KPiA+ID4gIAl9LA0KPiA+
ID4gIAlbSURfQUQ3MTcxXSA9IHsNCj4gPiA+ICAJCS5jaGFubmVsID0gQUQ3NzgwX0NIQU5ORUwo
MTYsIDI0KSwNCj4gPiA+ICAJCS5wYXR0ZXJuID0gMHg1LA0KPiA+ID4gIAkJLnBhdHRlcm5fbWFz
ayA9IDB4NywNCj4gPiA+ICsJCS5pc19hZDc3OHggPSBmYWxzZSwNCj4gPiA+ICAJfSwNCj4gPiA+
ICAJW0lEX0FENzc4MF0gPSB7DQo+ID4gPiAgCQkuY2hhbm5lbCA9IEFENzc4MF9DSEFOTkVMKDI0
LCAzMiksDQo+ID4gPiAgCQkucGF0dGVybiA9IDB4MSwNCj4gPiA+ICAJCS5wYXR0ZXJuX21hc2sg
PSAweDMsDQo+ID4gPiArCQkuaXNfYWQ3Nzh4ID0gdHJ1ZSwNCj4gPiA+ICAJfSwNCj4gPiA+ICAJ
W0lEX0FENzc4MV0gPSB7DQo+ID4gPiAgCQkuY2hhbm5lbCA9IEFENzc4MF9DSEFOTkVMKDIwLCAz
MiksDQo+ID4gPiAgCQkucGF0dGVybiA9IDB4MSwNCj4gPiA+ICAJCS5wYXR0ZXJuX21hc2sgPSAw
eDMsDQo+ID4gPiArCQkuaXNfYWQ3Nzh4ID0gdHJ1ZSwNCj4gPiA+ICAJfSwNCj4gPiA+ICB9Ow0K
PiA+ID4gICAgDQo+IA0KPiANCg==

Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update

[Jonathan Cameron]

On Mon, 12 Nov 2018 07:57:58 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Sun, 2018-11-11 at 12:58 +0000, Jonathan Cameron wrote:
> > On Thu, 8 Nov 2018 13:44:17 +0000
> > "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:
> >   
> > > On Thu, 2018-11-08 at 11:03 -0200, Giuliano Belinassi wrote:  
> > > > Only the ad778x have the 'gain' status bit. Check it before updating
> > > > through a new variable is_ad778x in chip_info.
> > > >     
> > > 
> > > Looks good.  
> > 
> > Alex, formal tags definitely preferred!  It's not as though a
> > looks good is any less of a review than an Ack, it's just better
> > hidden as people need to look at mailing list archives...
> > 
> > Jonathan
> >   
> 
> Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
I haven't pushed out togreg yet so can still rebase.

Added. Thanks,

Jonathan

> 
> // Will remember that next time :)
> 
> Thanks
> Alex
> 
> > > 
> > > Alex
> > >   
> > > > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br>
> > > > ---
> > > > Changes in v2:
> > > > 	- Squashed is_ad778x declaration commit with the ad778x checkage
> > > > 	- Changed is_ad778x type to bool
> > > >  
> > > >  drivers/staging/iio/adc/ad7780.c | 15 +++++++++++----
> > > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/iio/adc/ad7780.c
> > > > b/drivers/staging/iio/adc/ad7780.c
> > > > index 91e016d534ed..9ec2b002539e 100644
> > > > --- a/drivers/staging/iio/adc/ad7780.c
> > > > +++ b/drivers/staging/iio/adc/ad7780.c
> > > > @@ -35,6 +35,7 @@ struct ad7780_chip_info {
> > > >  	struct iio_chan_spec	channel;
> > > >  	unsigned int		pattern_mask;
> > > >  	unsigned int		pattern;
> > > > +	bool			is_ad778x;
> > > >  };
> > > >  
> > > >  struct ad7780_state {
> > > > @@ -113,10 +114,12 @@ static int ad7780_postprocess_sample(struct
> > > > ad_sigma_delta *sigma_delta,
> > > >  	    ((raw_sample & chip_info->pattern_mask) != chip_info->pattern))
> > > >  		return -EIO;
> > > >  
> > > > -	if (raw_sample & AD7780_GAIN)
> > > > -		st->gain = 1;
> > > > -	else
> > > > -		st->gain = 128;
> > > > +	if (chip_info->is_ad778x) {
> > > > +		if (raw_sample & AD7780_GAIN)
> > > > +			st->gain = 1;
> > > > +		else
> > > > +			st->gain = 128;
> > > > +	}
> > > >  
> > > >  	return 0;
> > > >  }
> > > > @@ -135,21 +138,25 @@ static const struct ad7780_chip_info
> > > > ad7780_chip_info_tbl[] = {
> > > >  		.channel = AD7780_CHANNEL(12, 24),
> > > >  		.pattern = 0x5,
> > > >  		.pattern_mask = 0x7,
> > > > +		.is_ad778x = false,
> > > >  	},
> > > >  	[ID_AD7171] = {
> > > >  		.channel = AD7780_CHANNEL(16, 24),
> > > >  		.pattern = 0x5,
> > > >  		.pattern_mask = 0x7,
> > > > +		.is_ad778x = false,
> > > >  	},
> > > >  	[ID_AD7780] = {
> > > >  		.channel = AD7780_CHANNEL(24, 32),
> > > >  		.pattern = 0x1,
> > > >  		.pattern_mask = 0x3,
> > > > +		.is_ad778x = true,
> > > >  	},
> > > >  	[ID_AD7781] = {
> > > >  		.channel = AD7780_CHANNEL(20, 32),
> > > >  		.pattern = 0x1,
> > > >  		.pattern_mask = 0x3,
> > > > +		.is_ad778x = true,
> > > >  	},
> > > >  };
> > > >      
> > 
> >