Re: [PATCH RFC 4/5] iio: adc: ad4030: add support for ad4630-24 and ad4630-16

[Esteban Blanc]

On Sat Jun 29, 2024 at 6:55 PM CEST, Jonathan Cameron wrote:
> On Thu, 27 Jun 2024 13:59:15 +0200
> Esteban Blanc <eblanc@xxxxxxxxxxxx> wrote:
>
> > AD4630-24 and AD4630-16 are 2 channels ADCs. Both channels are
> > interleaved bit per bit on SDO line.
> > 
> > Signed-off-by: Esteban Blanc <eblanc@xxxxxxxxxxxx>
> > ---
> >  drivers/iio/adc/ad4030.c | 130 +++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 126 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> > index 1bcbcbd40a45..09d2f6d8cfe6 100644
> > --- a/drivers/iio/adc/ad4030.c
> > +++ b/drivers/iio/adc/ad4030.c
> > @@ -32,6 +32,8 @@
> >  #define AD4030_REG_PRODUCT_ID_H					0x05
> >  #define AD4030_REG_CHIP_GRADE					0x06
> >  #define     AD4030_REG_CHIP_GRADE_AD4030_24_GRADE		0x10
> > +#define     AD4030_REG_CHIP_GRADE_AD4630_16_GRADE		0x03
> > +#define     AD4030_REG_CHIP_GRADE_AD4630_24_GRADE		0x00
> >  #define     AD4030_REG_CHIP_GRADE_MASK_CHIP_GRADE		GENMASK(7, 3)
> >  #define AD4030_REG_SCRATCH_PAD					0x0A
> >  #define AD4030_REG_SPI_REVISION					0x0B
> > @@ -391,7 +393,10 @@ static int ad4030_set_avg_frame_len(struct iio_dev *dev, unsigned int avg_len)
> >  static bool ad4030_is_common_byte_asked(struct ad4030_state *st,
> >  					unsigned int mask)
> >  {
> > -	return mask & BIT(st->chip->num_channels);
> > +	if (st->chip->num_channels == 1)
> > +		return mask & BIT(st->chip->num_channels);
> > +
> > +	return mask & GENMASK(st->chip->num_channels + 1, st->chip->num_channels);
> >  }
> >  
> >  static int ad4030_set_mode(struct iio_dev *indio_dev, unsigned long mask)
> > @@ -412,6 +417,45 @@ static int ad4030_set_mode(struct iio_dev *indio_dev, unsigned long mask)
> >  				st->mode);
> >  }
> >  
> > +/*
> > + * @brief Descramble 2 32bits numbers out of a 64bits. The bits are interleaved: 1 bit for first
> line wrap at 80 chars unless good reason to be longer.
Sure.
> > + * number, 1 bit for the second, and so on...
>
> Do you have a reference for the alg used? 
> Google fed me a bunch of options for a perfect unshuffle 
> though it is probably microarch dependent.

I used this IIRC: https://stackoverflow.com/a/3233173
I adjusted the masks and shifts to get both the high and low parts of a
byte. I'm also doing both number at the same time.

> > + */
> > +static void ad4030_extract_interleaved(u8 *src, u32 *out)
> > +{
> > +	u8 h0, h1, l0, l1;
> > +	u32 out0, out1;
> > +	u8 *out0_raw = (u8 *)&out0;
> > +	u8 *out1_raw = (u8 *)&out1;
> > +
> > +	for (int i = 0; i < 4; i++) {
> > +		h0 = src[i * 2];
> > +		l1 = src[i * 2 + 1];
> > +		h1 = h0 << 1;
> > +		l0 = l1 >> 1;
> > +
> > +		h0 &= 0xAA;
> > +		l0 &= 0x55;
> > +		h1 &= 0xAA;
> > +		l1 &= 0x55;
> > +
> > +		h0 = (h0 | h0 << 001) & 0xCC;
> > +		h1 = (h1 | h1 << 001) & 0xCC;
> > +		l0 = (l0 | l0 >> 001) & 0x33;
> > +		l1 = (l1 | l1 >> 001) & 0x33;
> > +		h0 = (h0 | h0 << 002) & 0xF0;
> > +		h1 = (h1 | h1 << 002) & 0xF0;
> > +		l0 = (l0 | l0 >> 002) & 0x0F;
> > +		l1 = (l1 | l1 >> 002) & 0x0F;
> > +
> > +		out0_raw[i] = h0 | l0;
> > +		out1_raw[i] = h1 | l1;
> > +	}
> > +
> > +	out[0] = out0;
> > +	out[1] = out1;
> > +}

All the other comments will be fixed for V2.

Best regards,

-- 
Esteban "Skallwar" Blanc
BayLibre