Hi Andy,

First, thanks for the great review. I agree with all of your comments, just one question/comment inline.

Best Regards,
Tomislav

-----Original Message-----
From: Andy Shevchenko <redacted>
Sent: 21 January 2021 17:42
To: Denis, Tomislav AVL DiTEST <redacted>
Cc: Jonathan Cameron <jic23@kernel.org>; linux-iio <redacted>;
devicetree [off-list ref]
Subject: Re: [PATCH v3 1/2] iio: adc: Add driver for Texas Instruments
ADS131E0x ADC family

On Tue, Jan 12, 2021 at 2:37 PM [off-list ref] wrote:

quoted

From: Tomislav Denis <redacted>

The ADS131E0x are a family of multichannel, simultaneous sampling,
24-bit, delta-sigma, analog-to-digital converters (ADCs) with a
built-in programmable gain amplifier (PGA), internal reference and an
onboard oscillator.

...

quoted

+static const struct ads131e08_data_rate_desc ads131e08_data_rate_tbl[] = {
+       { .rate = 64,   .reg = 0x00 },
+       { .rate = 32,   .reg = 0x01 },
+       { .rate = 16,   .reg = 0x02 },
+       { .rate = 8,    .reg = 0x03 },
+       { .rate = 4,    .reg = 0x04 },
+       { .rate = 2,    .reg = 0x05 },
+       { .rate = 1,    .reg = 0x06 },
+};

Can't you use simple bit operations on this?

rate = BIT(6 - index)
reg = index

...

quoted

+static const struct ads131e08_pga_gain_desc ads131e08_pga_gain_tbl[] = {
+       { .gain = 1,   .reg = 0x01 },
+       { .gain = 2,   .reg = 0x02 },

reg == 3 valid?

quoted

+       { .gain = 4,   .reg = 0x04 },
+       { .gain = 8,   .reg = 0x05 },
+       { .gain = 12,  .reg = 0x06 },
+};

Also can be changed by formula, but I remember that in some cases tables are
preferable.

I would like to keep those tables as they are, because I find them more intuitive and easy understandable!?

quoted

+static int ads131e08_exec_cmd(struct ads131e08_state *st, u8 cmd) {
+       int ret;
+

....

--
With Best Regards,
Andy Shevchenko