[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 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 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

[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,
> > > >  	},
> > > >  };
> > > >      
> > 
> >