* [PATCH v2 0/2] pattern generation and gain update @ 2018-11-08 13:03 Giuliano Belinassi 2018-11-08 13:03 ` [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before " Giuliano Belinassi 2018-11-08 13:03 ` [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits Giuliano Belinassi 0 siblings, 2 replies; 16+ messages in thread From: Giuliano Belinassi @ 2018-11-08 13:03 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, renatogeh Cc: linux-iio, devel, linux-kernel, kernel-usp 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update 2018-11-08 13:03 [PATCH v2 0/2] pattern generation and gain update Giuliano Belinassi @ 2018-11-08 13:03 ` Giuliano Belinassi 2018-11-08 13:44 ` Ardelean, Alexandru 2018-11-08 18:26 ` Tomasz Duszynski 2018-11-08 13:03 ` [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits Giuliano Belinassi 1 sibling, 2 replies; 16+ messages in thread From: Giuliano Belinassi @ 2018-11-08 13:03 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, renatogeh Cc: linux-iio, devel, linux-kernel, kernel-usp 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update 2018-11-08 13:03 ` [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before " Giuliano Belinassi @ 2018-11-08 13:44 ` Ardelean, Alexandru 2018-11-08 18:26 ` Tomasz Duszynski 1 sibling, 0 replies; 16+ messages in thread From: Ardelean, Alexandru @ 2018-11-08 13:44 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-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, > }, > }; > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update @ 2018-11-08 13:44 ` Ardelean, Alexandru 0 siblings, 0 replies; 16+ messages in thread From: Ardelean, Alexandru @ 2018-11-08 13:44 UTC (permalink / raw) To: lars, knaack.h, jic23, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh Cc: linux-kernel, linux-iio, devel, kernel-usp 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== ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update 2018-11-08 13:44 ` Ardelean, Alexandru (?) @ 2018-11-11 12:58 ` Jonathan Cameron 2018-11-12 7:57 ` Ardelean, Alexandru -1 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2018-11-11 12:58 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, 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, > > }, > > }; > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update 2018-11-11 12:58 ` Jonathan Cameron @ 2018-11-12 7:57 ` Ardelean, Alexandru 0 siblings, 0 replies; 16+ messages in thread From: Ardelean, Alexandru @ 2018-11-12 7:57 UTC (permalink / raw) To: jic23 Cc: kernel-usp, linux-kernel, lars, knaack.h, Hennerich, Michael, linux-iio, devel, renatogeh, pmeerw, giuliano.belinassi, gregkh 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, > > > }, > > > }; > > > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update @ 2018-11-12 7:57 ` Ardelean, Alexandru 0 siblings, 0 replies; 16+ messages in thread From: Ardelean, Alexandru @ 2018-11-12 7:57 UTC (permalink / raw) To: jic23 Cc: kernel-usp, linux-kernel, lars, knaack.h, Hennerich, Michael, linux-iio, devel, renatogeh, pmeerw, giuliano.belinassi, gregkh 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== ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update 2018-11-12 7:57 ` Ardelean, Alexandru (?) @ 2018-11-16 18:32 ` Jonathan Cameron -1 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2018-11-16 18:32 UTC (permalink / raw) To: Ardelean, Alexandru Cc: kernel-usp, linux-kernel, lars, knaack.h, Hennerich, Michael, linux-iio, devel, renatogeh, pmeerw, giuliano.belinassi, gregkh 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, > > > > }, > > > > }; > > > > > > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update 2018-11-08 13:03 ` [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before " Giuliano Belinassi 2018-11-08 13:44 ` Ardelean, Alexandru @ 2018-11-08 18:26 ` Tomasz Duszynski 2018-11-09 22:15 ` Giuliano Augusto Faulin Belinassi 1 sibling, 1 reply; 16+ messages in thread From: Tomasz Duszynski @ 2018-11-08 18:26 UTC (permalink / raw) To: Giuliano Belinassi, lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, renatogeh Cc: linux-iio, devel, linux-kernel, kernel-usp 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, > }, > }; > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update 2018-11-08 18:26 ` Tomasz Duszynski @ 2018-11-09 22:15 ` Giuliano Augusto Faulin Belinassi 2018-11-11 13:00 ` Jonathan Cameron 0 siblings, 1 reply; 16+ messages in thread From: Giuliano Augusto Faulin Belinassi @ 2018-11-09 22:15 UTC (permalink / raw) To: tduszyns Cc: giuliano.belinassi, lars, Michael.Hennerich, jic23, knaack.h, Peter Meerwald-Stadler, gregkh, Renato Geh, linux-iio, devel, linux-kernel, Kernel USP > 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before gain update 2018-11-09 22:15 ` Giuliano Augusto Faulin Belinassi @ 2018-11-11 13:00 ` Jonathan Cameron 0 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2018-11-11 13:00 UTC (permalink / raw) To: Giuliano Augusto Faulin Belinassi Cc: tduszyns, giuliano.belinassi, lars, Michael.Hennerich, knaack.h, Peter Meerwald-Stadler, gregkh, Renato Geh, linux-iio, devel, linux-kernel, Kernel USP 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits 2018-11-08 13:03 [PATCH v2 0/2] pattern generation and gain update Giuliano Belinassi 2018-11-08 13:03 ` [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before " Giuliano Belinassi @ 2018-11-08 13:03 ` Giuliano Belinassi 2018-11-08 13:51 ` Ardelean, Alexandru 1 sibling, 1 reply; 16+ messages in thread From: Giuliano Belinassi @ 2018-11-08 13:03 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, renatogeh Cc: linux-iio, devel, linux-kernel, kernel-usp 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits 2018-11-08 13:03 ` [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits Giuliano Belinassi @ 2018-11-08 13:51 ` Ardelean, Alexandru 0 siblings, 0 replies; 16+ messages in thread From: Ardelean, Alexandru @ 2018-11-08 13:51 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-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, > }, > }; ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits @ 2018-11-08 13:51 ` Ardelean, Alexandru 0 siblings, 0 replies; 16+ messages in thread From: Ardelean, Alexandru @ 2018-11-08 13:51 UTC (permalink / raw) To: lars, knaack.h, jic23, Hennerich, Michael, renatogeh, giuliano.belinassi, pmeerw, gregkh Cc: linux-kernel, linux-iio, devel, kernel-usp 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= ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits 2018-11-08 13:51 ` Ardelean, Alexandru (?) @ 2018-11-09 22:18 ` Giuliano Augusto Faulin Belinassi 2018-11-11 13:04 ` Jonathan Cameron -1 siblings, 1 reply; 16+ messages in thread From: Giuliano Augusto Faulin Belinassi @ 2018-11-09 22:18 UTC (permalink / raw) To: alexandru.Ardelean Cc: lars, knaack.h, jic23, Michael.Hennerich, Renato Geh, giuliano.belinassi, Peter Meerwald-Stadler, gregkh, linux-kernel, linux-iio, devel, Kernel USP 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits 2018-11-09 22:18 ` Giuliano Augusto Faulin Belinassi @ 2018-11-11 13:04 ` Jonathan Cameron 0 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2018-11-11 13:04 UTC (permalink / raw) To: Giuliano Augusto Faulin Belinassi Cc: alexandru.Ardelean, lars, knaack.h, Michael.Hennerich, Renato Geh, giuliano.belinassi, Peter Meerwald-Stadler, gregkh, linux-kernel, linux-iio, devel, Kernel USP 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-11-16 18:32 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-08 13:03 [PATCH v2 0/2] pattern generation and gain update Giuliano Belinassi 2018-11-08 13:03 ` [PATCH v2 1/2] staging: iio: ad7780: check if ad778x before " Giuliano Belinassi 2018-11-08 13:44 ` Ardelean, Alexandru 2018-11-08 13:44 ` Ardelean, Alexandru 2018-11-11 12:58 ` Jonathan Cameron 2018-11-12 7:57 ` Ardelean, Alexandru 2018-11-12 7:57 ` Ardelean, Alexandru 2018-11-16 18:32 ` Jonathan Cameron 2018-11-08 18:26 ` Tomasz Duszynski 2018-11-09 22:15 ` Giuliano Augusto Faulin Belinassi 2018-11-11 13:00 ` Jonathan Cameron 2018-11-08 13:03 ` [PATCH v2 2/2] staging: iio: ad7780: generates pattern_mask from PAT bits Giuliano Belinassi 2018-11-08 13:51 ` Ardelean, Alexandru 2018-11-08 13:51 ` Ardelean, Alexandru 2018-11-09 22:18 ` Giuliano Augusto Faulin Belinassi 2018-11-11 13:04 ` Jonathan Cameron
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.