On Tue, 2024-09-24 at 16:11 +0200, Angelo Dureghello wrote:
>
> On 20/09/24 14:50, Nuno Sá wrote:
> > On Thu, 2024-09-19 at 11:20 +0200, Angelo Dureghello wrote:
> > > From: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
> > >
> > > Extend backend features with new calls needed later on this
> > > patchset from axi version of ad3552r.
> > >
> > > The follwoing calls are added:
> > >
> > > iio_backend_ext_sync_enable
> > > enable synchronize channels on external trigger
> > > iio_backend_ext_sync_disable
> > > disable synchronize channels on external trigger
> > > iio_backend_ddr_enable
> > > enable ddr bus transfer
> > > iio_backend_ddr_disable
> > > disable ddr bus transfer
> > > iio_backend_set_bus_mode
> > > select the type of bus, so that specific read / write
> > > operations are performed accordingly
> > > iio_backend_buffer_enable
> > > enable buffer
> > > iio_backend_buffer_disable
> > > disable buffer
> > > iio_backend_data_transfer_addr
> > > define the target register address where the DAC sample
> > > will be written.
> > >
> > > Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
> > > ---
> > > drivers/iio/industrialio-backend.c | 111
> > > +++++++++++++++++++++++++++++++++++++
> > > include/linux/iio/backend.h | 23 ++++++++
> > > 2 files changed, 134 insertions(+)
> > >
> > > diff --git a/drivers/iio/industrialio-backend.c
> > > b/drivers/iio/industrialio-
> > > backend.c
> > > index 20b3b5212da7..f4802c422dbf 100644
> > > --- a/drivers/iio/industrialio-backend.c
> > > +++ b/drivers/iio/industrialio-backend.c
> > > @@ -718,6 +718,117 @@ static int __devm_iio_backend_get(struct device
> > > *dev, struct
> > > iio_backend *back)
> > > return 0;
> > > }
> > >
> > > +/**
> > > + * iio_backend_ext_sync_enable - Enable external synchronization
> > > + * @back: Backend device
> > > + *
> > > + * Enable synchronization by external signal.
> > > + *
> > > + * RETURNS:
> > > + * 0 on success, negative error number on failure.
> > > + */
> > > +int iio_backend_ext_sync_enable(struct iio_backend *back)
> > > +{
> > > + return iio_backend_op_call(back, ext_sync_enable);
> > > +}
> > > +EXPORT_SYMBOL_NS_GPL(iio_backend_ext_sync_enable, IIO_BACKEND);
> > > +
> > > +/**
> > > + * iio_backend_ext_sync_disable - Disable external synchronization
> > > + * @back: Backend device
> > > + *
> > > + * Disable synchronization by external signal.
> > > + *
> > > + * RETURNS:
> > > + * 0 on success, negative error number on failure.
> > > + */
> > > +int iio_backend_ext_sync_disable(struct iio_backend *back)
> > > +{
> > > + return iio_backend_op_call(back, ext_sync_disable);
> > > +}
> > > +EXPORT_SYMBOL_NS_GPL(iio_backend_ext_sync_disable, IIO_BACKEND);
> > > +
> > > +/**
> > > + * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
> > > + * @back: Backend device
> > > + *
> > > + * Enabling DDR, data is generated by the IP at each front
> > > + * (raising and falling) of the bus clock signal.
> > > + *
> > > + * RETURNS:
> > > + * 0 on success, negative error number on failure.
> > > + */
> > > +int iio_backend_ddr_enable(struct iio_backend *back)
> > > +{
> > > + return iio_backend_op_call(back, ddr_enable);
> > > +}
> > > +EXPORT_SYMBOL_NS_GPL(iio_backend_ddr_enable, IIO_BACKEND);
> > > +
> > > +/**
> > > + * iio_backend_ddr_disable - Disable interface DDR (Double Data Rate)
> > > mode
> > > + * @back: Backend device
> > > + *
> > > + * Disabling DDR data is generated byt the IP at rising or falling front
> > > + * of the interface clock signal (SDR, Single Data Rate).
> > > + *
> > > + * RETURNS:
> > > + * 0 on success, negative error number on failure.
> > > + */
> > > +int iio_backend_ddr_disable(struct iio_backend *back)
> > > +{
> > > + return iio_backend_op_call(back, ddr_disable);
> > > +}
> > > +EXPORT_SYMBOL_NS_GPL(iio_backend_ddr_disable, IIO_BACKEND);
> > > +
> > > +/**
> > > + * iio_backend_buffer_enable - Enable iio buffering
> > > + * @back: Backend device
> > > + *
> > > + * Enabling the buffer, buffer data is processed and sent out from the
> > > + * bus interface.
> > > + *
> > > + * RETURNS:
> > > + * 0 on success, negative error number on failure.
> > > + */
> > > +int iio_backend_buffer_enable(struct iio_backend *back)
> > > +{
> > > + return iio_backend_op_call(back, buffer_enable);
> > > +}
> > > +EXPORT_SYMBOL_NS_GPL(iio_backend_buffer_enable, IIO_BACKEND);
> > > +
> > > +/**
> > > + * iio_backend_buffer_disable - Disable iio buffering
> > > + * @back: Backend device
> > > + *
> > > + * Disabling the buffer, buffer data transfer on the bus interface
> > > + * is stopped.
> > > + *
> > > + * RETURNS:
> > > + * 0 on success, negative error number on failure.
> > > + */
> > > +int iio_backend_buffer_disable(struct iio_backend *back)
> > > +{
> > > + return iio_backend_op_call(back, buffer_disable);
> > > +}
> > > +EXPORT_SYMBOL_NS_GPL(iio_backend_buffer_disable, IIO_BACKEND);
> > > +
> > IIRC, both me and Jonathan had some comments about the above 2 calls? Aren't
> > they
> > about buffering? I think I mentioned something about using the same buffer
> > ops as
> > typical IIO devices use.
>
> i have now separated iio_backend_ops, keeping buffer enable/disable
> for axi-ad3352r case only,
>
> static const struct iio_backend_ops axi_dac_generic_ops = {
> .enable = axi_dac_enable,
> .disable = axi_dac_disable,
> .request_buffer = axi_dac_request_buffer,
> .free_buffer = axi_dac_free_buffer,
> .extend_chan_spec = axi_dac_extend_chan,
> .ext_info_set = axi_dac_ext_info_set,
> .ext_info_get = axi_dac_ext_info_get,
> .data_source_set = axi_dac_data_source_set,
> .set_sample_rate = axi_dac_set_sample_rate,
> .debugfs_reg_access = iio_backend_debugfs_ptr(axi_dac_reg_access),
> };
>
> static const struct iio_backend_ops axi_ad3552r_ops = {
> .enable = axi_dac_enable,
> .read_raw = axi_dac_read_raw,
> .request_buffer = axi_dac_request_buffer,
> .data_source_set = axi_dac_data_source_set,
> .ext_sync_enable = axi_dac_ext_sync_enable,
> .ext_sync_disable = axi_dac_ext_sync_disable,
> .ddr_enable = axi_dac_ddr_enable,
> .ddr_disable = axi_dac_ddr_disable,
> .buffer_enable = axi_dac_buffer_enable,
> .buffer_disable = axi_dac_buffer_disable,
> .data_format_set = axi_dac_data_format_set,
> .data_transfer_addr = axi_dac_data_transfer_addr,
> };
>
>
> could this be good ?
>
I think you're replying to the wrong email :). But yeah, I made a comment about
the above and that is something I'm also expecting.
Regarding the buffer_enable/disable() stuff, please go check past discussions (I
think on the RFC). I'm fairly sure I had (and Jonathan as well) some comments
about directly using IIO buffer options or replicating them in the backend_ops.
Likely the second option is the best one so we can take a reference to backend
object directly.
- Nuno Sá
