On Sat, 2020-03-07 at 15:05 +0000, Jonathan Cameron wrote: > [External] > > On Fri, 6 Mar 2020 13:00:59 +0200 > Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote: > > > From: Michael Hennerich <michael.hennerich@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> > > 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.. sure > > > + > > + 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? left-over from the original driver; and it slipped when i adapted from that; no idea why it was added; and git history is not helping either; will drop; > > > + 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. good point; will drop; looks like something i added inertially > > > + } > > + > > + 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 ack > > > + > > + 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@xxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Analog Devices AD9467 ADC driver"); > > +MODULE_LICENSE("GPL v2");
