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.
> + * 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.
> + */
> +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;
> +}
> +
> static int ad4030_conversion(struct ad4030_state *st, const struct iio_chan_spec *chan)
> {
> unsigned int bytes_to_read = (BITS_TO_BYTES(chan->scan_type.realbits) +
> @@ -437,10 +481,16 @@ static int ad4030_conversion(struct ad4030_state *st, const struct iio_chan_spec
> ndelay(AD4030_TCONV_NS);
>
> ret = spi_sync_transfer(st->spi, &xfer, 1);
> - if (ret || (st->mode != AD4030_OUT_DATA_MD_16_DIFF_8_COM &&
> - st->mode != AD4030_OUT_DATA_MD_24_DIFF_8_COM))
> + if (ret)
> return ret;
>
> + if (st->chip->num_channels == 2)
> + ad4030_extract_interleaved(st->rx_data.raw, st->rx_data.val);
> +
> + if (st->mode != AD4030_OUT_DATA_MD_16_DIFF_8_COM &&
> + st->mode != AD4030_OUT_DATA_MD_24_DIFF_8_COM)
> + return 0;
If you make the suggested split of the error and good paths from patch 2 review
this will be a simpler diff.
> +
> byte_index = st->chip->precision_bits == 16 ? 3 : 4;
> for (i = 0; i < st->chip->num_channels; i++)
> st->rx_data.common[i] = ((u8 *)&st->rx_data.val[i])[byte_index];
> @@ -776,12 +826,25 @@ static int ad4030_detect_chip_info(const struct ad4030_state *st)
>
> static int ad4030_config(struct ad4030_state *st)
> {
> + int ret;
> + u8 reg_modes = 0;
Seems you can just use = below instead of |=
> +
> st->min_offset = (int)BIT(st->chip->precision_bits) * -1;
> st->max_offset = BIT(st->chip->precision_bits) - 1;
> st->offset_avail[0] = st->min_offset;
> st->offset_avail[1] = 1;
> st->offset_avail[2] = st->max_offset;
>
> + if (st->chip->num_channels > 1)
> + reg_modes |= FIELD_PREP(AD4030_REG_MODES_MASK_LANE_MODE,
> + AD4030_LANE_MD_INTERLEAVED);
> + else
> + reg_modes |= FIELD_PREP(AD4030_REG_MODES_MASK_LANE_MODE, AD4030_LANE_MD_1_PER_CH);
wrap that line. Aim for sub 80 chars.
> +
> + ret = regmap_write(st->regmap, AD4030_REG_MODES, reg_modes);
> + if (ret)
> + return ret;
> +
> if (st->vio_uv < AD4030_VIO_THRESHOLD_UV)
> return regmap_write(st->regmap, AD4030_REG_IO,
> AD4030_REG_IO_MASK_IO2X);
> @@ -863,8 +926,14 @@ static const unsigned long ad4030_channel_masks[] = {
> 0,
> };
>
Thanks,
Jonathan
