On Sat, 2024-04-20 at 16:34 +0100, Jonathan Cameron wrote:
> On Fri, 19 Apr 2024 17:36:51 +0200
> Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@xxxxxxxxxx> wrote:
>
> > From: Nuno Sa <nuno.sa@xxxxxxxxxx>
> >
> > To make sure that we have the best timings on the serial data interface
> > we should calibrate it. This means going through the device supported
> > values and see for which ones we get a successful result. To do that, we
> > use a prbs test pattern both in the IIO backend and in the frontend
> > devices. Then for each of the test points we see if there are any
> > errors. Note that the backend is responsible to look for those errors.
> >
> > As calibrating the interface also requires that the data format is disabled
> > (the one thing being done in ad9467_setup()), ad9467_setup() was removed
> > and configuring the data fomat is now part of the calibration process.
> >
> > Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> Trivial comments inline.
>
> Jonathan
>
> > ---
> > drivers/iio/adc/ad9467.c | 337 +++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 296 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> > index 7db87ccc1ea4..44552dd6f4c6 100644
> > --- a/drivers/iio/adc/ad9467.c
> > +++ b/drivers/iio/adc/ad9467.c
> > @@ -4,6 +4,9 @@
> > *
> > * Copyright 2012-2020 Analog Devices Inc.
> > */
> > +
> > +#include <linux/bitmap.h>
> > +#include <linux/bitops.h>
> > #include <linux/cleanup.h>
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > @@ -100,6 +103,8 @@
> > #define AD9467_DEF_OUTPUT_MODE 0x08
> > #define AD9467_REG_VREF_MASK 0x0F
> >
> > +#define AD9647_MAX_TEST_POINTS 32
> > +
> > struct ad9467_chip_info {
> > const char *name;
> > unsigned int id;
> > @@ -110,6 +115,8 @@ struct ad9467_chip_info {
> > unsigned long max_rate;
> > unsigned int default_output_mode;
> > unsigned int vref_mask;
> > + unsigned int num_lanes;
> > + bool has_dco;
>
> What is dco? Perhaps a comment, or expand the naming somewhat.
>
data clock output.. will add a comment
> > };
>
> >
> > +static void ad9467_dump_table(const unsigned long *err_map, unsigned int size,
> > + bool invert)
> > +{
> > +#ifdef DEBUG
> > + unsigned int bit;
> > +
> > + pr_debug("Dump calibration table:\n");
>
> If it's useful, poke it in debugfs, otherwise, drop this code.
>From our experience, it's useful to dump the table in the tuning process. But I guess
I can also just save the bitmap and dump it on demand (on debugfs).
>
> > + for (bit = 0; bit < size; bit++) {
> > + if (bit == size / 2) {
> > + if (!invert)
> > + break;
> > + pr_cont("\n");
> > + }
> > +
> > + pr_cont("%c", test_bit(bit, err_map) ? 'x' : 'o');
> > + }
> > +#endif
> > +}
> > +
>
> > +static int ad9467_calibrate_apply(const struct ad9467_state *st,
> > + unsigned int val)
> > +{
> > + unsigned int lane;
> > + int ret;
> > +
> > + if (st->info->has_dco) {
> > + int ret;
> Shadowing ret above.
>
ups...
> > +
> > + ret = ad9467_spi_write(st->spi, AN877_ADC_REG_OUTPUT_DELAY,
> > + val);
> > + if (ret)
> > + return ret;
> > +
> > + return ad9467_spi_write(st->spi, AN877_ADC_REG_TRANSFER,
> > + AN877_ADC_TRANSFER_SYNC);
> > + }
> > +
> > + for (lane = 0; lane < st->info->num_lanes; lane++) {
> > + ret = iio_backend_iodelay_set(st->back, lane, val);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
>
> > +
> > +static int ad9467_calibrate(const struct ad9467_state *st)
>
> Some docs on the sequence or a reference would be good.
I can point into the datasheets..
- Nuno Sá
