| Message ID | e7f1b071157abbb92d5cb42f310fb471ea43ff01.1541615978.git.giuliano.belinassi@usp.br (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: iio: ad7780: pattern generation and gain update | expand |
On Wed, 2018-11-07 at 16:49 -0200, Giuliano Belinassi wrote: > This patch allows further checking of whatever the chip is (ad778x or > ad717x). Hey, The patch looks good overall. I only have one nitpick for this patch. See inline. And you can squash this patch with patch `[PATCH 2/3] staging: iio: ad7780: check if ad778x before gain update`. In fact, the title of the squashed patch can just be `staging: iio: ad7780: check if ad778x before gain update` ; because the code in this patch implies that it's used to check if the device is an ad778x chip. This patch doesn't have much value on it's own without the 2nd patch, and you can do them in a single go. /* * Note: yes, I know that these subtle semantics between patch * splitting & squashing can be a bit annoying ; I don't have a general * recommendation for them, other than just to keep sending patches */ Thanks Alex > > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > --- > drivers/staging/iio/adc/ad7780.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/staging/iio/adc/ad7780.c > b/drivers/staging/iio/adc/ad7780.c > index 91e016d534ed..6e51bfdb076a 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; > + u8 is_ad778x; You could make this `bool` type since you are assigning `true/false` values to this field. It's generally good to be consistent between type names & type values when using them [even though in C, these are pretty much the same]. > }; > > struct ad7780_state { > @@ -135,21 +136,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, > }, > }; >
diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c index 91e016d534ed..6e51bfdb076a 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; + u8 is_ad778x; }; struct ad7780_state { @@ -135,21 +136,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, }, };
This patch allows further checking of whatever the chip is (ad778x or ad717x). Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> --- drivers/staging/iio/adc/ad7780.c | 5 +++++ 1 file changed, 5 insertions(+)