From: "Denis, Tomislav AVL DiTEST" <Tomislav.Denis@avl.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
linux-iio <linux-iio@vger.kernel.org>,
devicetree <devicetree@vger.kernel.org>
Subject: RE: [PATCH v3 1/2] iio: adc: Add driver for Texas Instruments ADS131E0x ADC family
Date: Sat, 23 Jan 2021 08:28:42 +0000 [thread overview]
Message-ID: <7474dd07191545b0bb9ae091e54c2013@avl.com> (raw)
In-Reply-To: <CAHp75VfNweJyXgzCcvZUWj=3mCVkP=+HaFmG2XaKmKP1cATa=Q@mail.gmail.com>
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 <andy.shevchenko@gmail.com>
> Sent: 21 January 2021 17:42
> To: Denis, Tomislav AVL DiTEST <Tomislav.Denis@avl.com>
> Cc: Jonathan Cameron <jic23@kernel.org>; linux-iio <linux-iio@vger.kernel.org>;
> devicetree <devicetree@vger.kernel.org>
> 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 <tomislav.denis@avl.com> wrote:
> >
> > From: Tomislav Denis <tomislav.denis@avl.com>
> >
> > 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.
>
...
>
> > +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
>
> ...
>
> > +static const struct ads131e08_pga_gain_desc ads131e08_pga_gain_tbl[] = {
> > + { .gain = 1, .reg = 0x01 },
> > + { .gain = 2, .reg = 0x02 },
>
> reg == 3 valid?
>
> > + { .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!?
>
> > +static int ads131e08_exec_cmd(struct ads131e08_state *st, u8 cmd) {
> > + int ret;
> > +
....
>
> --
> With Best Regards,
> Andy Shevchenko
next prev parent reply other threads:[~2021-01-23 8:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-12 9:51 [PATCH v3 0/2] Add support for ADS131E0x ADC family tomislav.denis
2021-01-12 9:51 ` [PATCH v3 1/2] iio: adc: Add driver for Texas Instruments " tomislav.denis
2021-01-17 12:31 ` Jonathan Cameron
2021-01-21 16:42 ` Andy Shevchenko
2021-01-23 8:28 ` Denis, Tomislav AVL DiTEST [this message]
2021-01-12 9:51 ` [PATCH v3 2/2] bindings: iio: adc: Add documentation for ADS131E0x ADC driver tomislav.denis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7474dd07191545b0bb9ae091e54c2013@avl.com \
--to=tomislav.denis@avl.com \
--cc=andy.shevchenko@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.