Re: [PATCH 4/8] iio: backend: add API for interface configuration

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 17 Dec 2024 11:13:59 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Mon, 2024-12-16 at 21:36 +0100, Angelo Dureghello wrote:
> > From: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > 
> > Add backend support for setting and getting the interface type
> > in use.
> > 
> > Link:
> > https://lore.kernel.org/linux-iio/20241129153546.63584-1-antoniu.miclaus@analog.com/T/#m6d86939078d780512824f1540145aade38b0990b
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
> > This patch has been picked up from the Antoniu patchset
> > still not accepted, and extended with the interface setter,
> > fixing also namespace names to be between quotation marks.
> > ---
> >  drivers/iio/industrialio-backend.c | 42
> > ++++++++++++++++++++++++++++++++++++++
> >  include/linux/iio/backend.h        | 19 +++++++++++++++++
> >  2 files changed, 61 insertions(+)
> > 
> > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> > backend.c
> > index 363281272035..6edc3e685f6a 100644
> > --- a/drivers/iio/industrialio-backend.c
> > +++ b/drivers/iio/industrialio-backend.c
> > @@ -636,6 +636,48 @@ 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 interface 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_MAX)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_get, "IIO_BACKEND");
> > +
> > +/**
> > + * iio_backend_interface_type_set - set the interface type used.
> > + * @back: Backend device
> > + * @type: Interface type
> > + *
> > + * RETURNS:
> > + * 0 on success, negative error number on failure.
> > + */
> > +int iio_backend_interface_type_set(struct iio_backend *back,
> > +				   enum iio_backend_interface_type type)
> > +{
> > +	if (type >= IIO_BACKEND_INTERFACE_MAX)
> > +		return -EINVAL;
> > +
> > +	return  iio_backend_op_call(back, interface_type_set, type);
> > +}
> > +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_type_set, "IIO_BACKEND");
> > +
> >  /**
> >   * iio_backend_extend_chan_spec - Extend an IIO channel
> >   * @back: Backend device
> > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> > index 10be00f3b120..2b7221099d8c 100644
> > --- a/include/linux/iio/backend.h
> > +++ b/include/linux/iio/backend.h
> > @@ -70,6 +70,15 @@ enum iio_backend_sample_trigger {
> >  	IIO_BACKEND_SAMPLE_TRIGGER_MAX
> >  };
> >  
> > +enum iio_backend_interface_type {
> > +	IIO_BACKEND_INTERFACE_SERIAL_LVDS,
> > +	IIO_BACKEND_INTERFACE_SERIAL_CMOS,  
> 
> The above are apparently not used in the next patch so I would not add them now.
> > +	IIO_BACKEND_INTERFACE_SERIAL_SPI,
> > +	IIO_BACKEND_INTERFACE_SERIAL_DSPI,
> > +	IIO_BACKEND_INTERFACE_SERIAL_QSPI,  
> 
> I'll throw my 2 cents but it would be nice to have more feedback on this. I'm
> not completely sure about the xSPI stuff in here. We treated the QSPI as a bus
> both for control and data in which we also add child devices. And we've been
> adding specific bus operations/configurations through the 'struct
> ad3552r_hs_platform_data' interface. So, I'm wondering if this should also not
> be set through that interface?

Maybe - kind of hard to tell until we actually have code.
I'd go for kicking them into the long grass for now if they aren't used and
just dropping them from this patch.  If we ever need them,easy to bring back
and then we should have a justification for why!

J


> 
> LVDS/CMOS still looks slightly different to me...
> 
> - Nuno Sá
> 
> 
>