From: Angelo Dureghello <adureghello@baylibre.com>
Extend backend features with new calls needed later on this
patchset from axi version of ad3552r.
A bus type property has been added to the devicetree to
inform the backend about the type of bus (interface) in use
bu the IP.
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.
iio_backend_bus_reg_read
generic bus read, bus-type dependent
iio_backend_bus_read_write
generic bus write, bus-type dependent
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/industrialio-backend.c | 151 +++++++++++++++++++++++++++++++++++++
include/linux/iio/backend.h | 24 ++++++
2 files changed, 175 insertions(+)
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index a52a6b61c8b5..1f60c8626be7 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -718,6 +718,157 @@ 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 data buffering
+ * @back: Backend device
+ *
+ * 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_set_buffer_disable - Disable data buffering
+ * @back: Backend device
+ *
+ * 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);
+
+/**
+ * iio_backend_buffer_transfer_addr - Set data address.
+ * @back: Backend device
+ * @chan_address: Channel register address
+ *
+ * Some devices may need to inform the backend about an address/location
+ * where to read or write the data.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_data_transfer_addr(struct iio_backend *back, u32 address)
+{
+ return iio_backend_op_call(back, data_transfer_addr, address);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_transfer_addr, IIO_BACKEND);
+
+/**
+ * iio_backend_bus_reg_read - Read from the interface bus
+ * @back: Backend device
+ * @reg: Register valule
+ * @val: Pointer to register value
+ * @size: Size, in bytes
+ *
+ * A backend may operate on a specific interface with a related bus.
+ * Read from the interface bus.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_bus_reg_read(struct iio_backend *back,
+ u32 reg, void *val, size_t size)
+{
+ if (!size)
+ return -EINVAL;
+
+ return iio_backend_op_call(back, bus_reg_read, reg, val, size);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_bus_reg_read, IIO_BACKEND);
+
+/**
+ * iio_backend_bus_reg_write - Write on the interface bus
+ * @back: Backend device
+ * @reg: Register value
+ * @val: Register Value
+ * @size: Size in bytes
+ *
+ * A backend may operate on a specific interface with a related bus.
+ * Write to the interface bus.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_bus_reg_write(struct iio_backend *back,
+ u32 reg, void *val, size_t size)
+{
+ if (!size)
+ return -EINVAL;
+
+ return iio_backend_op_call(back, bus_reg_write, reg, val, size);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_bus_reg_write, IIO_BACKEND);
+
static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, const char *name,
struct fwnode_handle *fwnode)
{
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 37d56914d485..6f56bbb9e391 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -14,12 +14,14 @@ struct iio_dev;
enum iio_backend_data_type {
IIO_BACKEND_TWOS_COMPLEMENT,
IIO_BACKEND_OFFSET_BINARY,
+ IIO_BACKEND_DATA_UNSIGNED,
IIO_BACKEND_DATA_TYPE_MAX
};
enum iio_backend_data_source {
IIO_BACKEND_INTERNAL_CONTINUOUS_WAVE,
IIO_BACKEND_EXTERNAL,
+ IIO_BACKEND_INTERNAL_RAMP_16,
IIO_BACKEND_DATA_SOURCE_MAX
};
@@ -129,6 +131,17 @@ struct iio_backend_ops {
size_t len);
int (*debugfs_reg_access)(struct iio_backend *back, unsigned int reg,
unsigned int writeval, unsigned int *readval);
+ int (*ext_sync_enable)(struct iio_backend *back);
+ int (*ext_sync_disable)(struct iio_backend *back);
+ int (*ddr_enable)(struct iio_backend *back);
+ int (*ddr_disable)(struct iio_backend *back);
+ int (*buffer_enable)(struct iio_backend *back);
+ int (*buffer_disable)(struct iio_backend *back);
+ int (*data_transfer_addr)(struct iio_backend *back, u32 address);
+ int (*bus_reg_read)(struct iio_backend *back, u32 reg, void *val,
+ size_t size);
+ int (*bus_reg_write)(struct iio_backend *back, u32 reg, void *val,
+ size_t size);
};
/**
@@ -164,6 +177,17 @@ int iio_backend_data_sample_trigger(struct iio_backend *back,
int devm_iio_backend_request_buffer(struct device *dev,
struct iio_backend *back,
struct iio_dev *indio_dev);
+int iio_backend_ext_sync_enable(struct iio_backend *back);
+int iio_backend_ext_sync_disable(struct iio_backend *back);
+int iio_backend_ddr_enable(struct iio_backend *back);
+int iio_backend_ddr_disable(struct iio_backend *back);
+int iio_backend_buffer_enable(struct iio_backend *back);
+int iio_backend_buffer_disable(struct iio_backend *back);
+int iio_backend_data_transfer_addr(struct iio_backend *back, u32 address);
+int iio_backend_bus_reg_read(struct iio_backend *back,
+ u32 reg, void *val, size_t size);
+int iio_backend_bus_reg_write(struct iio_backend *back,
+ u32 reg, void *val, size_t size);
ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
const struct iio_chan_spec *chan,
const char *buf, size_t len);
--
2.45.0.rc1On Thu, 29 Aug 2024 14:32:00 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Extend backend features with new calls needed later on this
> patchset from axi version of ad3552r.
>
> A bus type property has been added to the devicetree to
> inform the backend about the type of bus (interface) in use
> bu the IP.
>
> 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.
> iio_backend_bus_reg_read
> generic bus read, bus-type dependent
> iio_backend_bus_read_write
> generic bus write, bus-type dependent
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> drivers/iio/industrialio-backend.c | 151 +++++++++++++++++++++++++++++++++++++
> include/linux/iio/backend.h | 24 ++++++
> 2 files changed, 175 insertions(+)
>
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index a52a6b61c8b5..1f60c8626be7 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -718,6 +718,157 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
> return 0;
> }
> +
> +/**
> + * iio_backend_buffer_enable - Enable data buffering
Data buffering is a very vague term. Perhaps some more detail on what
this means?
> + * @back: Backend device
> + *
> + * 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_set_buffer_disable - Disable data buffering
> + * @back: Backend device
> + *
> + * 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);
> +
> +/**
> + * iio_backend_buffer_transfer_addr - Set data address.
> + * @back: Backend device
> + * @chan_address: Channel register address
Run scripts/kernel-doc on this and fix the errors (parameter name is
wrong). W=1 builds might also point the simpler ones out.
> + *
> + * Some devices may need to inform the backend about an address/location
> + * where to read or write the data.
I'd drop the 'location' part unless this gets used later because you
are referring register address above.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_data_transfer_addr(struct iio_backend *back, u32 address)
> +{
> + return iio_backend_op_call(back, data_transfer_addr, address);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_data_transfer_addr, IIO_BACKEND);
> +
> +/**
> + * iio_backend_bus_reg_read - Read from the interface bus
> + * @back: Backend device
> + * @reg: Register valule
> + * @val: Pointer to register value
> + * @size: Size, in bytes
> + *
> + * A backend may operate on a specific interface with a related bus.
> + * Read from the interface bus.
So this is effectively routing control plane data through the offloaded
bus? That sounds a lot more like a conventional bus than IIO backend.
Perhaps it should be presented as that with the IIO device attached
to that bus? I don't fully understand what is wired up here.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_bus_reg_read(struct iio_backend *back,
> + u32 reg, void *val, size_t size)
> +{
> + if (!size)
> + return -EINVAL;
> +
> + return iio_backend_op_call(back, bus_reg_read, reg, val, size);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_bus_reg_read, IIO_BACKEND);
> +
Hi Jonathan,
thanks for the feedbacks,
On 31/08/24 1:23 PM, Jonathan Cameron wrote:
> On Thu, 29 Aug 2024 14:32:00 +0200
> Angelo Dureghello <adureghello@baylibre.com> wrote:
>
>> From: Angelo Dureghello <adureghello@baylibre.com>
>>
>> Extend backend features with new calls needed later on this
>> patchset from axi version of ad3552r.
>>
>> A bus type property has been added to the devicetree to
>> inform the backend about the type of bus (interface) in use
>> bu the IP.
>>
>> 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.
>> iio_backend_bus_reg_read
>> generic bus read, bus-type dependent
>> iio_backend_bus_read_write
>> generic bus write, bus-type dependent
>>
>> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
>> ---
>> drivers/iio/industrialio-backend.c | 151 +++++++++++++++++++++++++++++++++++++
>> include/linux/iio/backend.h | 24 ++++++
>> 2 files changed, 175 insertions(+)
>>
>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
>> index a52a6b61c8b5..1f60c8626be7 100644
>> --- a/drivers/iio/industrialio-backend.c
>> +++ b/drivers/iio/industrialio-backend.c
>> @@ -718,6 +718,157 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
>> return 0;
>> }
>
>> +
>> +/**
>> + * iio_backend_buffer_enable - Enable data buffering
> Data buffering is a very vague term. Perhaps some more detail on what
> this means?
for this DAC IP, it is the dma buffer where i write the samples,
for other non-dac frontends may be something different, so i kept it
generic. Not sure what a proper name may be, maybe
"Enable optional data buffer" ?
>> + * @back: Backend device
>> + *
>> + * 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_set_buffer_disable - Disable data buffering
>> + * @back: Backend device
>> + *
>> + * 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);
>> +
>> +/**
>> + * iio_backend_buffer_transfer_addr - Set data address.
>> + * @back: Backend device
>> + * @chan_address: Channel register address
> Run scripts/kernel-doc on this and fix the errors (parameter name is
> wrong). W=1 builds might also point the simpler ones out.
ack, done
>> + *
>> + * Some devices may need to inform the backend about an address/location
>> + * where to read or write the data.
> I'd drop the 'location' part unless this gets used later because you
> are referring register address above.
ack, done
>> + *
>> + * RETURNS:
>> + * 0 on success, negative error number on failure.
>> + */
>> +int iio_backend_data_transfer_addr(struct iio_backend *back, u32 address)
>> +{
>> + return iio_backend_op_call(back, data_transfer_addr, address);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(iio_backend_data_transfer_addr, IIO_BACKEND);
>> +
>> +/**
>> + * iio_backend_bus_reg_read - Read from the interface bus
>> + * @back: Backend device
>> + * @reg: Register valule
>> + * @val: Pointer to register value
>> + * @size: Size, in bytes
>> + *
>> + * A backend may operate on a specific interface with a related bus.
>> + * Read from the interface bus.
> So this is effectively routing control plane data through the offloaded
> bus? That sounds a lot more like a conventional bus than IIO backend.
> Perhaps it should be presented as that with the IIO device attached
> to that bus? I don't fully understand what is wired up here.
>
Mainly, an IP may include a bus as 16bit parallel, or LVDS, or similar
to QSPI as in my case (ad3552r).
In particular, the bus is physically as a QSPI bus, but the data format
over it is a bit different.
So ad3552r needs this 5 lanes bus + double data rate to reach 33MUPS.
https://analogdevicesinc.github.io/hdl/library/axi_ad3552r/index.html
>> + *
>> + * RETURNS:
>> + * 0 on success, negative error number on failure.
>> + */
>> +int iio_backend_bus_reg_read(struct iio_backend *back,
>> + u32 reg, void *val, size_t size)
>> +{
>> + if (!size)
>> + return -EINVAL;
>> +
>> + return iio_backend_op_call(back, bus_reg_read, reg, val, size);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(iio_backend_bus_reg_read, IIO_BACKEND);
>> +
Thanks a lot,
regards,
Angelo
--
,,, Angelo Dureghello
:: :. BayLibre -runtime team- Developer
:`___:
`____:
On Mon, 2 Sep 2024 16:03:22 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:
> Hi Jonathan,
>
> thanks for the feedbacks,
>
> On 31/08/24 1:23 PM, Jonathan Cameron wrote:
> > On Thu, 29 Aug 2024 14:32:00 +0200
> > Angelo Dureghello <adureghello@baylibre.com> wrote:
> >
> >> From: Angelo Dureghello <adureghello@baylibre.com>
> >>
> >> Extend backend features with new calls needed later on this
> >> patchset from axi version of ad3552r.
> >>
> >> A bus type property has been added to the devicetree to
> >> inform the backend about the type of bus (interface) in use
> >> bu the IP.
> >>
> >> 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.
> >> iio_backend_bus_reg_read
> >> generic bus read, bus-type dependent
> >> iio_backend_bus_read_write
> >> generic bus write, bus-type dependent
> >>
> >> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> >> ---
> >> drivers/iio/industrialio-backend.c | 151 +++++++++++++++++++++++++++++++++++++
> >> include/linux/iio/backend.h | 24 ++++++
> >> 2 files changed, 175 insertions(+)
> >>
> >> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> >> index a52a6b61c8b5..1f60c8626be7 100644
> >> --- a/drivers/iio/industrialio-backend.c
> >> +++ b/drivers/iio/industrialio-backend.c
> >> @@ -718,6 +718,157 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
> >> return 0;
> >> }
> >
> >> +
> >> +/**
> >> + * iio_backend_buffer_enable - Enable data buffering
> > Data buffering is a very vague term. Perhaps some more detail on what
> > this means?
>
> for this DAC IP, it is the dma buffer where i write the samples,
> for other non-dac frontends may be something different, so i kept it
> generic. Not sure what a proper name may be, maybe
>
> "Enable optional data buffer" ?
How do you 'enable' a buffer? Enable writing into it maybe?
>
>
> >> + * @back: Backend device
> >> + *
> >> + * 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_bus_reg_read - Read from the interface bus
> >> + * @back: Backend device
> >> + * @reg: Register valule
> >> + * @val: Pointer to register value
> >> + * @size: Size, in bytes
> >> + *
> >> + * A backend may operate on a specific interface with a related bus.
> >> + * Read from the interface bus.
> > So this is effectively routing control plane data through the offloaded
> > bus? That sounds a lot more like a conventional bus than IIO backend.
> > Perhaps it should be presented as that with the IIO device attached
> > to that bus? I don't fully understand what is wired up here.
> >
> Mainly, an IP may include a bus as 16bit parallel, or LVDS, or similar
> to QSPI as in my case (ad3552r).
ok.
If this is a bus used for both control and dataplane, then we should really
be presenting it as a bus (+ offload) similar to do for spi + offload.
>
> In particular, the bus is physically as a QSPI bus, but the data format
> over it is a bit different.
>
> So ad3552r needs this 5 lanes bus + double data rate to reach 33MUPS.
>
> https://analogdevicesinc.github.io/hdl/library/axi_ad3552r/index.html
>
Jonathan
Hi Jonathan,
On 03/09/24 9:11 PM, Jonathan Cameron wrote:
> On Mon, 2 Sep 2024 16:03:22 +0200
> Angelo Dureghello <adureghello@baylibre.com> wrote:
>
>> Hi Jonathan,
>>
>> thanks for the feedbacks,
>>
>> On 31/08/24 1:23 PM, Jonathan Cameron wrote:
>>> On Thu, 29 Aug 2024 14:32:00 +0200
>>> Angelo Dureghello <adureghello@baylibre.com> wrote:
>>>
>>>> From: Angelo Dureghello <adureghello@baylibre.com>
>>>>
>>>> Extend backend features with new calls needed later on this
>>>> patchset from axi version of ad3552r.
>>>>
>>>> A bus type property has been added to the devicetree to
>>>> inform the backend about the type of bus (interface) in use
>>>> bu the IP.
>>>>
>>>> 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.
>>>> iio_backend_bus_reg_read
>>>> generic bus read, bus-type dependent
>>>> iio_backend_bus_read_write
>>>> generic bus write, bus-type dependent
>>>>
>>>> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
>>>> ---
>>>> drivers/iio/industrialio-backend.c | 151 +++++++++++++++++++++++++++++++++++++
>>>> include/linux/iio/backend.h | 24 ++++++
>>>> 2 files changed, 175 insertions(+)
>>>>
>>>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
>>>> index a52a6b61c8b5..1f60c8626be7 100644
>>>> --- a/drivers/iio/industrialio-backend.c
>>>> +++ b/drivers/iio/industrialio-backend.c
>>>> @@ -718,6 +718,157 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
>>>> return 0;
>>>> }
>>>
>>>> +
>>>> +/**
>>>> + * iio_backend_buffer_enable - Enable data buffering
>>> Data buffering is a very vague term. Perhaps some more detail on what
>>> this means?
>> for this DAC IP, it is the dma buffer where i write the samples,
>> for other non-dac frontends may be something different, so i kept it
>> generic. Not sure what a proper name may be, maybe
>>
>> "Enable optional data buffer" ?
> How do you 'enable' a buffer? Enable writing into it maybe?
for the current case, this is done using the custom register
of the AXI IP, enabling a "stream".
return regmap_set_bits(st->regmap, AXI_DAC_REG_CUSTOM_CTRL,
AXI_DAC_STREAM_ENABLE);
Functionally, looks like dma data is processed (sent over qspi)
when the stream is enabled.
Maybe a name as "stream_enable" would me more appropriate ?
"Stream" seems less generic btw.
>>
>>>> + * @back: Backend device
>>>> + *
>>>> + * 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_bus_reg_read - Read from the interface bus
>>>> + * @back: Backend device
>>>> + * @reg: Register valule
>>>> + * @val: Pointer to register value
>>>> + * @size: Size, in bytes
>>>> + *
>>>> + * A backend may operate on a specific interface with a related bus.
>>>> + * Read from the interface bus.
>>> So this is effectively routing control plane data through the offloaded
>>> bus? That sounds a lot more like a conventional bus than IIO backend.
>>> Perhaps it should be presented as that with the IIO device attached
>>> to that bus? I don't fully understand what is wired up here.
>>>
>> Mainly, an IP may include a bus as 16bit parallel, or LVDS, or similar
>> to QSPI as in my case (ad3552r).
> ok.
>
> If this is a bus used for both control and dataplane, then we should really
> be presenting it as a bus (+ offload) similar to do for spi + offload.
>
>> In particular, the bus is physically as a QSPI bus, but the data format
>> over it is a bit different.
>>
>> So ad3552r needs this 5 lanes bus + double data rate to reach 33MUPS.
>>
>> https://analogdevicesinc.github.io/hdl/library/axi_ad3552r/index.html
>>
> Jonathan
>
--
,,, Angelo Dureghello
:: :. BayLibre -runtime team- Developer
:`___:
`____:
On Wed, 2024-09-04 at 14:01 +0200, Angelo Dureghello wrote:
> Hi Jonathan,
>
> On 03/09/24 9:11 PM, Jonathan Cameron wrote:
> > On Mon, 2 Sep 2024 16:03:22 +0200
> > Angelo Dureghello <adureghello@baylibre.com> wrote:
> >
> > > Hi Jonathan,
> > >
> > > thanks for the feedbacks,
> > >
> > > On 31/08/24 1:23 PM, Jonathan Cameron wrote:
> > > > On Thu, 29 Aug 2024 14:32:00 +0200
> > > > Angelo Dureghello <adureghello@baylibre.com> wrote:
> > > >
> > > > > From: Angelo Dureghello <adureghello@baylibre.com>
> > > > >
> > > > > Extend backend features with new calls needed later on this
> > > > > patchset from axi version of ad3552r.
> > > > >
> > > > > A bus type property has been added to the devicetree to
> > > > > inform the backend about the type of bus (interface) in use
> > > > > bu the IP.
> > > > >
> > > > > 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.
> > > > > iio_backend_bus_reg_read
> > > > > generic bus read, bus-type dependent
> > > > > iio_backend_bus_read_write
> > > > > generic bus write, bus-type dependent
> > > > >
> > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > > > > ---
> > > > > drivers/iio/industrialio-backend.c | 151
> > > > > +++++++++++++++++++++++++++++++++++++
> > > > > include/linux/iio/backend.h | 24 ++++++
> > > > > 2 files changed, 175 insertions(+)
> > > > >
> > > > > diff --git a/drivers/iio/industrialio-backend.c
> > > > > b/drivers/iio/industrialio-backend.c
> > > > > index a52a6b61c8b5..1f60c8626be7 100644
> > > > > --- a/drivers/iio/industrialio-backend.c
> > > > > +++ b/drivers/iio/industrialio-backend.c
> > > > > @@ -718,6 +718,157 @@ static int __devm_iio_backend_get(struct device
> > > > > *dev, struct iio_backend *back)
> > > > > return 0;
> > > > > }
> > > >
> > > > > +
> > > > > +/**
> > > > > + * iio_backend_buffer_enable - Enable data buffering
> > > > Data buffering is a very vague term. Perhaps some more detail on what
> > > > this means?
> > > for this DAC IP, it is the dma buffer where i write the samples,
> > > for other non-dac frontends may be something different, so i kept it
> > > generic. Not sure what a proper name may be, maybe
> > >
> > > "Enable optional data buffer" ?
> > How do you 'enable' a buffer? Enable writing into it maybe?
>
> for the current case, this is done using the custom register
> of the AXI IP, enabling a "stream".
>
> return regmap_set_bits(st->regmap, AXI_DAC_REG_CUSTOM_CTRL,
> AXI_DAC_STREAM_ENABLE);
>
> Functionally, looks like dma data is processed (sent over qspi)
> when the stream is enabled.
>
> Maybe a name as "stream_enable" would me more appropriate ?
> "Stream" seems less generic btw.
>
Yes, stream enable is very specific for this usecase. This is basically
connected to typical IIO buffering. So maybe we could either:
1) Embed struct iio_buffer_setup_ops in the backend ops struct;
2) Or just define directly the ones we need now in backend ops.
> > >
> > > > > + * @back: Backend device
> > > > > + *
> > > > > + * 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_bus_reg_read - Read from the interface bus
> > > > > + * @back: Backend device
> > > > > + * @reg: Register valule
> > > > > + * @val: Pointer to register value
> > > > > + * @size: Size, in bytes
> > > > > + *
> > > > > + * A backend may operate on a specific interface with a related bus.
> > > > > + * Read from the interface bus.
> > > > So this is effectively routing control plane data through the offloaded
> > > > bus? That sounds a lot more like a conventional bus than IIO backend.
> > > > Perhaps it should be presented as that with the IIO device attached
> > > > to that bus? I don't fully understand what is wired up here.
> > > >
> > > Mainly, an IP may include a bus as 16bit parallel, or LVDS, or similar
> > > to QSPI as in my case (ad3552r).
> > ok.
> >
> > If this is a bus used for both control and dataplane, then we should really
> > be presenting it as a bus (+ offload) similar to do for spi + offload.
> >
Yes, indeed. In this case we also use the axi-dac core for controlling the
frontend device (accessing it's register) which is fairly weird. But not sure
how we can do it differently. For the spi_engine that is really a spi controller
with the extra offloading capability. For this one, it's now "acting" as a spi
controller but in the future it may also "act" as a parallel controller (the
axi-adc already is in works for that with the ad7606 series).
I was also very skeptical when I first saw these new functions but I'm not
really sure how to do it differently. I mean, it also does not make much sense
to have an additional bus driver as the register maps are the same. Not sure if
turning it in a MFD device, helps...
FWIW, I still don't fully understand why can't we have this supported by the
spi_engine core. My guess is that we need features from the axi-dac (for the
dataplane) so we are incorporating the controlplane on it instead of going
spi_engine + axi-dac.
Also want to leave a quick note about LVDS (that was mentioned). That interface
is typically only used for data so I'm not seeing any special handling like this
for that interface.
- Nuno Sá
> >
On Thu, 05 Sep 2024 12:28:51 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Wed, 2024-09-04 at 14:01 +0200, Angelo Dureghello wrote:
> > Hi Jonathan,
> >
> > On 03/09/24 9:11 PM, Jonathan Cameron wrote:
> > > On Mon, 2 Sep 2024 16:03:22 +0200
> > > Angelo Dureghello <adureghello@baylibre.com> wrote:
> > >
> > > > Hi Jonathan,
> > > >
> > > > thanks for the feedbacks,
> > > >
> > > > On 31/08/24 1:23 PM, Jonathan Cameron wrote:
> > > > > On Thu, 29 Aug 2024 14:32:00 +0200
> > > > > Angelo Dureghello <adureghello@baylibre.com> wrote:
> > > > >
> > > > > > From: Angelo Dureghello <adureghello@baylibre.com>
> > > > > >
> > > > > > Extend backend features with new calls needed later on this
> > > > > > patchset from axi version of ad3552r.
> > > > > >
> > > > > > A bus type property has been added to the devicetree to
> > > > > > inform the backend about the type of bus (interface) in use
> > > > > > bu the IP.
> > > > > >
> > > > > > 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.
> > > > > > iio_backend_bus_reg_read
> > > > > > generic bus read, bus-type dependent
> > > > > > iio_backend_bus_read_write
> > > > > > generic bus write, bus-type dependent
> > > > > >
> > > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > > > > > ---
> > > > > > drivers/iio/industrialio-backend.c | 151
> > > > > > +++++++++++++++++++++++++++++++++++++
> > > > > > include/linux/iio/backend.h | 24 ++++++
> > > > > > 2 files changed, 175 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/iio/industrialio-backend.c
> > > > > > b/drivers/iio/industrialio-backend.c
> > > > > > index a52a6b61c8b5..1f60c8626be7 100644
> > > > > > --- a/drivers/iio/industrialio-backend.c
> > > > > > +++ b/drivers/iio/industrialio-backend.c
> > > > > > @@ -718,6 +718,157 @@ static int __devm_iio_backend_get(struct device
> > > > > > *dev, struct iio_backend *back)
> > > > > > return 0;
> > > > > > }
> > > > >
> > > > > > +
> > > > > > +/**
> > > > > > + * iio_backend_buffer_enable - Enable data buffering
> > > > > Data buffering is a very vague term. Perhaps some more detail on what
> > > > > this means?
> > > > for this DAC IP, it is the dma buffer where i write the samples,
> > > > for other non-dac frontends may be something different, so i kept it
> > > > generic. Not sure what a proper name may be, maybe
> > > >
> > > > "Enable optional data buffer" ?
> > > How do you 'enable' a buffer? Enable writing into it maybe?
> >
> > for the current case, this is done using the custom register
> > of the AXI IP, enabling a "stream".
> >
> > return regmap_set_bits(st->regmap, AXI_DAC_REG_CUSTOM_CTRL,
> > AXI_DAC_STREAM_ENABLE);
> >
> > Functionally, looks like dma data is processed (sent over qspi)
> > when the stream is enabled.
> >
> > Maybe a name as "stream_enable" would me more appropriate ?
> > "Stream" seems less generic btw.
> >
Ok. Maybe "enable buffer filling" or something like that?
>
> Yes, stream enable is very specific for this usecase. This is basically
> connected to typical IIO buffering. So maybe we could either:
>
> 1) Embed struct iio_buffer_setup_ops in the backend ops struct;
> 2) Or just define directly the ones we need now in backend ops.
Structurally whatever makes sense - I was just quibbling over the
documentation ;)
>
> > > >
> > > > > > + * @back: Backend device
> > > > > > + *
> > > > > > + * 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_bus_reg_read - Read from the interface bus
> > > > > > + * @back: Backend device
> > > > > > + * @reg: Register valule
> > > > > > + * @val: Pointer to register value
> > > > > > + * @size: Size, in bytes
> > > > > > + *
> > > > > > + * A backend may operate on a specific interface with a related bus.
> > > > > > + * Read from the interface bus.
> > > > > So this is effectively routing control plane data through the offloaded
> > > > > bus? That sounds a lot more like a conventional bus than IIO backend.
> > > > > Perhaps it should be presented as that with the IIO device attached
> > > > > to that bus? I don't fully understand what is wired up here.
> > > > >
> > > > Mainly, an IP may include a bus as 16bit parallel, or LVDS, or similar
> > > > to QSPI as in my case (ad3552r).
> > > ok.
> > >
> > > If this is a bus used for both control and dataplane, then we should really
> > > be presenting it as a bus (+ offload) similar to do for spi + offload.
> > >
>
> Yes, indeed. In this case we also use the axi-dac core for controlling the
> frontend device (accessing it's register) which is fairly weird. But not sure
> how we can do it differently. For the spi_engine that is really a spi controller
> with the extra offloading capability. For this one, it's now "acting" as a spi
> controller but in the future it may also "act" as a parallel controller (the
> axi-adc already is in works for that with the ad7606 series).
>
> I was also very skeptical when I first saw these new functions but I'm not
> really sure how to do it differently. I mean, it also does not make much sense
> to have an additional bus driver as the register maps are the same. Not sure if
> turning it in a MFD device, helps...
Hmm. A given adi-axi-adc interface is going to be one of (or something else)
1) SPI(ish) controller + offloads like this one.
2) Parallel bus - data only
3) Parallel bus with control.
Maybe we argue these are tightly coupled enough that we don't care but it
feels like a direction that might bite us in the long run, particularly
if we end up dt bindings that are hard to work with if we change
how this fits together - imagine an SPI engine with a mode that does work
for this + an SPI offload engine that works with that.
Then the binding here will be hard to deal with.
>
> FWIW, I still don't fully understand why can't we have this supported by the
> spi_engine core. My guess is that we need features from the axi-dac (for the
> dataplane) so we are incorporating the controlplane on it instead of going
> spi_engine + axi-dac.
>
> Also want to leave a quick note about LVDS (that was mentioned). That interface
> is typically only used for data so I'm not seeing any special handling like this
> for that interface.
Makes sense. I'm a bit surprised that the parallel bus being used for control
is on the list given that is also a bit messy to do (need some signalling that
direction is changing or a lot of wires).
Jonathan
>
> - Nuno Sá
> > >
© 2016 - 2026 Red Hat, Inc.