Re: [PATCH 0/8] iio: ad9467: support interface tuning

Jonathan Cameron
2024-04-20T15:39:36Z

On Fri, 19 Apr 2024 17:36:43 +0200
Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> Hi Jonathan,
Hi Nuno,

Friday special :)

>
> Here it goes one more set of new functionality for the backend
> framework. This allows for one of the most important missing features of
> the ad9467 driver. I hope the new interfaces to be fairly straight.
> Though, there's one that's likely to catch your attention:
>
> iio_backend_iodelay_set()
>
> as you would expect (rightfully) some delay with actual units. The
> reason why it does not have any units is because the IO delay thing is
> mostly a calibration done at the backend level and the actually values
> and timings (each tap corresponds to) is very HW specific. For example
> the Xilinx/AMD zedboard has different specifications when compared to
> zc706.
>
> Given the above, I admit (:sweat smile:) I went the easier path and just added a
> parameter with no meaningful unit (with proper docs). I'm definitely open
> for ideas if this fells to hacky. One thing that I thought would be to
> have any additional API that could be called during probe and get an
> array of delays from the backend. Something like:
>
> iio_backend_iodelays_get(back, const unsigned int **delays_ps, unsigned int *ndelays)
>
> The backend should know what delays it supports. For the axi-adc IP we
> do have registers to detect the fpga grade etc so we could return the
> delays based on the HW we are running on. We would also need an addition
> refclk as the actual delay each tap introduces depends on a refclk.

From a userspace point of view do we care about the real values? (in units we understand)
Does the front end driver algorithm ever care about what they actually mean either?

Feels like all these are is a sequence of magic flags that we try, the only
thing that assigns them any absolute meaning is that you assume they
are monotonic in some sense, so picking the middle value of a set in a row that
works makes sense.

So I'm not really that bothered about the lack of units.
Whether this interface generalizes well to other device will be interesting
to see, but if it doesn't and we come up with something better a later stage,
this is all in kernel interface, so we can change it anwyay at that point.

>
> The series also has some "unrelated" patches for improvements and fixes.

Hmm. Next time pull those out as a separate set and just mention
it as a dependency.

>
> ---
> Nuno Sa (8):
> iio: backend: add API for interface tuning
> iio: adc: adi-axi-adc: only error out in major version mismatch
> dt-bindings: adc: axi-adc: add clocks property
> iio: adc: axi-adc: make sure AXI clock is enabled
> iio: adc: adi-axi-adc: remove regmap max register
> iio: adc: adi-axi-adc: support digital interface calibration
> iio: adc: ad9467: cache the sample rate
> iio: adc: ad9467: support digital interface calibration
>
> .../devicetree/bindings/iio/adc/adi,axi-adc.yaml | 5 +
> drivers/iio/adc/ad9467.c | 340 ++++++++++++++++++---
> drivers/iio/adc/adi-axi-adc.c | 123 +++++++-
> drivers/iio/industrialio-backend.c | 86 ++++++
> include/linux/iio/backend.h | 57 +++-
> 5 files changed, 561 insertions(+), 50 deletions(-)
> ---
> base-commit: 62d3fb9dcc091ccdf25eb3b716e90e07e3ed861f
> change-id: 20240419-ad9467-new-features-fbfbaa5edf06
> --
>
> Thanks!
> - Nuno Sá
>
>