On Fri, 2024-02-09 at 18:45 +0200, Andy Shevchenko wrote:
> On Fri, Feb 9, 2024 at 5:26 PM Nuno Sa <nuno.sa@xxxxxxxxxx> wrote:
> >
> > Move to the IIO backend framework. Devices supported by adi-axi-adc now
> > register themselves as backend devices.
>
> ...
>
> > -#include <linux/iio/iio.h>
> > -#include <linux/iio/sysfs.h>
> > -#include <linux/iio/buffer.h>
> > -#include <linux/iio/buffer-dmaengine.h>
> > -
> > #include <linux/fpga/adi-axi-common.h>
> > -#include <linux/iio/adc/adi-axi-adc.h>
>
> + Blank line?
>
can be...
> > +#include <linux/iio/backend.h>
> > +#include <linux/iio/buffer-dmaengine.h>
> > +#include <linux/iio/buffer.h>
> > +#include <linux/iio/iio.h>
>
> ...
>
> > +static int axi_adc_enable(struct iio_backend *back)
> > {
> > + struct adi_axi_adc_state *st = iio_backend_get_priv(back);
> > int ret;
> >
> > + ret = regmap_set_bits(st->regmap, ADI_AXI_REG_RSTN,
> > + ADI_AXI_REG_RSTN_MMCM_RSTN);
> > + if (ret)
> > + return ret;
>
> > + fsleep(10);
>
> Would be nice to have a comment that probably the datasheet defines
> the minimum timeout for reset. Ah and you decreased it 1000x times,
> why?
>
Arghh, always forget that fsleep() is us... That said, there's no minimum timeout
specified in the docs. I guess the original developer has the sleeps out of habit or
some "hidden" knowledge. In my testing I did not noticed anything weird with the
10us. I'll move back to the original timeout though. I can then check and make sure
10us is enough and send a follow up patch.
> > + return regmap_set_bits(st->regmap, ADI_AXI_REG_RSTN,
> > + ADI_AXI_REG_RSTN_RSTN |
> > ADI_AXI_REG_RSTN_MMCM_RSTN);
> > }
>
> ...
>
> > + expected_ver = (unsigned int *)device_get_match_data(&pdev->dev);
>
> expected_ver should have const and you can drop the casting IIUC.
Right...
>
> > + if (!expected_ver)
> > + return -ENODEV;
>
