From: Jonathan Cameron <jic23@kernel.org>
To: Marcelo Schmitt <marcelo.schmitt@analog.com>
Cc: <lars@metafoo.de>, <Michael.Hennerich@analog.com>,
<robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
<conor+dt@kernel.org>, <marcelo.schmitt1@gmail.com>,
<linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] iio: adc: Add support for AD4000
Date: Sat, 13 Apr 2024 17:39:32 +0100 [thread overview]
Message-ID: <20240413173932.338b574c@jic23-huawei> (raw)
In-Reply-To: <1d95d7d023dad69b894a2d0e7b0bad9d569ae382.1712585500.git.marcelo.schmitt@analog.com>
On Mon, 8 Apr 2024 11:31:42 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> Add support for AD4000 family of low noise, low power, high speed,
> successive aproximation register (SAR) ADCs.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
A few trivial comments inline to add to David's much more thorough review.
> +#include <asm/unaligned.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/math.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/units.h>
> +#include <linux/util_macros.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
You don't have custom attributes, so doubt you need that include.
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +static int ad4000_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long info)
> +{
> + struct ad4000_state *st = iio_priv(indio_dev);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_RAW:
> + iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
> + return ad4000_single_conversion(indio_dev, chan, val);
> + unreachable();
> + case IIO_CHAN_INFO_SCALE:
> + *val = st->scale_tbl[st->pin_gain][st->span_comp][0];
> + *val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
> + return IIO_VAL_INT_PLUS_NANO;
> + case IIO_CHAN_INFO_OFFSET:
> + *val = 0;
> + if (st->span_comp)
> + *val = mult_frac(st->vref / 1000, 1, 10);
> +
> + return IIO_VAL_INT;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
As below - I'd push into the default: and drop down here.
> +}
> +
> +static int ad4000_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long info)
> +{
> + struct ad4000_state *st = iio_priv(indio_dev);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_SCALE:
> + *vals = (int *)st->scale_tbl[st->pin_gain];
> + *length = 2 * 2;
> + *type = IIO_VAL_INT_PLUS_NANO;
> + return IIO_AVAIL_LIST;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad4000_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + return IIO_VAL_INT_PLUS_NANO;
> + default:
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> + return -EINVAL;
dead code.
> +}
> +
> +static int ad4000_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val, int val2,
> + long mask)
> +{
> + struct ad4000_state *st = iio_priv(indio_dev);
> + unsigned int reg_val;
> + bool span_comp_en;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> + ret = ad4000_read_reg(st, ®_val);
> + if (ret < 0)
> + return ret;
> +
> + span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
> + reg_val &= ~AD4000_SPAN_COMP;
> + reg_val |= FIELD_PREP(AD4000_SPAN_COMP, span_comp_en);
> +
> + ret = ad4000_write_reg(st, reg_val);
> + if (ret < 0)
> + return ret;
> +
> + st->span_comp = span_comp_en;
> + return 0;
> + }
> + unreachable();
> + default:
> + break;
> + }
> +
> + return -EINVAL;
Trivial but could push up into the default and make it clear there are no other ways out
of the switch statement.
> +}
next prev parent reply other threads:[~2024-04-13 16:39 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-08 14:30 [PATCH v2 0/2] Add support for AD4000 series Marcelo Schmitt
2024-04-08 14:31 ` [PATCH v2 1/2] dt-bindings: iio: adc: Add AD4000 Marcelo Schmitt
2024-04-09 2:57 ` David Lechner
2024-04-09 15:30 ` Marcelo Schmitt
2024-04-13 16:14 ` Jonathan Cameron
2024-04-13 17:33 ` David Lechner
2024-04-14 18:09 ` Jonathan Cameron
2024-04-16 21:46 ` Marcelo Schmitt
2024-04-20 14:17 ` Jonathan Cameron
2024-04-21 22:38 ` Marcelo Schmitt
2024-04-08 14:31 ` [PATCH v2 2/2] iio: adc: Add support for AD4000 Marcelo Schmitt
2024-04-09 3:05 ` David Lechner
2024-04-09 16:09 ` Marcelo Schmitt
2024-04-09 16:44 ` David Lechner
2024-04-09 18:12 ` Marcelo Schmitt
2024-04-13 16:34 ` Jonathan Cameron
2024-04-13 16:19 ` Jonathan Cameron
2024-04-13 16:39 ` Jonathan Cameron [this message]
2024-04-09 2:54 ` [PATCH v2 0/2] Add support for AD4000 series David Lechner
2024-04-09 14:59 ` Marcelo Schmitt
2024-04-09 16:31 ` David Lechner
2024-04-13 16:07 ` Jonathan Cameron
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=20240413173932.338b574c@jic23-huawei \
--to=jic23@kernel.org \
--cc=Michael.Hennerich@analog.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcelo.schmitt1@gmail.com \
--cc=marcelo.schmitt@analog.com \
--cc=robh+dt@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.