Re: [PATCH 1/7] iio: backend: add API for interface get

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

On Thu, 2024-09-26 at 10:40 +0200, David Lechner wrote:
> On Mon, Sep 23, 2024 at 12:15 PM Antoniu Miclaus
> <antoniu.miclaus@analog.com> wrote:
> > 
> > Add backend support for obtaining the interface type used.
> > 
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > ---
> >  drivers/iio/industrialio-backend.c | 24 ++++++++++++++++++++++++
> >  include/linux/iio/backend.h        | 10 ++++++++++
> >  2 files changed, 34 insertions(+)
> > 
> > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> > backend.c
> > index efe05be284b6..53ab6bc86a50 100644
> > --- a/drivers/iio/industrialio-backend.c
> > +++ b/drivers/iio/industrialio-backend.c
> > @@ -449,6 +449,30 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev,
> > uintptr_t private,
> >  }
> >  EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, IIO_BACKEND);
> > 
> > +/**
> > + * iio_backend_interface_type_get - get the interace type used.
> > + * @back: Backend device
> > + * @type: Interface type
> > + *
> > + * RETURNS:
> > + * 0 on success, negative error number on failure.
> > + */
> > +int iio_backend_interface_type_get(struct iio_backend *back,
> > +                                  enum iio_backend_interface_type *type)
> > +{
> > +       int ret;
> > +
> > +       ret = iio_backend_op_call(back, interface_type_get, type);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (*type > IIO_BACKEND_INTERFACE_CMOS)
> > +               return -EINVAL;
> > +
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, IIO_BACKEND);
> > +
> >  /**
> >   * iio_backend_extend_chan_spec - Extend an IIO channel
> >   * @indio_dev: IIO device
> > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> > index 8099759d7242..ba8ad30ac9ba 100644
> > --- a/include/linux/iio/backend.h
> > +++ b/include/linux/iio/backend.h
> > @@ -63,6 +63,11 @@ enum iio_backend_sample_trigger {
> >         IIO_BACKEND_SAMPLE_TRIGGER_MAX
> >  };
> > 
> > +enum iio_backend_interface_type {
> > +       IIO_BACKEND_INTERFACE_LVDS,
> > +       IIO_BACKEND_INTERFACE_CMOS
> > +};
> > +
> >  /**
> >   * struct iio_backend_ops - operations structure for an iio_backend
> >   * @enable: Enable backend.
> > @@ -81,6 +86,7 @@ enum iio_backend_sample_trigger {
> >   * @extend_chan_spec: Extend an IIO channel.
> >   * @ext_info_set: Extended info setter.
> >   * @ext_info_get: Extended info getter.
> > + * @interface_type_get: Interface type.
> >   **/
> >  struct iio_backend_ops {
> >         int (*enable)(struct iio_backend *back);
> > @@ -113,6 +119,8 @@ struct iio_backend_ops {
> >                             const char *buf, size_t len);
> >         int (*ext_info_get)(struct iio_backend *back, uintptr_t private,
> >                             const struct iio_chan_spec *chan, char *buf);
> > +       int (*interface_type_get)(struct iio_backend *back,
> > +                                 enum iio_backend_interface_type *type);
> >  };
> > 
> >  int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
> > @@ -142,6 +150,8 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev,
> > uintptr_t private,
> >  ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private,
> >                                  const struct iio_chan_spec *chan, char *buf);
> > 
> > +int iio_backend_interface_type_get(struct iio_backend *back,
> > +                                  enum iio_backend_interface_type *type);
> >  int iio_backend_extend_chan_spec(struct iio_dev *indio_dev,
> >                                  struct iio_backend *back,
> >                                  struct iio_chan_spec *chan);
> > --
> > 2.46.0
> > 
> 
> This seems very specific to the AD485x chips and the AXI ADC backend.
> Since it is describing how the chip is wired to the AXI DAC IP block,
> I would be tempted to use the devicetree for this info instead of
> adding a new backend function.

Not sure If I'm following your point but I think this is the typical case where the
chip (being it a DAC or ADC) supports both CMOS and LVDS interfaces. Naturally you
only use one on your system and this is a synthesis parameter on the FPGA IP core.
Therefore, it makes sense for the frontend to have way to ask for this information to
the backend.

That said, we could also have a DT parameter but, ideally, we would then need a way
to match the parameter with the backend otherwise we could have DT stating LVDS and
the backend built with CMOS.

Other thing that we could think about is a new devm_iio_backend_get_with_info() where
the frontend would get some constant and static info about the backend (the interface
type would an ideal match for something like this). But I feel it's still early days
for something like this :)

- Nuno Sá