From: Jonathan Cameron <jic23@kernel.org>
To: Alexandru Ardelean <alexandru.ardelean@analog.com>
Cc: <linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<devicetree@vger.kernel.org>, <robh+dt@kernel.org>,
Michael Hennerich <michael.hennerich@analog.com>,
Lars-Peter Clausen <lars@metafoo.de>
Subject: Re: [PATCH v8 7/8] iio: adc: ad9467: add support AD9467 ADC
Date: Sat, 7 Mar 2020 15:05:13 +0000 [thread overview]
Message-ID: <20200307150513.053b198a@archlinux> (raw)
In-Reply-To: <20200306110100.22092-8-alexandru.ardelean@analog.com>
On Fri, 6 Mar 2020 13:00:59 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
>
> The AD9467 is a 16-bit, monolithic, IF sampling analog-to-digital converter
> (ADC). It is optimized for high performanceover wide bandwidths and ease of
> use. The product operates at a 250 MSPS conversion rate and is designed for
> wireless receivers, instrumentation, and test equipment that require a high
> dynamic range. The ADC requires 1.8 V and 3.3 V power supplies and a low
> voltage differential input clock for full performance operation. No
> external reference or driver components are required for many applications.
> Data outputs are LVDS compatible (ANSI-644 compatible) and include the
> means to reduce the overall current needed for short trace distances.
>
> Since the chip can operate at such high sample-rates (much higher than
> classical interfaces), it requires that a DMA controller be used to
> interface directly to the chip and push data into memory.
> Typically, the AXI ADC IP core is used to interface with it.
>
> Link: https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
A few minor things but otherwise looks good to me..
> ---
> drivers/iio/adc/Kconfig | 15 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad9467.c | 432 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 448 insertions(+)
> create mode 100644 drivers/iio/adc/ad9467.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 445070abf376..a0796510f9d4 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -246,6 +246,21 @@ config AD799X
> To compile this driver as a module, choose M here: the module will be
> called ad799x.
>
...
> +static int ad9467_spi_read(struct spi_device *spi, unsigned int reg)
> +{
> + unsigned char buf[3];
> + int ret;
> +
> + buf[0] = 0x80 | (reg >> 8);
> + buf[1] = reg & 0xFF;
> +
> + ret = spi_write_then_read(spi, &buf[0], 2, &buf[2], 1);
Why not split buf into send part and receive? Might make it slightly
more readable for no actual cost..
> +
> + if (ret < 0)
> + return ret;
> +
> + return buf[2];
> +}
...
> +static int ad9467_write_raw(struct adi_axi_adc_conv *conv,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + const struct adi_axi_adc_chip_info *info = conv->chip_info;
> + struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
> + unsigned long r_clk;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + return ad9467_set_scale(conv, val, val2);
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (!st->clk)
> + return -ENODEV;
> +
> + if (chan->extend_name)
This is a very 'odd' test. Why?
> + return -ENODEV;
> +
> + r_clk = clk_round_rate(st->clk, val);
> + if (r_clk < 0 || r_clk > info->max_rate) {
> + dev_warn(&st->spi->dev,
> + "Error setting ADC sample rate %ld", r_clk);
> + return -EINVAL;
> + }
> +
> + return clk_set_rate(st->clk, r_clk);
> + default:
> + return -EINVAL;
> + }
> +}
> +
...
> +static int ad9467_probe(struct spi_device *spi)
> +{
> + const struct of_device_id *oid;
> + struct adi_axi_adc_conv *conv;
> + struct ad9467_state *st;
> + unsigned int id;
> + int ret;
> +
> + if (!spi->dev.of_node) {
> + dev_err(&spi->dev, "DT node is null\n");
> + return -ENODEV;
Silly question for you. Can this happen? We can only probe this
if it is in DT and hence there must be a node to get here I think.
> + }
> +
> + oid = of_match_node(ad9467_of_match, spi->dev.of_node);
> + if (!oid)
> + return -ENODEV;
You only ever want the data field so you can get that directly.
of_device_get_match_data
> +
> + conv = devm_adi_axi_adc_conv_register(&spi->dev, sizeof(*st));
> + if (IS_ERR(conv))
> + return PTR_ERR(conv);
> +
> + st = adi_axi_adc_conv_priv(conv);
> + st->spi = spi;
> +
> + st->clk = devm_clk_get(&spi->dev, "adc-clk");
> + if (IS_ERR(st->clk))
> + return PTR_ERR(st->clk);
> +
> + ret = clk_prepare_enable(st->clk);
> + if (ret < 0)
> + return ret;
> +
> + ret = devm_add_action_or_reset(&spi->dev, ad9467_clk_disable, st);
> + if (ret)
> + return ret;
> +
> + st->pwrdown_gpio = devm_gpiod_get_optional(&spi->dev, "powerdown",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(st->pwrdown_gpio))
> + return PTR_ERR(st->pwrdown_gpio);
> +
> + st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(st->reset_gpio))
> + return PTR_ERR(st->reset_gpio);
> +
> + if (st->reset_gpio) {
> + udelay(1);
> + ret = gpiod_direction_output(st->reset_gpio, 1);
> + mdelay(10);
> + }
> +
> + spi_set_drvdata(spi, st);
> +
> + id = (unsigned int)oid->data;
> + conv->chip_info = &ad9467_chip_info_tbl[id];
> +
> + id = ad9467_spi_read(spi, AN877_ADC_REG_CHIP_ID);
> + if (id != conv->chip_info->id) {
> + dev_err(&spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
> + return -ENODEV;
> + }
> +
> + conv->reg_access = ad9467_reg_access;
> + conv->write_raw = ad9467_write_raw;
> + conv->read_raw = ad9467_read_raw;
> + conv->preenable_setup = ad9467_preenable_setup;
> +
> + return ad9467_setup(st, id);
> +}
> +
> +static struct spi_driver ad9467_driver = {
> + .driver = {
> + .name = "ad9467",
> + .of_match_table = ad9467_of_match,
> + },
> + .probe = ad9467_probe,
> +};
> +module_spi_driver(ad9467_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD9467 ADC driver");
> +MODULE_LICENSE("GPL v2");
next prev parent reply other threads:[~2020-03-07 15:05 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space Alexandru Ardelean
2020-03-07 14:25 ` Jonathan Cameron
2020-03-07 19:56 ` Moritz Fischer
2020-03-09 13:49 ` Ardelean, Alexandru
2020-03-06 11:00 ` [PATCH v8 2/8] include: fpga: adi-axi-common.h: add version helper macros Alexandru Ardelean
2020-03-07 14:26 ` Jonathan Cameron
2020-03-07 19:56 ` Moritz Fischer
2020-03-09 13:58 ` Ardelean, Alexandru
2020-03-06 11:00 ` [PATCH v8 3/8] iio: buffer-dmaengine: use %zu specifier for sprintf(align) Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 4/8] iio: buffer-dmaengine: add dev-managed calls for buffer alloc Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core Alexandru Ardelean
2020-03-07 14:54 ` Jonathan Cameron
2020-03-09 11:34 ` Ardelean, Alexandru
2020-03-09 15:15 ` Ardelean, Alexandru
2020-03-15 9:23 ` Jonathan Cameron
2020-03-06 11:00 ` [PATCH v8 6/8] dt-bindings: iio: adc: add bindings doc for AXI ADC driver Alexandru Ardelean
2020-03-07 14:55 ` Jonathan Cameron
2020-03-06 11:00 ` [PATCH v8 7/8] iio: adc: ad9467: add support AD9467 ADC Alexandru Ardelean
2020-03-07 15:05 ` Jonathan Cameron [this message]
2020-03-09 11:51 ` Ardelean, Alexandru
2020-03-06 11:01 ` [PATCH v8 8/8] dt-bindings: iio: adc: add bindings doc for " Alexandru Ardelean
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=20200307150513.053b198a@archlinux \
--to=jic23@kernel.org \
--cc=alexandru.ardelean@analog.com \
--cc=devicetree@vger.kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michael.hennerich@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).