Re: [PATCH v10 7/7] iio: adc: adi-axi-adc: move to backend framework

["Nuno Sá" <noname.nuno@gmail.com>]

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@analog.com> 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;
>