[PATCH v3 0/8] iio: add new backend framework

[PATCH v3 0/8] iio: add new backend framework

[Nuno Sa via B4 Relay]

v1:
 https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f5175273b81dbfe40b7f0daffcdc67d6cb8ff

v2:
 https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.com

Changes in v3:
- Patch 1:
 * Use proposed generic schema [1]. Also make it a required property;
 * Improved the commit message.
- Patch 2:
 * Improved commit message.
- Patch 4:
 * Namespace all IIO DMAENGINE buffer exports;
 * Removed unrelated new line removal change.
- Patch 5:
 * Namespace all IIO backend exports.
- Patch 6:
 * Set backend.h in alphabetical order;
 * Import IIO backend namespace.
- Patch 7:
 * Don't depend on OF in kbuild anymore;
 * Import IIO backend namespace.

For the bindings patches, I tried not to enter into much details about
the IIO framework as I think specifics of the implementation don't care
from the bindings perspective. Hopefully the commit messages are good
enough.

I'm also aware that patch 1 is not backward compatible but we are
anyways doing it on the driver side (and on the driver the property is
indeed required). Anyways, just let me know if making the property
required is not acceptable (I'm fairly confident no one was using the
upstream version of the driver and so validating devicetrees for it). 

Keeping the block diagram in v3's cover so we don't have to follow links
to check the one of the typicals setups. 

                                           -------------------------------------------------------
 ------------------                        | -----------         ------------      -------  FPGA |
 |     ADC        |------------------------| | AXI ADC |---------| DMA CORE |------| RAM |       |
 | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------|          |------|     |       |
 |                |------------------------| -----------         ------------      -------       |
 ------------------                        -------------------------------------------------------
 
Jonathan, I was also tempted in including the diagram in the source
file. Would that be a good idea?

[1]: https://github.com/devicetree-org/dt-schema/pull/120

---
Nuno Sa (7):
      dt-bindings: adc: ad9467: add new io-backend property
      dt-bindings: adc: axi-adc: deprecate 'adi,adc-dev'
      driver: core: allow modifying device_links flags
      iio: buffer-dmaengine: export buffer alloc and free functions
      iio: add the IIO backend framework
      iio: adc: ad9467: convert to backend framework
      iio: adc: adi-axi-adc: move to backend framework

Olivier Moysan (1):
      of: property: add device link support for io-backends

 .../devicetree/bindings/iio/adc/adi,ad9467.yaml    |   5 +
 .../devicetree/bindings/iio/adc/adi,axi-adc.yaml   |   4 +-
 MAINTAINERS                                        |   8 +
 drivers/base/core.c                                |  14 +-
 drivers/iio/Kconfig                                |   5 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/adc/Kconfig                            |   4 +-
 drivers/iio/adc/ad9467.c                           | 243 +++++++------
 drivers/iio/adc/adi-axi-adc.c                      | 381 +++++---------------
 drivers/iio/buffer/industrialio-buffer-dmaengine.c |   8 +-
 drivers/iio/industrialio-backend.c                 | 386 +++++++++++++++++++++
 drivers/of/property.c                              |   2 +
 include/linux/iio/adc/adi-axi-adc.h                |  68 ----
 include/linux/iio/backend.h                        |  68 ++++
 include/linux/iio/buffer-dmaengine.h               |   3 +
 15 files changed, 730 insertions(+), 470 deletions(-)
---
base-commit: 3cde3cdb0b23151dbaac476b17eb1868335993aa
change-id: 20231120-dev-iio-backend-d14b473a1d9f
--

Thanks!
- Nuno Sá

[PATCH v3 1/8] dt-bindings: adc: ad9467: add new io-backend property

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

The ad9467 will make use of the new IIO backend framework which is a
provider - consumer interface where IIO backends provide services to
consumers. As such, and being this device a consumer,  add the new
generic io-backend property to the bindings.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
index 7aa748d6b7a0..74e6827cbd47 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
@@ -44,6 +44,9 @@ properties:
       Pin that controls the powerdown mode of the device.
     maxItems: 1
 
+  io-backends:
+    maxItems: 1
+
   reset-gpios:
     description:
       Reset pin for the device.
@@ -54,6 +57,7 @@ required:
   - reg
   - clocks
   - clock-names
+  - io-backends
 
 additionalProperties: false
 
@@ -68,6 +72,7 @@ examples:
             reg = <0>;
             clocks = <&adc_clk>;
             clock-names = "adc-clk";
+            io-backends = <&iio_backend>;
         };
     };
 ...

-- 
2.43.0

Re: [PATCH v3 1/8] dt-bindings: adc: ad9467: add new io-backend property

[Rob Herring]

On Wed, 13 Dec 2023 16:02:32 +0100, Nuno Sa wrote:
> The ad9467 will make use of the new IIO backend framework which is a
> provider - consumer interface where IIO backends provide services to
> consumers. As such, and being this device a consumer,  add the new
> generic io-backend property to the bindings.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml: io-backends: missing type definition

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231213-dev-iio-backend-v3-1-bb9f12a5c6dc@analog.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v3 1/8] dt-bindings: adc: ad9467: add new io-backend property

[Nuno Sá]

On Wed, 2023-12-13 at 11:55 -0600, Rob Herring wrote:
> 
> On Wed, 13 Dec 2023 16:02:32 +0100, Nuno Sa wrote:
> > The ad9467 will make use of the new IIO backend framework which is a
> > provider - consumer interface where IIO backends provide services to
> > consumers. As such, and being this device a consumer,  add the new
> > generic io-backend property to the bindings.
> > 
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> >  Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-
> ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml: io-backends:
> missing type definition
> 
> doc reference errors (make refcheckdocs):
> 
> See
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231213-dev-iio-backend-v3-1-bb9f12a5c6dc@analog.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

This is kind of expected as the property is being proposed as a generic one...

- Nuno Sá

Re: [PATCH v3 1/8] dt-bindings: adc: ad9467: add new io-backend property

[Rob Herring]

On Wed, Dec 13, 2023 at 04:02:32PM +0100, Nuno Sa wrote:
> The ad9467 will make use of the new IIO backend framework which is a
> provider - consumer interface where IIO backends provide services to
> consumers. As such, and being this device a consumer,  add the new
> generic io-backend property to the bindings.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
> index 7aa748d6b7a0..74e6827cbd47 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
> @@ -44,6 +44,9 @@ properties:
>        Pin that controls the powerdown mode of the device.
>      maxItems: 1
>  
> +  io-backends:
> +    maxItems: 1
> +
>    reset-gpios:
>      description:
>        Reset pin for the device.
> @@ -54,6 +57,7 @@ required:
>    - reg
>    - clocks
>    - clock-names
> +  - io-backends

New required properties are an ABI break. Please justify this in the 
commit message.

Rob

Re: [PATCH v3 1/8] dt-bindings: adc: ad9467: add new io-backend property

[Nuno Sá]

On Thu, 2023-12-14 at 11:05 -0600, Rob Herring wrote:
> On Wed, Dec 13, 2023 at 04:02:32PM +0100, Nuno Sa wrote:
> > The ad9467 will make use of the new IIO backend framework which is a
> > provider - consumer interface where IIO backends provide services to
> > consumers. As such, and being this device a consumer,  add the new
> > generic io-backend property to the bindings.
> > 
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> >  Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
> > b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
> > index 7aa748d6b7a0..74e6827cbd47 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
> > @@ -44,6 +44,9 @@ properties:
> >        Pin that controls the powerdown mode of the device.
> >      maxItems: 1
> >  
> > +  io-backends:
> > +    maxItems: 1
> > +
> >    reset-gpios:
> >      description:
> >        Reset pin for the device.
> > @@ -54,6 +57,7 @@ required:
> >    - reg
> >    - clocks
> >    - clock-names
> > +  - io-backends
> 
> New required properties are an ABI break. Please justify this in the 
> commit message.
> 

Yeah, I know... I'll further comment on the cover and depending on the
conclusion we get I'll justify it (or not if not needed) in the message.

- Nuno Sá

[PATCH v3 2/8] dt-bindings: adc: axi-adc: deprecate 'adi,adc-dev'

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

'adi,adc-dev' is now deprecated and must not be used anymore. Hence,
also remove it from being required.

The reason why it's being deprecated is because the axi-adc CORE is now
an IIO service provider hardware (IIO backends) for consumers to make use
of. Before, the logic with 'adi,adc-dev' was the opposite (it was kind
of consumer referencing other nodes/devices) and that proved to be wrong
and to not scale.

Now, IIO consumers of this hardware are expected to reference it using the
io-backends property.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
index 9996dd93f84b..835b40063343 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
@@ -39,12 +39,12 @@ properties:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
       A reference to a the actual ADC to which this FPGA ADC interfaces to.
+    deprecated: true
 
 required:
   - compatible
   - dmas
   - reg
-  - adi,adc-dev
 
 additionalProperties: false
 
@@ -55,7 +55,5 @@ examples:
         reg = <0x44a00000 0x10000>;
         dmas = <&rx_dma 0>;
         dma-names = "rx";
-
-        adi,adc-dev = <&spi_adc>;
     };
 ...

-- 
2.43.0

[PATCH v3 3/8] driver: core: allow modifying device_links flags

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

If a device_link is previously created (eg: via
fw_devlink_create_devlink()) before the supplier + consumer are both
present and bound to their respective drivers, there's no way to set
DL_FLAG_AUTOREMOVE_CONSUMER anymore while one can still set
DL_FLAG_AUTOREMOVE_SUPPLIER. Hence, rework the flags checks to allow
for DL_FLAG_AUTOREMOVE_CONSUMER in the same way
DL_FLAG_AUTOREMOVE_SUPPLIER is done.

While at it, make sure that we are never left with
DL_FLAG_AUTOPROBE_CONSUMER set together with one of
DL_FLAG_AUTOREMOVE_CONSUMER or DL_FLAG_AUTOREMOVE_SUPPLIER.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/base/core.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 67ba592afc77..fdbb5abc75d5 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -807,11 +807,15 @@ struct device_link *device_link_add(struct device *consumer,
 		 * update the existing link to stay around longer.
 		 */
 		if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) {
-			if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) {
-				link->flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER;
-				link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER;
-			}
-		} else if (!(flags & DL_FLAG_AUTOREMOVE_CONSUMER)) {
+			link->flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER;
+			link->flags &= ~DL_FLAG_AUTOPROBE_CONSUMER;
+			link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER;
+
+		} else if (flags & DL_FLAG_AUTOREMOVE_CONSUMER) {
+			link->flags &= ~DL_FLAG_AUTOREMOVE_SUPPLIER;
+			link->flags &= ~DL_FLAG_AUTOPROBE_CONSUMER;
+			link->flags |= DL_FLAG_AUTOREMOVE_CONSUMER;
+		} else {
 			link->flags &= ~(DL_FLAG_AUTOREMOVE_CONSUMER |
 					 DL_FLAG_AUTOREMOVE_SUPPLIER);
 		}

-- 
2.43.0

[PATCH v3 4/8] of: property: add device link support for io-backends

[Nuno Sa via B4 Relay]

From: Olivier Moysan <olivier.moysan@foss.st.com>

Add support for creating device links out of more DT properties.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/of/property.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index afdaefbd03f6..a4835447759f 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1244,6 +1244,7 @@ DEFINE_SIMPLE_PROP(interconnects, "interconnects", "#interconnect-cells")
 DEFINE_SIMPLE_PROP(iommus, "iommus", "#iommu-cells")
 DEFINE_SIMPLE_PROP(mboxes, "mboxes", "#mbox-cells")
 DEFINE_SIMPLE_PROP(io_channels, "io-channel", "#io-channel-cells")
+DEFINE_SIMPLE_PROP(io_backends, "io-backends", NULL)
 DEFINE_SIMPLE_PROP(interrupt_parent, "interrupt-parent", NULL)
 DEFINE_SIMPLE_PROP(dmas, "dmas", "#dma-cells")
 DEFINE_SIMPLE_PROP(power_domains, "power-domains", "#power-domain-cells")
@@ -1334,6 +1335,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_iommu_maps, .optional = true, },
 	{ .parse_prop = parse_mboxes, },
 	{ .parse_prop = parse_io_channels, },
+	{ .parse_prop = parse_io_backends, },
 	{ .parse_prop = parse_interrupt_parent, },
 	{ .parse_prop = parse_dmas, .optional = true, },
 	{ .parse_prop = parse_power_domains, },

-- 
2.43.0

[PATCH v3 5/8] iio: buffer-dmaengine: export buffer alloc and free functions

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Export iio_dmaengine_buffer_free() and iio_dmaengine_buffer_alloc().
This is in preparation of introducing IIO backends support. This will
allow us to allocate a buffer and control it's lifetime from a device
different from the one holding the DMA firmware properties. Effectively,
in this case the struct device holding the firmware information about
the DMA channels is not the same as iio_dev->dev.parent (typical case).

While at it, namespace the buffer-dmaengine exports and update the
current user of these buffers.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c                      | 1 +
 drivers/iio/buffer/industrialio-buffer-dmaengine.c | 8 +++++---
 include/linux/iio/buffer-dmaengine.h               | 3 +++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index c247ff1541d2..0f21d1d98b9f 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -447,3 +447,4 @@ module_platform_driver(adi_axi_adc_driver);
 MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
 MODULE_DESCRIPTION("Analog Devices Generic AXI ADC IP core driver");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_DMAENGINE_BUFFER);
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index 5f85ba38e6f6..0d53c0a07b0d 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -159,7 +159,7 @@ static const struct iio_dev_attr *iio_dmaengine_buffer_attrs[] = {
  * Once done using the buffer iio_dmaengine_buffer_free() should be used to
  * release it.
  */
-static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
+struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
 	const char *channel)
 {
 	struct dmaengine_buffer *dmaengine_buffer;
@@ -210,6 +210,7 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
 	kfree(dmaengine_buffer);
 	return ERR_PTR(ret);
 }
+EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_alloc, IIO_DMAENGINE_BUFFER);
 
 /**
  * iio_dmaengine_buffer_free() - Free dmaengine buffer
@@ -217,7 +218,7 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
  *
  * Frees a buffer previously allocated with iio_dmaengine_buffer_alloc().
  */
-static void iio_dmaengine_buffer_free(struct iio_buffer *buffer)
+void iio_dmaengine_buffer_free(struct iio_buffer *buffer)
 {
 	struct dmaengine_buffer *dmaengine_buffer =
 		iio_buffer_to_dmaengine_buffer(buffer);
@@ -227,6 +228,7 @@ static void iio_dmaengine_buffer_free(struct iio_buffer *buffer)
 
 	iio_buffer_put(buffer);
 }
+EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_free, IIO_DMAENGINE_BUFFER);
 
 static void __devm_iio_dmaengine_buffer_free(void *buffer)
 {
@@ -288,7 +290,7 @@ int devm_iio_dmaengine_buffer_setup(struct device *dev,
 
 	return iio_device_attach_buffer(indio_dev, buffer);
 }
-EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_setup);
+EXPORT_SYMBOL_NS_GPL(devm_iio_dmaengine_buffer_setup, IIO_DMAENGINE_BUFFER);
 
 MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
 MODULE_DESCRIPTION("DMA buffer for the IIO framework");
diff --git a/include/linux/iio/buffer-dmaengine.h b/include/linux/iio/buffer-dmaengine.h
index 5c355be89814..cbb8ba957fad 100644
--- a/include/linux/iio/buffer-dmaengine.h
+++ b/include/linux/iio/buffer-dmaengine.h
@@ -10,6 +10,9 @@
 struct iio_dev;
 struct device;
 
+struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
+					      const char *channel);
+void iio_dmaengine_buffer_free(struct iio_buffer *buffer);
 int devm_iio_dmaengine_buffer_setup(struct device *dev,
 				    struct iio_dev *indio_dev,
 				    const char *channel);

-- 
2.43.0

[PATCH v3 6/8] iio: add the IIO backend framework

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

This is a Framework to handle complex IIO aggregate devices.

The typical architecture is to have one device as the frontend device which
can be "linked" against one or multiple backend devices. All the IIO and
userspace interface is expected to be registers/managed by the frontend
device which will callback into the backends when needed (to get/set
some configuration that it does not directly control).

The basic framework interface is pretty simple:
 - Backends should register themselves with @devm_iio_backend_register()
 - Frontend devices should get backends with @devm_iio_backend_get()

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 MAINTAINERS                        |   8 +
 drivers/iio/Kconfig                |   5 +
 drivers/iio/Makefile               |   1 +
 drivers/iio/industrialio-backend.c | 386 +++++++++++++++++++++++++++++++++++++
 include/linux/iio/backend.h        |  68 +++++++
 5 files changed, 468 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1338e1176ea5..637ce907dcbf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10320,6 +10320,14 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	drivers/media/rc/iguanair.c
 
+IIO BACKEND FRAMEWORK
+M:	Nuno Sa <nuno.sa@analog.com>
+R:	Olivier Moysan <olivier.moysan@foss.st.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	drivers/iio/industrialio-backend.c
+F:	include/linux/iio/backend.h
+
 IIO DIGITAL POTENTIOMETER DAC
 M:	Peter Rosin <peda@axentia.se>
 L:	linux-iio@vger.kernel.org
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 52eb46ef84c1..451671112f73 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -71,6 +71,11 @@ config IIO_TRIGGERED_EVENT
 	help
 	  Provides helper functions for setting up triggered events.
 
+config IIO_BACKEND
+	tristate
+	help
+	  Framework to handle complex IIO aggregate devices.
+
 source "drivers/iio/accel/Kconfig"
 source "drivers/iio/adc/Kconfig"
 source "drivers/iio/addac/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 9622347a1c1b..0ba0e1521ba4 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_IIO_GTS_HELPER) += industrialio-gts-helper.o
 obj-$(CONFIG_IIO_SW_DEVICE) += industrialio-sw-device.o
 obj-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
 obj-$(CONFIG_IIO_TRIGGERED_EVENT) += industrialio-triggered-event.o
+obj-$(CONFIG_IIO_BACKEND) += industrialio-backend.o
 
 obj-y += accel/
 obj-y += adc/
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
new file mode 100644
index 000000000000..a2116d53869d
--- /dev/null
+++ b/drivers/iio/industrialio-backend.c
@@ -0,0 +1,386 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Framework to handle complex IIO aggregate devices.
+ *
+ * The typical architecture is to have one device as the frontend device which
+ * can be "linked" against one or multiple backend devices. All the IIO and
+ * userspace interface is expected to be registers/managed by the frontend
+ * device which will callback into the backends when needed (to get/set some
+ * configuration that it does not directly control).
+ *
+ * The framework interface is pretty simple:
+ *   - Backends should register themselves with @devm_iio_backend_register()
+ *   - Frontend devices should get backends with @devm_iio_backend_get()
+ *
+ * Also to note that the primary target for this framework are converters like
+ * ADC/DACs so @iio_backend_ops will have some operations typical of converter
+ * devices. On top of that, this is "generic" for all IIO which means any kind
+ * of device can make use of the framework. That said, If the @iio_backend_ops
+ * struct begins to grow out of control, we can always refactor things so that
+ * the industrialio-backend.c is only left with the really generic stuff. Then,
+ * we can build on top of it depending on the needs.
+ *
+ * Copyright (C) 2023 Analog Devices Inc.
+ */
+#define pr_fmt(fmt) "iio-backend: " fmt
+
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/list.h>
+#include <linux/lockdep.h>
+#include <linux/kref.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+#include <linux/iio/backend.h>
+
+struct iio_backend {
+	struct list_head entry;
+	const struct iio_backend_ops *ops;
+	struct device *dev;
+	struct module *owner;
+	void *priv;
+	/*
+	 * mutex used to synchronize backend callback access with concurrent
+	 * calls to @iio_backend_unregister. The lock makes sure a device is
+	 * not unregistered while a callback is being run.
+	 */
+	struct mutex lock;
+	struct kref ref;
+};
+
+/*
+ * Helper struct for requesting buffers. Allows for multiple buffers per
+ * backend.
+ */
+struct iio_backend_buffer_pair {
+	struct iio_backend *back;
+	struct iio_buffer *buffer;
+};
+
+static LIST_HEAD(iio_back_list);
+static DEFINE_MUTEX(iio_back_lock);
+
+/*
+ * Helper macros to properly call backend ops. The main point for these macros
+ * is to properly lock the backend mutex on every call plus checking if the
+ * backend device is still around (by looking at the *ops pointer).
+ */
+
+#define iio_backend_check_op(back, op) ({ \
+	struct iio_backend *____back = back;				\
+	int ____ret = 0;						\
+									\
+	lockdep_assert_held(&____back->lock);				\
+	if (!____back->ops) {						\
+		pr_warn_once("Backend is no longer available\n");	\
+		____ret = -ENODEV;					\
+	} else if (!____back->ops->op) {				\
+		____ret = -EOPNOTSUPP;					\
+	}								\
+									\
+	____ret;								\
+})
+
+#define iio_backend_op_call(back, op, args...) ({		\
+	struct iio_backend *__back = back;			\
+	int __ret;						\
+								\
+	guard(mutex)(&__back->lock);				\
+	__ret = iio_backend_check_op(__back, op);		\
+	if (!__ret)						\
+		__ret = __back->ops->op(__back, ##args);	\
+								\
+	__ret;							\
+})
+
+#define iio_backend_ptr_op_call(back, op, args...) ({		\
+	struct iio_backend *__back = back;			\
+	void *ptr_err;						\
+	int __ret;						\
+								\
+	guard(mutex)(&__back->lock);				\
+	__ret = iio_backend_check_op(__back, op);		\
+	if (__ret)						\
+		ptr_err = ERR_PTR(__ret);			\
+	else							\
+		ptr_err = __back->ops->op(__back, ##args);	\
+								\
+	ptr_err;						\
+})
+
+#define iio_backend_void_op_call(back, op, args...) {		\
+	struct iio_backend *__back = back;			\
+	int __ret;						\
+								\
+	guard(mutex)(&__back->lock);				\
+	__ret = iio_backend_check_op(__back, op);		\
+	if (!__ret)						\
+		__back->ops->op(__back, ##args);		\
+}
+
+/**
+ * iio_backend_chan_enable - Enable a backend channel.
+ * @back:	Backend device.
+ * @chan:	Channel number.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan)
+{
+	return iio_backend_op_call(back, chan_enable, chan);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_chan_enable, IIO_BACKEND);
+
+/**
+ * iio_backend_chan_disable - Disable a backend channel.
+ * @back:	Backend device.
+ * @chan:	Channel number.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_chan_disable(struct iio_backend *back, unsigned int chan)
+{
+	return iio_backend_op_call(back, chan_disable, chan);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_chan_disable, IIO_BACKEND);
+
+/**
+ * iio_backend_chan_enable - Enable the backend.
+ * @back:	Backend device
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_enable(struct iio_backend *back)
+{
+	return iio_backend_op_call(back, enable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_enable, IIO_BACKEND);
+
+/**
+ * iio_backend_disable - Disable the backend.
+ * @back:	Backend device
+ */
+void iio_backend_disable(struct iio_backend *back)
+{
+	iio_backend_void_op_call(back, disable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_disable, IIO_BACKEND);
+
+/**
+ * iio_backend_data_format_set - Configure the channel data format
+ * @back:	Backend device
+ * @chan:	Channel number.
+ * @data:	Data format.
+ *
+ * Properly configure a channel with respect to the expected data format. A
+ * @struct iio_backend_data_fmt must be passed with the settings.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure
+ */
+int iio_backend_data_format_set(struct iio_backend *back, unsigned int chan,
+				const struct iio_backend_data_fmt *data)
+{
+	if (!data || data->type >= IIO_BACKEND_DATA_TYPE_MAX)
+		return -EINVAL;
+
+	return iio_backend_op_call(back, data_format_set, chan, data);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_format_set, IIO_BACKEND);
+
+static void iio_backend_free_buffer(void *arg)
+{
+	struct iio_backend_buffer_pair *pair = arg;
+
+	iio_backend_void_op_call(pair->back, free_buffer, pair->buffer);
+}
+
+/**
+ * devm_iio_backend_request_buffer - Request an IIO buffer from the backend.
+ * @dev:	Device to bind the buffer lifetime.
+ * @back:	Backend device.
+ * @indio_dev:	IIO device.
+ *
+ * Request an IIO buffer from the backend. The type of the buffer (typically
+ * INDIO_BUFFER_HARDWARE) is up to the backend to decide. This is because,
+ * normally, the backend dictates what kind of buffering we can get.
+ *
+ * The backend .free_buffer() hooks is automatically called on @dev detach.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure
+ */
+int devm_iio_backend_request_buffer(struct device *dev,
+				    struct iio_backend *back,
+				    struct iio_dev *indio_dev)
+{
+	struct iio_backend_buffer_pair *pair;
+	struct iio_buffer *buffer;
+
+	buffer = iio_backend_ptr_op_call(back, request_buffer, indio_dev);
+	if (IS_ERR(buffer))
+		return PTR_ERR(buffer);
+
+	/* backend lock should not be need after getting the buffer */
+	pair = devm_kzalloc(dev, sizeof(*pair), GFP_KERNEL);
+	if (!pair)
+		return -ENOMEM;
+
+	/* weak reference should be all what we need */
+	pair->back = back;
+	pair->buffer = buffer;
+
+	return devm_add_action_or_reset(dev, iio_backend_free_buffer, pair);
+}
+EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND);
+
+static void iio_backend_free(struct kref *ref)
+{
+	struct iio_backend *back = container_of(ref, struct iio_backend, ref);
+
+	kfree(back);
+}
+
+static void iio_backend_release(void *arg)
+{
+	struct iio_backend *back = arg;
+
+	module_put(back->owner);
+	kref_put(&back->ref, iio_backend_free);
+}
+
+/**
+ * devm_iio_backend_get - Get a backend device
+ * @dev:	Device where to look for the backend.
+ * @name:	Backend name.
+ *
+ * Get's the backend associated with @dev.
+ *
+ * RETURNS:
+ * A backend pointer, negative error pointer otherwise.
+ */
+struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
+{
+	struct fwnode_handle *fwnode;
+	struct iio_backend *back;
+	int index = 0, ret;
+
+	if (name) {
+		index = device_property_match_string(dev, "io-backends-names",
+						     name);
+		if (index < 0)
+			return ERR_PTR(index);
+	}
+
+	fwnode = fwnode_find_reference(dev_fwnode(dev), "io-backends", index);
+	if (IS_ERR(fwnode)) {
+		dev_err(dev, "Cannot get Firmware reference\n");
+		return ERR_CAST(fwnode);
+	}
+
+	guard(mutex)(&iio_back_lock);
+	list_for_each_entry(back, &iio_back_list, entry) {
+		struct device_link *link;
+
+		if (!device_match_fwnode(back->dev, fwnode))
+			continue;
+
+		fwnode_handle_put(fwnode);
+		kref_get(&back->ref);
+		if (!try_module_get(back->owner)) {
+			dev_err(dev, "Cannot get module reference\n");
+			return ERR_PTR(-ENODEV);
+		}
+
+		ret = devm_add_action_or_reset(dev, iio_backend_release, back);
+		if (ret)
+			return ERR_PTR(ret);
+
+		link = device_link_add(dev, back->dev,
+				       DL_FLAG_AUTOREMOVE_CONSUMER);
+		if (!link)
+			dev_warn(dev, "Could not link to supplier(%s)\n",
+				 dev_name(back->dev));
+
+		dev_dbg(dev, "Found backend(%s) device\n", dev_name(back->dev));
+		return back;
+	}
+
+	fwnode_handle_put(fwnode);
+	return ERR_PTR(-EPROBE_DEFER);
+}
+EXPORT_SYMBOL_NS_GPL(devm_iio_backend_get, IIO_BACKEND);
+
+/**
+ * iio_backend_get_priv - Get driver private data
+ * @back:	Backend device
+ */
+void *iio_backend_get_priv(const struct iio_backend *back)
+{
+	return back->priv;
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_get_priv, IIO_BACKEND);
+
+static void iio_backend_unregister(void *arg)
+{
+	struct iio_backend *back = arg;
+
+	mutex_lock(&iio_back_lock);
+	list_del(&back->entry);
+	mutex_unlock(&iio_back_lock);
+
+	mutex_lock(&back->lock);
+	back->ops = NULL;
+	mutex_unlock(&back->lock);
+	kref_put(&back->ref, iio_backend_free);
+}
+
+/**
+ * devm_iio_backend_register - Register a new backend device
+ * @dev:	Backend device being registered.
+ * @ops:	Backend ops
+ * @priv:	Device private data.
+ *
+ * @ops and @priv are both mandatory. Not providing them results in -EINVAL.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int devm_iio_backend_register(struct device *dev,
+			      const struct iio_backend_ops *ops, void *priv)
+{
+	struct iio_backend *back;
+
+	if (!ops || !priv) {
+		dev_err(dev, "No backend ops or private data given\n");
+		return -EINVAL;
+	}
+
+	back = kzalloc(sizeof(*back), GFP_KERNEL);
+	if (!back)
+		return -ENOMEM;
+
+	kref_init(&back->ref);
+	mutex_init(&back->lock);
+	back->ops = ops;
+	back->owner = dev->driver->owner;
+	back->dev = dev;
+	back->priv = priv;
+	mutex_lock(&iio_back_lock);
+	list_add(&back->entry, &iio_back_list);
+	mutex_unlock(&iio_back_lock);
+
+	return devm_add_action_or_reset(dev, iio_backend_unregister, back);
+}
+EXPORT_SYMBOL_NS_GPL(devm_iio_backend_register, IIO_BACKEND);
+
+MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>");
+MODULE_DESCRIPTION("Framework to handle complex IIO aggregate devices");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
new file mode 100644
index 000000000000..d2555333cb19
--- /dev/null
+++ b/include/linux/iio/backend.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _IIO_BACKEND_H_
+#define _IIO_BACKEND_H_
+
+#include <linux/types.h>
+
+struct iio_backend;
+struct device;
+struct iio_dev;
+
+enum iio_backend_data_type {
+	IIO_BACKEND_TWOS_COMPLEMENT,
+	IIO_BACKEND_OFFSET_BINARY,
+	IIO_BACKEND_DATA_TYPE_MAX
+};
+
+/**
+ * struct iio_backend_data_fmt - Backend data format
+ * @type:		Data type.
+ * @sign_extend:	Bool to tell if the data is sign extended.
+ * @enable:		Enable/Disable the data format module. If disabled,
+ *			not formatting will happen.
+ */
+struct iio_backend_data_fmt {
+	enum iio_backend_data_type type;
+	bool sign_extend;
+	bool enable;
+};
+
+/**
+ * struct iio_backend_ops - operations structure for an iio_backend
+ * @enable:		Enable backend.
+ * @disable:		Disable backend.
+ * @chan_enable:	Enable one channel.
+ * @chan_disable:	Disable one channel.
+ * @data_format_set:	Configure the data format for a specific channel.
+ * @request_buffer:	Request an IIO buffer.
+ * @free_buffer:	Free an IIO buffer.
+ **/
+struct iio_backend_ops {
+	int (*enable)(struct iio_backend *back);
+	void (*disable)(struct iio_backend *back);
+	int (*chan_enable)(struct iio_backend *back, unsigned int chan);
+	int (*chan_disable)(struct iio_backend *back, unsigned int chan);
+	int (*data_format_set)(struct iio_backend *back, unsigned int chan,
+			       const struct iio_backend_data_fmt *data);
+	struct iio_buffer *(*request_buffer)(struct iio_backend *back,
+					     struct iio_dev *indio_dev);
+	void (*free_buffer)(struct iio_backend *back,
+			    struct iio_buffer *buffer);
+};
+
+int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
+int iio_backend_chan_disable(struct iio_backend *back, unsigned int chan);
+int iio_backend_enable(struct iio_backend *back);
+void iio_backend_disable(struct iio_backend *back);
+int iio_backend_data_format_set(struct iio_backend *back, unsigned int chan,
+				const struct iio_backend_data_fmt *data);
+int devm_iio_backend_request_buffer(struct device *dev,
+				    struct iio_backend *back,
+				    struct iio_dev *indio_dev);
+
+void *iio_backend_get_priv(const struct iio_backend *conv);
+struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name);
+int devm_iio_backend_register(struct device *dev,
+			      const struct iio_backend_ops *ops, void *priv);
+
+#endif

-- 
2.43.0

[PATCH v3 7/8] iio: adc: ad9467: convert to backend framework

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Convert the driver to use the new IIO backend framework. The device
functionality is expected to be the same (meaning no added or removed
features).

Also note this patch effectively breaks ABI and that's needed so we can
properly support this device and add needed features making use of the
new IIO framework.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/Kconfig  |   2 +-
 drivers/iio/adc/ad9467.c | 243 +++++++++++++++++++++++++++--------------------
 2 files changed, 143 insertions(+), 102 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 4eebd5161419..10e0e340cdae 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -275,7 +275,7 @@ config AD799X
 config AD9467
 	tristate "Analog Devices AD9467 High Speed ADC driver"
 	depends on SPI
-	depends on ADI_AXI_ADC
+	select IIO_BACKEND
 	help
 	  Say yes here to build support for Analog Devices:
 	  * AD9467 16-Bit, 200 MSPS/250 MSPS Analog-to-Digital Converter
diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 6581fce4ba95..f78019dd140c 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -17,13 +17,12 @@
 #include <linux/of.h>
 
 
+#include <linux/iio/backend.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
 #include <linux/clk.h>
 
-#include <linux/iio/adc/adi-axi-adc.h>
-
 /*
  * ADI High-Speed ADC common spi interface registers
  * See Application-Note AN-877:
@@ -102,15 +101,20 @@
 #define AD9467_REG_VREF_MASK		0x0F
 
 struct ad9467_chip_info {
-	struct adi_axi_adc_chip_info	axi_adc_info;
-	unsigned int			default_output_mode;
-	unsigned int			vref_mask;
+	const char		*name;
+	unsigned int		id;
+	const struct		iio_chan_spec *channels;
+	unsigned int		num_channels;
+	const unsigned int	(*scale_table)[2];
+	int			num_scales;
+	unsigned long		max_rate;
+	unsigned int		default_output_mode;
+	unsigned int		vref_mask;
 };
 
-#define to_ad9467_chip_info(_info)	\
-	container_of(_info, struct ad9467_chip_info, axi_adc_info)
-
 struct ad9467_state {
+	const struct ad9467_chip_info	*info;
+	struct iio_backend		*back;
 	struct spi_device		*spi;
 	struct clk			*clk;
 	unsigned int			output_mode;
@@ -151,10 +155,10 @@ static int ad9467_spi_write(struct spi_device *spi, unsigned int reg,
 	return spi_write(spi, buf, ARRAY_SIZE(buf));
 }
 
-static int ad9467_reg_access(struct adi_axi_adc_conv *conv, unsigned int reg,
+static int ad9467_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 			     unsigned int writeval, unsigned int *readval)
 {
-	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
+	struct ad9467_state *st = iio_priv(indio_dev);
 	struct spi_device *spi = st->spi;
 	int ret;
 
@@ -191,14 +195,13 @@ static const unsigned int ad9467_scale_table[][2] = {
 	{2300, 8}, {2400, 9}, {2500, 10},
 };
 
-static void __ad9467_get_scale(struct adi_axi_adc_conv *conv, int index,
+static void __ad9467_get_scale(struct ad9467_state *st, int index,
 			       unsigned int *val, unsigned int *val2)
 {
-	const struct adi_axi_adc_chip_info *info = conv->chip_info;
-	const struct iio_chan_spec *chan = &info->channels[0];
+	const struct iio_chan_spec *chan = &st->info->channels[0];
 	unsigned int tmp;
 
-	tmp = (info->scale_table[index][0] * 1000000ULL) >>
+	tmp = (st->info->scale_table[index][0] * 1000000ULL) >>
 			chan->scan_type.realbits;
 	*val = tmp / 1000000;
 	*val2 = tmp % 1000000;
@@ -229,52 +232,43 @@ static const struct iio_chan_spec ad9467_channels[] = {
 };
 
 static const struct ad9467_chip_info ad9467_chip_tbl = {
-	.axi_adc_info = {
-		.name = "ad9467",
-		.id = CHIPID_AD9467,
-		.max_rate = 250000000UL,
-		.scale_table = ad9467_scale_table,
-		.num_scales = ARRAY_SIZE(ad9467_scale_table),
-		.channels = ad9467_channels,
-		.num_channels = ARRAY_SIZE(ad9467_channels),
-	},
+	.name = "ad9467",
+	.id = CHIPID_AD9467,
+	.max_rate = 250000000UL,
+	.scale_table = ad9467_scale_table,
+	.num_scales = ARRAY_SIZE(ad9467_scale_table),
+	.channels = ad9467_channels,
+	.num_channels = ARRAY_SIZE(ad9467_channels),
 	.default_output_mode = AD9467_DEF_OUTPUT_MODE,
 	.vref_mask = AD9467_REG_VREF_MASK,
 };
 
 static const struct ad9467_chip_info ad9434_chip_tbl = {
-	.axi_adc_info = {
-		.name = "ad9434",
-		.id = CHIPID_AD9434,
-		.max_rate = 500000000UL,
-		.scale_table = ad9434_scale_table,
-		.num_scales = ARRAY_SIZE(ad9434_scale_table),
-		.channels = ad9434_channels,
-		.num_channels = ARRAY_SIZE(ad9434_channels),
-	},
+	.name = "ad9434",
+	.id = CHIPID_AD9434,
+	.max_rate = 500000000UL,
+	.scale_table = ad9434_scale_table,
+	.num_scales = ARRAY_SIZE(ad9434_scale_table),
+	.channels = ad9434_channels,
+	.num_channels = ARRAY_SIZE(ad9434_channels),
 	.default_output_mode = AD9434_DEF_OUTPUT_MODE,
 	.vref_mask = AD9434_REG_VREF_MASK,
 };
 
 static const struct ad9467_chip_info ad9265_chip_tbl = {
-	.axi_adc_info = {
-		.name = "ad9265",
-		.id = CHIPID_AD9265,
-		.max_rate = 125000000UL,
-		.scale_table = ad9265_scale_table,
-		.num_scales = ARRAY_SIZE(ad9265_scale_table),
-		.channels = ad9467_channels,
-		.num_channels = ARRAY_SIZE(ad9467_channels),
-	},
+	.name = "ad9265",
+	.id = CHIPID_AD9265,
+	.max_rate = 125000000UL,
+	.scale_table = ad9265_scale_table,
+	.num_scales = ARRAY_SIZE(ad9265_scale_table),
+	.channels = ad9467_channels,
+	.num_channels = ARRAY_SIZE(ad9467_channels),
 	.default_output_mode = AD9265_DEF_OUTPUT_MODE,
 	.vref_mask = AD9265_REG_VREF_MASK,
 };
 
-static int ad9467_get_scale(struct adi_axi_adc_conv *conv, int *val, int *val2)
+static int ad9467_get_scale(struct ad9467_state *st, int *val, int *val2)
 {
-	const struct adi_axi_adc_chip_info *info = conv->chip_info;
-	const struct ad9467_chip_info *info1 = to_ad9467_chip_info(info);
-	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
 	unsigned int i, vref_val;
 	int ret;
 
@@ -282,25 +276,23 @@ static int ad9467_get_scale(struct adi_axi_adc_conv *conv, int *val, int *val2)
 	if (ret < 0)
 		return ret;
 
-	vref_val = ret & info1->vref_mask;
+	vref_val = ret & st->info->vref_mask;
 
-	for (i = 0; i < info->num_scales; i++) {
-		if (vref_val == info->scale_table[i][1])
+	for (i = 0; i < st->info->num_scales; i++) {
+		if (vref_val == st->info->scale_table[i][1])
 			break;
 	}
 
-	if (i == info->num_scales)
+	if (i == st->info->num_scales)
 		return -ERANGE;
 
-	__ad9467_get_scale(conv, i, val, val2);
+	__ad9467_get_scale(st, i, val, val2);
 
 	return IIO_VAL_INT_PLUS_MICRO;
 }
 
-static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
+static int ad9467_set_scale(struct ad9467_state *st, int val, int val2)
 {
-	const struct adi_axi_adc_chip_info *info = conv->chip_info;
-	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
 	unsigned int scale_val[2];
 	unsigned int i;
 	int ret;
@@ -308,14 +300,14 @@ static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
 	if (val != 0)
 		return -EINVAL;
 
-	for (i = 0; i < info->num_scales; i++) {
-		__ad9467_get_scale(conv, i, &scale_val[0], &scale_val[1]);
+	for (i = 0; i < st->info->num_scales; i++) {
+		__ad9467_get_scale(st, i, &scale_val[0], &scale_val[1]);
 		if (scale_val[0] != val || scale_val[1] != val2)
 			continue;
 
 		guard(mutex)(&st->lock);
 		ret = ad9467_spi_write(st->spi, AN877_ADC_REG_VREF,
-				       info->scale_table[i][1]);
+				       st->info->scale_table[i][1]);
 		if (ret < 0)
 			return ret;
 
@@ -326,15 +318,15 @@ static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
 	return -EINVAL;
 }
 
-static int ad9467_read_raw(struct adi_axi_adc_conv *conv,
+static int ad9467_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long m)
 {
-	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
+	struct ad9467_state *st = iio_priv(indio_dev);
 
 	switch (m) {
 	case IIO_CHAN_INFO_SCALE:
-		return ad9467_get_scale(conv, val, val2);
+		return ad9467_get_scale(st, val, val2);
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		*val = clk_get_rate(st->clk);
 
@@ -344,20 +336,19 @@ static int ad9467_read_raw(struct adi_axi_adc_conv *conv,
 	}
 }
 
-static int ad9467_write_raw(struct adi_axi_adc_conv *conv,
+static int ad9467_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int val, int val2, long mask)
 {
-	const struct adi_axi_adc_chip_info *info = conv->chip_info;
-	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
+	struct ad9467_state *st = iio_priv(indio_dev);
 	long r_clk;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		return ad9467_set_scale(conv, val, val2);
+		return ad9467_set_scale(st, val, val2);
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		r_clk = clk_round_rate(st->clk, val);
-		if (r_clk < 0 || r_clk > info->max_rate) {
+		if (r_clk < 0 || r_clk > st->info->max_rate) {
 			dev_warn(&st->spi->dev,
 				 "Error setting ADC sample rate %ld", r_clk);
 			return -EINVAL;
@@ -369,26 +360,53 @@ static int ad9467_write_raw(struct adi_axi_adc_conv *conv,
 	}
 }
 
-static int ad9467_read_avail(struct adi_axi_adc_conv *conv,
+static int ad9467_read_avail(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     const int **vals, int *type, int *length,
 			     long mask)
 {
-	const struct adi_axi_adc_chip_info *info = conv->chip_info;
-	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
+	struct ad9467_state *st = iio_priv(indio_dev);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
 		*vals = (const int *)st->scales;
 		*type = IIO_VAL_INT_PLUS_MICRO;
 		/* Values are stored in a 2D matrix */
-		*length = info->num_scales * 2;
+		*length = st->info->num_scales * 2;
 		return IIO_AVAIL_LIST;
 	default:
 		return -EINVAL;
 	}
 }
 
+static int ad9467_update_scan_mode(struct iio_dev *indio_dev,
+				   const unsigned long *scan_mask)
+{
+	struct ad9467_state *st = iio_priv(indio_dev);
+	unsigned int c;
+	int ret;
+
+	for (c = 0; c < st->info->num_channels; c++) {
+		if (test_bit(c, scan_mask))
+			ret = iio_backend_chan_enable(st->back, c);
+		else
+			ret = iio_backend_chan_disable(st->back, c);
+
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct iio_info ad9467_info = {
+	.read_raw = ad9467_read_raw,
+	.write_raw = ad9467_write_raw,
+	.update_scan_mode = ad9467_update_scan_mode,
+	.debugfs_reg_access = ad9467_reg_access,
+	.read_avail = ad9467_read_avail,
+};
+
 static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
 {
 	int ret;
@@ -401,19 +419,17 @@ static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
 				AN877_ADC_TRANSFER_SYNC);
 }
 
-static int ad9467_scale_fill(struct adi_axi_adc_conv *conv)
+static int ad9467_scale_fill(struct ad9467_state *st)
 {
-	const struct adi_axi_adc_chip_info *info = conv->chip_info;
-	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
 	unsigned int i, val1, val2;
 
-	st->scales = devm_kmalloc_array(&st->spi->dev, info->num_scales,
+	st->scales = devm_kmalloc_array(&st->spi->dev, st->info->num_scales,
 					sizeof(*st->scales), GFP_KERNEL);
 	if (!st->scales)
 		return -ENOMEM;
 
-	for (i = 0; i < info->num_scales; i++) {
-		__ad9467_get_scale(conv, i, &val1, &val2);
+	for (i = 0; i < st->info->num_scales; i++) {
+		__ad9467_get_scale(st, i, &val1, &val2);
 		st->scales[i][0] = val1;
 		st->scales[i][1] = val2;
 	}
@@ -421,11 +437,27 @@ static int ad9467_scale_fill(struct adi_axi_adc_conv *conv)
 	return 0;
 }
 
-static int ad9467_preenable_setup(struct adi_axi_adc_conv *conv)
+static int ad9467_setup(struct ad9467_state *st)
 {
-	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
+	struct iio_backend_data_fmt data = {
+		.sign_extend = true,
+		.enable = true,
+	};
+	unsigned int c, mode;
+	int ret;
+
+	mode = st->info->default_output_mode | AN877_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
+	ret = ad9467_outputmode_set(st->spi, mode);
+	if (ret)
+		return ret;
+
+	for (c = 0; c < st->info->num_channels; c++) {
+		ret = iio_backend_data_format_set(st->back, c, &data);
+		if (ret)
+			return ret;
+	}
 
-	return ad9467_outputmode_set(st->spi, st->output_mode);
+	return 0;
 }
 
 static int ad9467_reset(struct device *dev)
@@ -445,23 +477,22 @@ static int ad9467_reset(struct device *dev)
 
 static int ad9467_probe(struct spi_device *spi)
 {
-	const struct ad9467_chip_info *info;
-	struct adi_axi_adc_conv *conv;
+	struct iio_dev *indio_dev;
 	struct ad9467_state *st;
 	unsigned int id;
 	int ret;
 
-	info = spi_get_device_match_data(spi);
-	if (!info)
-		return -ENODEV;
-
-	conv = devm_adi_axi_adc_conv_register(&spi->dev, sizeof(*st));
-	if (IS_ERR(conv))
-		return PTR_ERR(conv);
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
 
-	st = adi_axi_adc_conv_priv(conv);
+	st = iio_priv(indio_dev);
 	st->spi = spi;
 
+	st->info = spi_get_device_match_data(spi);
+	if (!st->info)
+		return -ENODEV;
+
 	st->clk = devm_clk_get_enabled(&spi->dev, "adc-clk");
 	if (IS_ERR(st->clk))
 		return PTR_ERR(st->clk);
@@ -475,29 +506,39 @@ static int ad9467_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	conv->chip_info = &info->axi_adc_info;
-
-	ret = ad9467_scale_fill(conv);
+	ret = ad9467_scale_fill(st);
 	if (ret)
 		return ret;
 
 	id = ad9467_spi_read(spi, AN877_ADC_REG_CHIP_ID);
-	if (id != conv->chip_info->id) {
+	if (id != st->info->id) {
 		dev_err(&spi->dev, "Mismatch CHIP_ID, got 0x%X, expected 0x%X\n",
-			id, conv->chip_info->id);
+			id, st->info->id);
 		return -ENODEV;
 	}
 
-	conv->reg_access = ad9467_reg_access;
-	conv->write_raw = ad9467_write_raw;
-	conv->read_raw = ad9467_read_raw;
-	conv->read_avail = ad9467_read_avail;
-	conv->preenable_setup = ad9467_preenable_setup;
+	indio_dev->name = st->info->name;
+	indio_dev->channels = st->info->channels;
+	indio_dev->num_channels = st->info->num_channels;
+	indio_dev->info = &ad9467_info;
 
-	st->output_mode = info->default_output_mode |
-			  AN877_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
+	st->back = devm_iio_backend_get(&spi->dev, NULL);
+	if (IS_ERR(st->back))
+		return PTR_ERR(st->back);
 
-	return 0;
+	ret = devm_iio_backend_request_buffer(&spi->dev, st->back, indio_dev);
+	if (ret)
+		return ret;
+
+	ret = iio_backend_enable(st->back);
+	if (ret)
+		return ret;
+
+	ret = ad9467_setup(st);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct of_device_id ad9467_of_match[] = {
@@ -529,4 +570,4 @@ module_spi_driver(ad9467_driver);
 MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
 MODULE_DESCRIPTION("Analog Devices AD9467 ADC driver");
 MODULE_LICENSE("GPL v2");
-MODULE_IMPORT_NS(IIO_ADI_AXI);
+MODULE_IMPORT_NS(IIO_BACKEND);

-- 
2.43.0

[PATCH v3 8/8] iio: adc: adi-axi-adc: move to backend framework

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Move to the IIO backend framework. Devices supported by adi-axi-adc now
register themselves as backend devices.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/Kconfig             |   2 +-
 drivers/iio/adc/adi-axi-adc.c       | 380 +++++++++---------------------------
 include/linux/iio/adc/adi-axi-adc.h |  68 -------
 3 files changed, 93 insertions(+), 357 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 10e0e340cdae..b64e0d442a19 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -293,7 +293,7 @@ config ADI_AXI_ADC
 	select IIO_BUFFER_HW_CONSUMER
 	select IIO_BUFFER_DMAENGINE
 	select REGMAP_MMIO
-	depends on OF
+	select IIO_BACKEND
 	help
 	  Say yes here to build support for Analog Devices Generic
 	  AXI ADC IP core. The IP core is used for interfacing with
diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 0f21d1d98b9f..741b53c25bb1 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -8,6 +8,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/clk.h>
+#include <linux/err.h>
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/module.h>
@@ -17,13 +18,11 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 
-#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>
+#include <linux/iio/backend.h>
+#include <linux/iio/buffer-dmaengine.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
 
 /*
  * Register definitions:
@@ -44,6 +43,7 @@
 #define   ADI_AXI_REG_CHAN_CTRL_PN_SEL_OWR	BIT(10)
 #define   ADI_AXI_REG_CHAN_CTRL_IQCOR_EN	BIT(9)
 #define   ADI_AXI_REG_CHAN_CTRL_DCFILT_EN	BIT(8)
+#define   ADI_AXI_REG_CHAN_CTRL_FMT_MASK	GENMASK(6, 4)
 #define   ADI_AXI_REG_CHAN_CTRL_FMT_SIGNEXT	BIT(6)
 #define   ADI_AXI_REG_CHAN_CTRL_FMT_TYPE	BIT(5)
 #define   ADI_AXI_REG_CHAN_CTRL_FMT_EN		BIT(4)
@@ -55,286 +55,100 @@
 	 ADI_AXI_REG_CHAN_CTRL_FMT_EN |		\
 	 ADI_AXI_REG_CHAN_CTRL_ENABLE)
 
-struct adi_axi_adc_core_info {
-	unsigned int				version;
-};
-
 struct adi_axi_adc_state {
-	struct mutex				lock;
-
-	struct adi_axi_adc_client		*client;
 	struct regmap				*regmap;
-};
-
-struct adi_axi_adc_client {
-	struct list_head			entry;
-	struct adi_axi_adc_conv			conv;
-	struct adi_axi_adc_state		*state;
 	struct device				*dev;
-	const struct adi_axi_adc_core_info	*info;
 };
 
-static LIST_HEAD(registered_clients);
-static DEFINE_MUTEX(registered_clients_lock);
-
-static struct adi_axi_adc_client *conv_to_client(struct adi_axi_adc_conv *conv)
-{
-	return container_of(conv, struct adi_axi_adc_client, conv);
-}
-
-void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv)
-{
-	struct adi_axi_adc_client *cl = conv_to_client(conv);
-
-	return (char *)cl + ALIGN(sizeof(struct adi_axi_adc_client),
-				  IIO_DMA_MINALIGN);
-}
-EXPORT_SYMBOL_NS_GPL(adi_axi_adc_conv_priv, IIO_ADI_AXI);
-
-static int adi_axi_adc_config_dma_buffer(struct device *dev,
-					 struct iio_dev *indio_dev)
-{
-	const char *dma_name;
-
-	if (!device_property_present(dev, "dmas"))
-		return 0;
-
-	if (device_property_read_string(dev, "dma-names", &dma_name))
-		dma_name = "rx";
-
-	return devm_iio_dmaengine_buffer_setup(indio_dev->dev.parent,
-					       indio_dev, dma_name);
-}
-
-static int adi_axi_adc_read_raw(struct iio_dev *indio_dev,
-				struct iio_chan_spec const *chan,
-				int *val, int *val2, long mask)
-{
-	struct adi_axi_adc_state *st = iio_priv(indio_dev);
-	struct adi_axi_adc_conv *conv = &st->client->conv;
-
-	if (!conv->read_raw)
-		return -EOPNOTSUPP;
-
-	return conv->read_raw(conv, chan, val, val2, mask);
-}
-
-static int adi_axi_adc_write_raw(struct iio_dev *indio_dev,
-				 struct iio_chan_spec const *chan,
-				 int val, int val2, long mask)
-{
-	struct adi_axi_adc_state *st = iio_priv(indio_dev);
-	struct adi_axi_adc_conv *conv = &st->client->conv;
-
-	if (!conv->write_raw)
-		return -EOPNOTSUPP;
-
-	return conv->write_raw(conv, chan, val, val2, mask);
-}
-
-static int adi_axi_adc_read_avail(struct iio_dev *indio_dev,
-				  struct iio_chan_spec const *chan,
-				  const int **vals, int *type, int *length,
-				  long mask)
+static int axi_adc_enable(struct iio_backend *back)
 {
-	struct adi_axi_adc_state *st = iio_priv(indio_dev);
-	struct adi_axi_adc_conv *conv = &st->client->conv;
-
-	if (!conv->read_avail)
-		return -EOPNOTSUPP;
-
-	return conv->read_avail(conv, chan, vals, type, length, mask);
-}
-
-static int adi_axi_adc_update_scan_mode(struct iio_dev *indio_dev,
-					const unsigned long *scan_mask)
-{
-	struct adi_axi_adc_state *st = iio_priv(indio_dev);
-	struct adi_axi_adc_conv *conv = &st->client->conv;
-	unsigned int i;
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
 	int ret;
 
-	for (i = 0; i < conv->chip_info->num_channels; i++) {
-		if (test_bit(i, scan_mask))
-			ret = regmap_set_bits(st->regmap,
-					      ADI_AXI_REG_CHAN_CTRL(i),
-					      ADI_AXI_REG_CHAN_CTRL_ENABLE);
-		else
-			ret = regmap_clear_bits(st->regmap,
-						ADI_AXI_REG_CHAN_CTRL(i),
-						ADI_AXI_REG_CHAN_CTRL_ENABLE);
-		if (ret)
-			return ret;
-	}
+	ret = regmap_set_bits(st->regmap, ADI_AXI_REG_RSTN,
+			      ADI_AXI_REG_RSTN_MMCM_RSTN);
+	if (ret)
+		return ret;
 
-	return 0;
+	fsleep(10);
+	return regmap_set_bits(st->regmap, ADI_AXI_REG_RSTN,
+			       ADI_AXI_REG_RSTN_RSTN | ADI_AXI_REG_RSTN_MMCM_RSTN);
 }
 
-static struct adi_axi_adc_conv *adi_axi_adc_conv_register(struct device *dev,
-							  size_t sizeof_priv)
+static void axi_adc_disable(struct iio_backend *back)
 {
-	struct adi_axi_adc_client *cl;
-	size_t alloc_size;
-
-	alloc_size = ALIGN(sizeof(struct adi_axi_adc_client), IIO_DMA_MINALIGN);
-	if (sizeof_priv)
-		alloc_size += ALIGN(sizeof_priv, IIO_DMA_MINALIGN);
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
 
-	cl = kzalloc(alloc_size, GFP_KERNEL);
-	if (!cl)
-		return ERR_PTR(-ENOMEM);
-
-	mutex_lock(&registered_clients_lock);
-
-	cl->dev = get_device(dev);
-
-	list_add_tail(&cl->entry, &registered_clients);
-
-	mutex_unlock(&registered_clients_lock);
-
-	return &cl->conv;
+	regmap_write(st->regmap, ADI_AXI_REG_RSTN, 0);
 }
 
-static void adi_axi_adc_conv_unregister(struct adi_axi_adc_conv *conv)
+static int axi_adc_data_format_set(struct iio_backend *back, unsigned int chan,
+				   const struct iio_backend_data_fmt *data)
 {
-	struct adi_axi_adc_client *cl = conv_to_client(conv);
-
-	mutex_lock(&registered_clients_lock);
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+	u32 val;
 
-	list_del(&cl->entry);
-	put_device(cl->dev);
+	if (!data->enable)
+		return regmap_clear_bits(st->regmap,
+					 ADI_AXI_REG_CHAN_CTRL(chan),
+					 ADI_AXI_REG_CHAN_CTRL_FMT_EN);
 
-	mutex_unlock(&registered_clients_lock);
+	val = FIELD_PREP(ADI_AXI_REG_CHAN_CTRL_FMT_EN, true);
+	if (data->sign_extend)
+		val |= FIELD_PREP(ADI_AXI_REG_CHAN_CTRL_FMT_SIGNEXT, true);
+	if (data->type == IIO_BACKEND_OFFSET_BINARY)
+		val |= FIELD_PREP(ADI_AXI_REG_CHAN_CTRL_FMT_TYPE, true);
 
-	kfree(cl);
+	return regmap_update_bits(st->regmap, ADI_AXI_REG_CHAN_CTRL(chan),
+				  ADI_AXI_REG_CHAN_CTRL_FMT_MASK, val);
 }
 
-static void devm_adi_axi_adc_conv_release(void *conv)
+static int axi_adc_chan_enable(struct iio_backend *back, unsigned int chan)
 {
-	adi_axi_adc_conv_unregister(conv);
-}
-
-struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
-							size_t sizeof_priv)
-{
-	struct adi_axi_adc_conv *conv;
-	int ret;
-
-	conv = adi_axi_adc_conv_register(dev, sizeof_priv);
-	if (IS_ERR(conv))
-		return conv;
-
-	ret = devm_add_action_or_reset(dev, devm_adi_axi_adc_conv_release,
-				       conv);
-	if (ret)
-		return ERR_PTR(ret);
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
 
-	return conv;
+	return regmap_set_bits(st->regmap, ADI_AXI_REG_CHAN_CTRL(chan),
+			       ADI_AXI_REG_CHAN_CTRL_ENABLE);
 }
-EXPORT_SYMBOL_NS_GPL(devm_adi_axi_adc_conv_register, IIO_ADI_AXI);
-
-static const struct iio_info adi_axi_adc_info = {
-	.read_raw = &adi_axi_adc_read_raw,
-	.write_raw = &adi_axi_adc_write_raw,
-	.update_scan_mode = &adi_axi_adc_update_scan_mode,
-	.read_avail = &adi_axi_adc_read_avail,
-};
 
-static const struct adi_axi_adc_core_info adi_axi_adc_10_0_a_info = {
-	.version = ADI_AXI_PCORE_VER(10, 0, 'a'),
-};
-
-static struct adi_axi_adc_client *adi_axi_adc_attach_client(struct device *dev)
+static int axi_adc_chan_disable(struct iio_backend *back, unsigned int chan)
 {
-	const struct adi_axi_adc_core_info *info;
-	struct adi_axi_adc_client *cl;
-	struct device_node *cln;
-
-	info = of_device_get_match_data(dev);
-	if (!info)
-		return ERR_PTR(-ENODEV);
-
-	cln = of_parse_phandle(dev->of_node, "adi,adc-dev", 0);
-	if (!cln) {
-		dev_err(dev, "No 'adi,adc-dev' node defined\n");
-		return ERR_PTR(-ENODEV);
-	}
-
-	mutex_lock(&registered_clients_lock);
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
 
-	list_for_each_entry(cl, &registered_clients, entry) {
-		if (!cl->dev)
-			continue;
-
-		if (cl->dev->of_node != cln)
-			continue;
-
-		if (!try_module_get(cl->dev->driver->owner)) {
-			mutex_unlock(&registered_clients_lock);
-			of_node_put(cln);
-			return ERR_PTR(-ENODEV);
-		}
-
-		get_device(cl->dev);
-		cl->info = info;
-		mutex_unlock(&registered_clients_lock);
-		of_node_put(cln);
-		return cl;
-	}
-
-	mutex_unlock(&registered_clients_lock);
-	of_node_put(cln);
-
-	return ERR_PTR(-EPROBE_DEFER);
+	return regmap_clear_bits(st->regmap, ADI_AXI_REG_CHAN_CTRL(chan),
+				 ADI_AXI_REG_CHAN_CTRL_ENABLE);
 }
 
-static int adi_axi_adc_setup_channels(struct device *dev,
-				      struct adi_axi_adc_state *st)
+static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
+						 struct iio_dev *indio_dev)
 {
-	struct adi_axi_adc_conv *conv = &st->client->conv;
-	int i, ret;
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+	struct iio_buffer *buffer;
+	const char *dma_name;
+	int ret;
 
-	if (conv->preenable_setup) {
-		ret = conv->preenable_setup(conv);
-		if (ret)
-			return ret;
-	}
+	if (device_property_read_string(st->dev, "dma-names", &dma_name))
+		dma_name = "rx";
 
-	for (i = 0; i < conv->chip_info->num_channels; i++) {
-		ret = regmap_write(st->regmap, ADI_AXI_REG_CHAN_CTRL(i),
-				   ADI_AXI_REG_CHAN_CTRL_DEFAULTS);
-		if (ret)
-			return ret;
+	buffer = iio_dmaengine_buffer_alloc(st->dev, dma_name);
+	if (IS_ERR(buffer)) {
+		dev_err(st->dev, "Could not get DMA buffer, %ld\n",
+			PTR_ERR(buffer));
+		return ERR_CAST(buffer);
 	}
 
-	return 0;
-}
-
-static int axi_adc_reset(struct adi_axi_adc_state *st)
-{
-	int ret;
-
-	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN, 0);
-	if (ret)
-		return ret;
-
-	mdelay(10);
-	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN,
-			   ADI_AXI_REG_RSTN_MMCM_RSTN);
+	indio_dev->modes |= INDIO_BUFFER_HARDWARE;
+	ret = iio_device_attach_buffer(indio_dev, buffer);
 	if (ret)
-		return ret;
+		return ERR_PTR(ret);
 
-	mdelay(10);
-	return regmap_write(st->regmap, ADI_AXI_REG_RSTN,
-			    ADI_AXI_REG_RSTN_RSTN | ADI_AXI_REG_RSTN_MMCM_RSTN);
+	return buffer;
 }
 
-static void adi_axi_adc_cleanup(void *data)
+static void axi_adc_free_buffer(struct iio_backend *back,
+				struct iio_buffer *buffer)
 {
-	struct adi_axi_adc_client *cl = data;
-
-	put_device(cl->dev);
-	module_put(cl->dev->driver->owner);
+	iio_dmaengine_buffer_free(buffer);
 }
 
 static const struct regmap_config axi_adc_regmap_config = {
@@ -344,45 +158,46 @@ static const struct regmap_config axi_adc_regmap_config = {
 	.max_register = 0x0800,
 };
 
+static const struct iio_backend_ops adi_axi_adc_generic = {
+	.enable = axi_adc_enable,
+	.disable = axi_adc_disable,
+	.data_format_set = axi_adc_data_format_set,
+	.chan_enable = axi_adc_chan_enable,
+	.chan_disable = axi_adc_chan_disable,
+	.request_buffer = axi_adc_request_buffer,
+	.free_buffer = axi_adc_free_buffer,
+};
+
 static int adi_axi_adc_probe(struct platform_device *pdev)
 {
-	struct adi_axi_adc_conv *conv;
-	struct iio_dev *indio_dev;
-	struct adi_axi_adc_client *cl;
 	struct adi_axi_adc_state *st;
 	void __iomem *base;
-	unsigned int ver;
+	unsigned int ver, *expected_ver;
 	int ret;
 
-	cl = adi_axi_adc_attach_client(&pdev->dev);
-	if (IS_ERR(cl))
-		return PTR_ERR(cl);
-
-	ret = devm_add_action_or_reset(&pdev->dev, adi_axi_adc_cleanup, cl);
-	if (ret)
-		return ret;
-
-	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*st));
-	if (indio_dev == NULL)
+	st = devm_kzalloc(&pdev->dev, sizeof(*st), GFP_KERNEL);
+	if (!st)
 		return -ENOMEM;
 
-	st = iio_priv(indio_dev);
-	st->client = cl;
-	cl->state = st;
-	mutex_init(&st->lock);
-
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	st->dev = &pdev->dev;
 	st->regmap = devm_regmap_init_mmio(&pdev->dev, base,
 					   &axi_adc_regmap_config);
 	if (IS_ERR(st->regmap))
 		return PTR_ERR(st->regmap);
 
-	conv = &st->client->conv;
+	expected_ver = (unsigned int *)device_get_match_data(&pdev->dev);
+	if (!expected_ver)
+		return -ENODEV;
 
-	ret = axi_adc_reset(st);
+	/*
+	 * Force disable the core. Up to the frontend to enable us. And we can
+	 * still read/write registers...
+	 */
+	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN, 0);
 	if (ret)
 		return ret;
 
@@ -390,37 +205,23 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	if (cl->info->version > ver) {
+	if (*expected_ver > ver) {
 		dev_err(&pdev->dev,
 			"IP core version is too old. Expected %d.%.2d.%c, Reported %d.%.2d.%c\n",
-			ADI_AXI_PCORE_VER_MAJOR(cl->info->version),
-			ADI_AXI_PCORE_VER_MINOR(cl->info->version),
-			ADI_AXI_PCORE_VER_PATCH(cl->info->version),
+			ADI_AXI_PCORE_VER_MAJOR(*expected_ver),
+			ADI_AXI_PCORE_VER_MINOR(*expected_ver),
+			ADI_AXI_PCORE_VER_PATCH(*expected_ver),
 			ADI_AXI_PCORE_VER_MAJOR(ver),
 			ADI_AXI_PCORE_VER_MINOR(ver),
 			ADI_AXI_PCORE_VER_PATCH(ver));
 		return -ENODEV;
 	}
 
-	indio_dev->info = &adi_axi_adc_info;
-	indio_dev->name = "adi-axi-adc";
-	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->num_channels = conv->chip_info->num_channels;
-	indio_dev->channels = conv->chip_info->channels;
-
-	ret = adi_axi_adc_config_dma_buffer(&pdev->dev, indio_dev);
+	ret = devm_iio_backend_register(&pdev->dev, &adi_axi_adc_generic, st);
 	if (ret)
 		return ret;
 
-	ret = adi_axi_adc_setup_channels(&pdev->dev, st);
-	if (ret)
-		return ret;
-
-	ret = devm_iio_device_register(&pdev->dev, indio_dev);
-	if (ret)
-		return ret;
-
-	dev_info(&pdev->dev, "AXI ADC IP core (%d.%.2d.%c) probed\n",
+	dev_info(&pdev->dev, "AXI ADC IP core (%d.%.2d.%d) probed\n",
 		 ADI_AXI_PCORE_VER_MAJOR(ver),
 		 ADI_AXI_PCORE_VER_MINOR(ver),
 		 ADI_AXI_PCORE_VER_PATCH(ver));
@@ -428,6 +229,8 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static unsigned int adi_axi_adc_10_0_a_info = ADI_AXI_PCORE_VER(10, 0, 'a');
+
 /* Match table for of_platform binding */
 static const struct of_device_id adi_axi_adc_of_match[] = {
 	{ .compatible = "adi,axi-adc-10.0.a", .data = &adi_axi_adc_10_0_a_info },
@@ -448,3 +251,4 @@ MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
 MODULE_DESCRIPTION("Analog Devices Generic AXI ADC IP core driver");
 MODULE_LICENSE("GPL v2");
 MODULE_IMPORT_NS(IIO_DMAENGINE_BUFFER);
+MODULE_IMPORT_NS(IIO_BACKEND);
diff --git a/include/linux/iio/adc/adi-axi-adc.h b/include/linux/iio/adc/adi-axi-adc.h
deleted file mode 100644
index b7904992d561..000000000000
--- a/include/linux/iio/adc/adi-axi-adc.h
+++ /dev/null
@@ -1,68 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Analog Devices Generic AXI ADC IP core driver/library
- * Link: https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
- *
- * Copyright 2012-2020 Analog Devices Inc.
- */
-#ifndef __ADI_AXI_ADC_H__
-#define __ADI_AXI_ADC_H__
-
-struct device;
-struct iio_chan_spec;
-
-/**
- * struct adi_axi_adc_chip_info - Chip specific information
- * @name		Chip name
- * @id			Chip ID (usually product ID)
- * @channels		Channel specifications of type @struct iio_chan_spec
- * @num_channels	Number of @channels
- * @scale_table		Supported scales by the chip; tuples of 2 ints
- * @num_scales		Number of scales in the table
- * @max_rate		Maximum sampling rate supported by the device
- */
-struct adi_axi_adc_chip_info {
-	const char			*name;
-	unsigned int			id;
-
-	const struct iio_chan_spec	*channels;
-	unsigned int			num_channels;
-
-	const unsigned int		(*scale_table)[2];
-	int				num_scales;
-
-	unsigned long			max_rate;
-};
-
-/**
- * struct adi_axi_adc_conv - data of the ADC attached to the AXI ADC
- * @chip_info		chip info details for the client ADC
- * @preenable_setup	op to run in the client before enabling the AXI ADC
- * @reg_access		IIO debugfs_reg_access hook for the client ADC
- * @read_raw		IIO read_raw hook for the client ADC
- * @write_raw		IIO write_raw hook for the client ADC
- * @read_avail		IIO read_avail hook for the client ADC
- */
-struct adi_axi_adc_conv {
-	const struct adi_axi_adc_chip_info		*chip_info;
-
-	int (*preenable_setup)(struct adi_axi_adc_conv *conv);
-	int (*reg_access)(struct adi_axi_adc_conv *conv, unsigned int reg,
-			  unsigned int writeval, unsigned int *readval);
-	int (*read_raw)(struct adi_axi_adc_conv *conv,
-			struct iio_chan_spec const *chan,
-			int *val, int *val2, long mask);
-	int (*write_raw)(struct adi_axi_adc_conv *conv,
-			 struct iio_chan_spec const *chan,
-			 int val, int val2, long mask);
-	int (*read_avail)(struct adi_axi_adc_conv *conv,
-			  struct iio_chan_spec const *chan,
-			  const int **val, int *type, int *length, long mask);
-};
-
-struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
-							size_t sizeof_priv);
-
-void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv);
-
-#endif

-- 
2.43.0

Re: [PATCH v3 0/8] iio: add new backend framework

[Rob Herring]

On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:
> v1:
>  https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f5175273b81dbfe40b7f0daffcdc67d6cb8ff
> 
> v2:
>  https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.com
> 
> Changes in v3:
> - Patch 1:
>  * Use proposed generic schema [1]. Also make it a required property;
>  * Improved the commit message.
> - Patch 2:
>  * Improved commit message.
> - Patch 4:
>  * Namespace all IIO DMAENGINE buffer exports;
>  * Removed unrelated new line removal change.
> - Patch 5:
>  * Namespace all IIO backend exports.
> - Patch 6:
>  * Set backend.h in alphabetical order;
>  * Import IIO backend namespace.
> - Patch 7:
>  * Don't depend on OF in kbuild anymore;
>  * Import IIO backend namespace.
> 
> For the bindings patches, I tried not to enter into much details about
> the IIO framework as I think specifics of the implementation don't care
> from the bindings perspective. Hopefully the commit messages are good
> enough.
> 
> I'm also aware that patch 1 is not backward compatible but we are
> anyways doing it on the driver side (and on the driver the property is
> indeed required). Anyways, just let me know if making the property
> required is not acceptable (I'm fairly confident no one was using the
> upstream version of the driver and so validating devicetrees for it). 
> 
> Keeping the block diagram in v3's cover so we don't have to follow links
> to check the one of the typicals setups. 
> 
>                                            -------------------------------------------------------
>  ------------------                        | -----------         ------------      -------  FPGA |
>  |     ADC        |------------------------| | AXI ADC |---------| DMA CORE |------| RAM |       |
>  | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------|          |------|     |       |
>  |                |------------------------| -----------         ------------      -------       |
>  ------------------                        -------------------------------------------------------

Why doesn't axi-adc just have an io-channels property to adc? It's the 
opposite direction for the link, but it seems more logical to me that 
axi-adc depends on adc rather than the other way around.

And if there's another consumer in the chain, then a node could 
certainly be both an io-channels consumer and producer.

The architecture of the drivers seems odd to me. It looks similar to 
making a phy driver handle all the state and protocol with the host 
controller being a backend.

Rob

Re: [PATCH v3 0/8] iio: add new backend framework

[Nuno Sá]

On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote:
> On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:
> > v1:
> >  
> > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517
> > 5273b81dbfe40b7f0daffcdc67d6cb8ff
> > 
> > v2:
> >  https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.com
> > 
> > Changes in v3:
> > - Patch 1:
> >  * Use proposed generic schema [1]. Also make it a required property;
> >  * Improved the commit message.
> > - Patch 2:
> >  * Improved commit message.
> > - Patch 4:
> >  * Namespace all IIO DMAENGINE buffer exports;
> >  * Removed unrelated new line removal change.
> > - Patch 5:
> >  * Namespace all IIO backend exports.
> > - Patch 6:
> >  * Set backend.h in alphabetical order;
> >  * Import IIO backend namespace.
> > - Patch 7:
> >  * Don't depend on OF in kbuild anymore;
> >  * Import IIO backend namespace.
> > 
> > For the bindings patches, I tried not to enter into much details about
> > the IIO framework as I think specifics of the implementation don't care
> > from the bindings perspective. Hopefully the commit messages are good
> > enough.
> > 
> > I'm also aware that patch 1 is not backward compatible but we are
> > anyways doing it on the driver side (and on the driver the property is
> > indeed required). Anyways, just let me know if making the property
> > required is not acceptable (I'm fairly confident no one was using the
> > upstream version of the driver and so validating devicetrees for it). 
> > 
> > Keeping the block diagram in v3's cover so we don't have to follow links
> > to check the one of the typicals setups. 
> > 
> >                                            --------------------------------------
> > -----------------
> >  ------------------                        | -----------         ------------
> >       -------  FPGA |
> >  |     ADC        |------------------------| | AXI ADC |---------| DMA CORE |----
> > --| RAM |       |
> >  | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------|          |----
> > --|     |       |
> >  |                |------------------------| -----------         ------------
> >       -------       |
> >  ------------------                        --------------------------------------
> > -----------------
> 
> Why doesn't axi-adc just have an io-channels property to adc? It's the 
> opposite direction for the link, but it seems more logical to me that 
> axi-adc depends on adc rather than the other way around.
> 

We are not interested on a single channel but on the complete device. From the axi-
adc point of view, there's not much we could do with the adc channel. I mean, maybe
we could still do something but it would be far from ideal (see below).

The opposite direction is exactly what we had (look at patch 2) just with another
custom property. The problem with that is that we would then need a bidirectional
link (we would need to callback into the provider and the provider would need to
access the consumer) between consumers and providers and that would be far from
optimal. The bidirectional link would exist because if we want to support fundamental
things like LVDS/CMOS interface tuning we need to set/get settings from the axi-adc
core. And as Jonathan suggested, the real data is captured or sent on the converters
(ADC or DACs) and that is why we have the IIO device/interface in there and why we
call them "frontends". In ADI usecases, backends are these FPGA cores providing
"services" to the "real" high speed converters. To put it in another way, the real
converter is the one who knows how to use the axi-adc core and not the other way
around. That's also one of the reasons why it would be way more difficult to handle
things with the opposite link. That's how we have done it so far and the mess we have
out of tree is massive :sweat_smile: We ended up doing raw writes and reads on the
axi-adc MMIO registers from the converter driver just because we had to configure or
get something from the axi-adc device but the link was backwards.

> And if there's another consumer in the chain, then a node could 
> certainly be both an io-channels consumer and producer.
> 

This should also be possible with this architecture. A node can be both a backend
(provider) and a consumer and we have an out of tree design that fits this (that I
surely want to upstream after the foundations are done).

> The architecture of the drivers seems odd to me. It looks similar to 
> making a phy driver handle all the state and protocol with the host 
> controller being a backend.

In this case, it's not really a controller. It's more like an extension of the device
because we need a way to handle the high sample rates this ADCs can do. Then we can
also do test tones with the backend which is useful for interface tuning (as
mentioned above).

To give you a bit more context, I'm adding the generic property because we will have
more users for it (from ADI - the next should be the axi-dac core) but STM is also
interested in this (likely the next user).

Hope this makes it a bit more clear...

- Nuno Sá

Re: [PATCH v3 0/8] iio: add new backend framework

[Rob Herring]

On Thu, Dec 14, 2023 at 05:05:20PM +0100, Nuno Sá wrote:
> On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote:
> > On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:
> > > v1:
> > >  
> > > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517
> > > 5273b81dbfe40b7f0daffcdc67d6cb8ff
> > > 
> > > v2:
> > >  https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.com
> > > 
> > > Changes in v3:
> > > - Patch 1:
> > >  * Use proposed generic schema [1]. Also make it a required property;
> > >  * Improved the commit message.
> > > - Patch 2:
> > >  * Improved commit message.
> > > - Patch 4:
> > >  * Namespace all IIO DMAENGINE buffer exports;
> > >  * Removed unrelated new line removal change.
> > > - Patch 5:
> > >  * Namespace all IIO backend exports.
> > > - Patch 6:
> > >  * Set backend.h in alphabetical order;
> > >  * Import IIO backend namespace.
> > > - Patch 7:
> > >  * Don't depend on OF in kbuild anymore;
> > >  * Import IIO backend namespace.
> > > 
> > > For the bindings patches, I tried not to enter into much details about
> > > the IIO framework as I think specifics of the implementation don't care
> > > from the bindings perspective. Hopefully the commit messages are good
> > > enough.
> > > 
> > > I'm also aware that patch 1 is not backward compatible but we are
> > > anyways doing it on the driver side (and on the driver the property is
> > > indeed required). Anyways, just let me know if making the property
> > > required is not acceptable (I'm fairly confident no one was using the
> > > upstream version of the driver and so validating devicetrees for it). 
> > > 
> > > Keeping the block diagram in v3's cover so we don't have to follow links
> > > to check the one of the typicals setups. 
> > > 
> > >                                            --------------------------------------
> > > -----------------
> > >  ------------------                        | -----------         ------------
> > >       -------  FPGA |
> > >  |     ADC        |------------------------| | AXI ADC |---------| DMA CORE |----
> > > --| RAM |       |
> > >  | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------|          |----
> > > --|     |       |
> > >  |                |------------------------| -----------         ------------
> > >       -------       |
> > >  ------------------                        --------------------------------------
> > > -----------------
> > 
> > Why doesn't axi-adc just have an io-channels property to adc? It's the 
> > opposite direction for the link, but it seems more logical to me that 
> > axi-adc depends on adc rather than the other way around.
> > 
> 
> We are not interested on a single channel but on the complete device. >From the axi-
> adc point of view, there's not much we could do with the adc channel. I mean, maybe
> we could still do something but it would be far from ideal (see below).

Will this hold up for everyone? Could there be a backend device that 
handles multiple ADCs? IOW, do you need #io-backend-cells? It's somewhat 
better if we add that up front rather than later and have to treat 
missing as 0 cells. It is also the only way to generically identify the 
providers (well, there's also 'foo-controller' properties, but we've 
gone away from those).


> The opposite direction is exactly what we had (look at patch 2) just with another
> custom property. The problem with that is that we would then need a bidirectional
> link (we would need to callback into the provider and the provider would need to
> access the consumer) between consumers and providers and that would be far from
> optimal. The bidirectional link would exist because if we want to support fundamental
> things like LVDS/CMOS interface tuning we need to set/get settings from the axi-adc
> core. And as Jonathan suggested, the real data is captured or sent on the converters
> (ADC or DACs) and that is why we have the IIO device/interface in there and why we
> call them "frontends". In ADI usecases, backends are these FPGA cores providing
> "services" to the "real" high speed converters. To put it in another way, the real
> converter is the one who knows how to use the axi-adc core and not the other way
> around. That's also one of the reasons why it would be way more difficult to handle
> things with the opposite link. That's how we have done it so far and the mess we have
> out of tree is massive :sweat_smile: We ended up doing raw writes and reads on the
> axi-adc MMIO registers from the converter driver just because we had to configure or
> get something from the axi-adc device but the link was backwards.

The direction (for the binding) doesn't really matter. It doesn't 
dictate the direction in the OS. In the ad9467 driver, you can search 
the DT for 'adi,adc-dev' and find the node which matches your node's 
phandle. It's not exactly efficient, but you are doing it once. It would 
also prevent the DT ABI break you are introducing.


> > And if there's another consumer in the chain, then a node could 
> > certainly be both an io-channels consumer and producer.
> > 
> 
> This should also be possible with this architecture. A node can be both a backend
> (provider) and a consumer and we have an out of tree design that fits this (that I
> surely want to upstream after the foundations are done).
> 
> > The architecture of the drivers seems odd to me. It looks similar to 
> > making a phy driver handle all the state and protocol with the host 
> > controller being a backend.
> 
> In this case, it's not really a controller. It's more like an extension of the device
> because we need a way to handle the high sample rates this ADCs can do. Then we can
> also do test tones with the backend which is useful for interface tuning (as
> mentioned above).
> 
> To give you a bit more context, I'm adding the generic property because we will have
> more users for it (from ADI - the next should be the axi-dac core) but STM is also
> interested in this (likely the next user).
> 
> Hope this makes it a bit more clear...

Yes, thanks.

I generally ask for 2 users on new common bindings. I've accepted too 
many only to have the 2nd user come in a month later and need additions. 
An ack on the binding from the STM folks would be nice here. And 
Jonathan too.

Rob

Re: [PATCH v3 0/8] iio: add new backend framework

[Nuno Sá]

On Thu, 2023-12-14 at 11:03 -0600, Rob Herring wrote:
> On Thu, Dec 14, 2023 at 05:05:20PM +0100, Nuno Sá wrote:
> > On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote:
> > > On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:
> > > > v1:
> > > >  
> > > > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517
> > > > 5273b81dbfe40b7f0daffcdc67d6cb8ff
> > > > 
> > > > v2:
> > > >  
> > > > https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.co
> > > > m
> > > > 
> > > > Changes in v3:
> > > > - Patch 1:
> > > >  * Use proposed generic schema [1]. Also make it a required property;
> > > >  * Improved the commit message.
> > > > - Patch 2:
> > > >  * Improved commit message.
> > > > - Patch 4:
> > > >  * Namespace all IIO DMAENGINE buffer exports;
> > > >  * Removed unrelated new line removal change.
> > > > - Patch 5:
> > > >  * Namespace all IIO backend exports.
> > > > - Patch 6:
> > > >  * Set backend.h in alphabetical order;
> > > >  * Import IIO backend namespace.
> > > > - Patch 7:
> > > >  * Don't depend on OF in kbuild anymore;
> > > >  * Import IIO backend namespace.
> > > > 
> > > > For the bindings patches, I tried not to enter into much details about
> > > > the IIO framework as I think specifics of the implementation don't care
> > > > from the bindings perspective. Hopefully the commit messages are good
> > > > enough.
> > > > 
> > > > I'm also aware that patch 1 is not backward compatible but we are
> > > > anyways doing it on the driver side (and on the driver the property is
> > > > indeed required). Anyways, just let me know if making the property
> > > > required is not acceptable (I'm fairly confident no one was using the
> > > > upstream version of the driver and so validating devicetrees for it). 
> > > > 
> > > > Keeping the block diagram in v3's cover so we don't have to follow links
> > > > to check the one of the typicals setups. 
> > > > 
> > > >                                            ----------------------------------
> > > > ----
> > > > -----------------
> > > >  ------------------                        | -----------         ------------
> > > >       -------  FPGA |
> > > >  |     ADC        |------------------------| | AXI ADC |---------| DMA CORE
> > > > |----
> > > > --| RAM |       |
> > > >  | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------|         
> > > > |----
> > > > --|     |       |
> > > >  |                |------------------------| -----------         ------------
> > > >       -------       |
> > > >  ------------------                        ----------------------------------
> > > > ----
> > > > -----------------
> > > 
> > > Why doesn't axi-adc just have an io-channels property to adc? It's the 
> > > opposite direction for the link, but it seems more logical to me that 
> > > axi-adc depends on adc rather than the other way around.
> > > 
> > 
> > We are not interested on a single channel but on the complete device. >From the
> > axi-
> > adc point of view, there's not much we could do with the adc channel. I mean,
> > maybe
> > we could still do something but it would be far from ideal (see below).
> 
> Will this hold up for everyone? Could there be a backend device that 
> handles multiple ADCs? IOW, do you need #io-backend-cells? It's somewhat 
> better if we add that up front rather than later and have to treat 
> missing as 0 cells. It is also the only way to generically identify the 
> providers (well, there's also 'foo-controller' properties, but we've 
> gone away from those).
> 

For the axi-adc backend, it's very unlikely. The way the core connects to the
converters is through a serial data interface (LVDS, CMOS or JESD in ADI usecases).
This interface is not really a bus so it's kind of a 1:1 connection. Now, in more
complicated devices (like highly integrated RF transceivers) what we have is that we
have multiple of these cores (one per RX/TX port) connected to the frontend. So,
effectively 1 frontend could have multiple backends. So, yes, your first "doubts" are
not that "crazy" as this is also not the "typical" provider - consumer relationship.
However, for all of what I've said in the previous email, even in these cases,
thinking in these cores as the provider, makes things much more easier to handle.

However, the above is just ADI usecases. In theory, yes, it can be very possible for
another backend other than axi-adc to have multiple frontends connected so I guess we
can make #io-backend-cells already available in the schema.

For the axi-adc bindings this would be 'const: 0', right?

> 
> > The opposite direction is exactly what we had (look at patch 2) just with another
> > custom property. The problem with that is that we would then need a bidirectional
> > link (we would need to callback into the provider and the provider would need to
> > access the consumer) between consumers and providers and that would be far from
> > optimal. The bidirectional link would exist because if we want to support
> > fundamental
> > things like LVDS/CMOS interface tuning we need to set/get settings from the axi-
> > adc
> > core. And as Jonathan suggested, the real data is captured or sent on the
> > converters
> > (ADC or DACs) and that is why we have the IIO device/interface in there and why
> > we
> > call them "frontends". In ADI usecases, backends are these FPGA cores providing
> > "services" to the "real" high speed converters. To put it in another way, the
> > real
> > converter is the one who knows how to use the axi-adc core and not the other way
> > around. That's also one of the reasons why it would be way more difficult to
> > handle
> > things with the opposite link. That's how we have done it so far and the mess we
> > have
> > out of tree is massive :sweat_smile: We ended up doing raw writes and reads on
> > the
> > axi-adc MMIO registers from the converter driver just because we had to configure
> > or
> > get something from the axi-adc device but the link was backwards.
> 
> The direction (for the binding) doesn't really matter. It doesn't 
> dictate the direction in the OS. In the ad9467 driver, you can search 
> the DT for 'adi,adc-dev' and find the node which matches your node's 
> phandle. It's not exactly efficient, but you are doing it once. It would 
> also prevent the DT ABI break you are introducing.
> 

Hmm, I think I see your idea. So you mean something like
devm_iio_backend_get_optional() and if not present, then we would look for nodes
having the 'adi,adc-dev' property and look for the one pointing at us... Then, we
would need another API in the backend to look for registered backends matching that
fwnode. Right?

I mean, I guess this could work but we would already have to start a fresh framework
with API's that are not really meant to be used anymore other than the ad9467 driver
(not devm_iio_backend_get_optional() because sooner or later I think we'll need that
one). We are already breaking ABI in the driver and I'm still fairly confident no one
is really using the upstream driver because it's lacking support for devices and
important features (not even in ADI fork we're using it).

Anyways, if you still insist on having something like this (and feel more comfortable
in not breaking DT ABI), I can see how this would look like in the next version...

> 
> > > And if there's another consumer in the chain, then a node could 
> > > certainly be both an io-channels consumer and producer.
> > > 
> > 
> > This should also be possible with this architecture. A node can be both a backend
> > (provider) and a consumer and we have an out of tree design that fits this (that
> > I
> > surely want to upstream after the foundations are done).
> > 
> > > The architecture of the drivers seems odd to me. It looks similar to 
> > > making a phy driver handle all the state and protocol with the host 
> > > controller being a backend.
> > 
> > In this case, it's not really a controller. It's more like an extension of the
> > device
> > because we need a way to handle the high sample rates this ADCs can do. Then we
> > can
> > also do test tones with the backend which is useful for interface tuning (as
> > mentioned above).
> > 
> > To give you a bit more context, I'm adding the generic property because we will
> > have
> > more users for it (from ADI - the next should be the axi-dac core) but STM is
> > also
> > interested in this (likely the next user).
> > 
> > Hope this makes it a bit more clear...
> 
> Yes, thanks.
> 
> I generally ask for 2 users on new common bindings. I've accepted too 
> many only to have the 2nd user come in a month later and need additions. 
> An ack on the binding from the STM folks would be nice here. And 
> Jonathan too.
> 

Olivier, could we get an ack on the bindings patch? Do you also have any idea about
how long it would take for you to send patches so we have another user of the schema?

On my side, it might very well take a month or so (given we have holidays nearby) as
the axi-dac core is more complex than the axi-adc. Bah it might take less than a
month to have the first version of it posted in the lists but I can't make any
promises.

- Nuno Sá

Re: [PATCH v3 0/8] iio: add new backend framework

[Jonathan Cameron]

On Fri, 15 Dec 2023 16:18:39 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, 2023-12-14 at 11:03 -0600, Rob Herring wrote:
> > On Thu, Dec 14, 2023 at 05:05:20PM +0100, Nuno Sá wrote:  
> > > On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote:  
> > > > On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:  
> > > > > v1:
> > > > >  
> > > > > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517
> > > > > 5273b81dbfe40b7f0daffcdc67d6cb8ff
> > > > > 
> > > > > v2:
> > > > >  
> > > > > https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.co
> > > > > m
> > > > > 
> > > > > Changes in v3:
> > > > > - Patch 1:
> > > > >  * Use proposed generic schema [1]. Also make it a required property;
> > > > >  * Improved the commit message.
> > > > > - Patch 2:
> > > > >  * Improved commit message.
> > > > > - Patch 4:
> > > > >  * Namespace all IIO DMAENGINE buffer exports;
> > > > >  * Removed unrelated new line removal change.
> > > > > - Patch 5:
> > > > >  * Namespace all IIO backend exports.
> > > > > - Patch 6:
> > > > >  * Set backend.h in alphabetical order;
> > > > >  * Import IIO backend namespace.
> > > > > - Patch 7:
> > > > >  * Don't depend on OF in kbuild anymore;
> > > > >  * Import IIO backend namespace.
> > > > > 
> > > > > For the bindings patches, I tried not to enter into much details about
> > > > > the IIO framework as I think specifics of the implementation don't care
> > > > > from the bindings perspective. Hopefully the commit messages are good
> > > > > enough.
> > > > > 
> > > > > I'm also aware that patch 1 is not backward compatible but we are
> > > > > anyways doing it on the driver side (and on the driver the property is
> > > > > indeed required). Anyways, just let me know if making the property
> > > > > required is not acceptable (I'm fairly confident no one was using the
> > > > > upstream version of the driver and so validating devicetrees for it). 
> > > > > 
> > > > > Keeping the block diagram in v3's cover so we don't have to follow links
> > > > > to check the one of the typicals setups. 
> > > > > 
> > > > >                                            ----------------------------------
> > > > > ----
> > > > > -----------------
> > > > >  ------------------                        | -----------         ------------
> > > > >       -------  FPGA |
> > > > >  |     ADC        |------------------------| | AXI ADC |---------| DMA CORE
> > > > > |----
> > > > > --| RAM |       |
> > > > >  | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------|         
> > > > > |----
> > > > > --|     |       |
> > > > >  |                |------------------------| -----------         ------------
> > > > >       -------       |
> > > > >  ------------------                        ----------------------------------
> > > > > ----
> > > > > -----------------  
> > > > 
> > > > Why doesn't axi-adc just have an io-channels property to adc? It's the 
> > > > opposite direction for the link, but it seems more logical to me that 
> > > > axi-adc depends on adc rather than the other way around.
> > > >   
> > > 
> > > We are not interested on a single channel but on the complete device. >From the
> > > axi-
> > > adc point of view, there's not much we could do with the adc channel. I mean,
> > > maybe
> > > we could still do something but it would be far from ideal (see below).  
> > 
> > Will this hold up for everyone? Could there be a backend device that 
> > handles multiple ADCs? IOW, do you need #io-backend-cells? It's somewhat 
> > better if we add that up front rather than later and have to treat 
> > missing as 0 cells. It is also the only way to generically identify the 
> > providers (well, there's also 'foo-controller' properties, but we've 
> > gone away from those).
> >   
> 
> For the axi-adc backend, it's very unlikely. The way the core connects to the
> converters is through a serial data interface (LVDS, CMOS or JESD in ADI usecases).
> This interface is not really a bus so it's kind of a 1:1 connection. Now, in more
> complicated devices (like highly integrated RF transceivers) what we have is that we
> have multiple of these cores (one per RX/TX port) connected to the frontend. So,
> effectively 1 frontend could have multiple backends. So, yes, your first "doubts" are
> not that "crazy" as this is also not the "typical" provider - consumer relationship.
> However, for all of what I've said in the previous email, even in these cases,
> thinking in these cores as the provider, makes things much more easier to handle.
> 
> However, the above is just ADI usecases. In theory, yes, it can be very possible for
> another backend other than axi-adc to have multiple frontends connected so I guess we
> can make #io-backend-cells already available in the schema.

I'd like ultimately to consider making this work for new instances of
dfsdm devices (separately front end ADC that spits out a modulated signal that a host
controller then turns into something useful). In those cases we might well see a mix
of front ends connected to one backend (at least in theory - not sure anyone would
build such thing outside of an eval board).

Adding the flexibility from the start would be sensible. So I agree with Rob here.

> 
> For the axi-adc bindings this would be 'const: 0', right?
> 
> >   
> > > The opposite direction is exactly what we had (look at patch 2) just with another
> > > custom property. The problem with that is that we would then need a bidirectional
> > > link (we would need to callback into the provider and the provider would need to
> > > access the consumer) between consumers and providers and that would be far from
> > > optimal. The bidirectional link would exist because if we want to support
> > > fundamental
> > > things like LVDS/CMOS interface tuning we need to set/get settings from the axi-
> > > adc
> > > core. And as Jonathan suggested, the real data is captured or sent on the
> > > converters
> > > (ADC or DACs) and that is why we have the IIO device/interface in there and why
> > > we
> > > call them "frontends". In ADI usecases, backends are these FPGA cores providing
> > > "services" to the "real" high speed converters. To put it in another way, the
> > > real
> > > converter is the one who knows how to use the axi-adc core and not the other way
> > > around. That's also one of the reasons why it would be way more difficult to
> > > handle
> > > things with the opposite link. That's how we have done it so far and the mess we
> > > have
> > > out of tree is massive :sweat_smile: We ended up doing raw writes and reads on
> > > the
> > > axi-adc MMIO registers from the converter driver just because we had to configure
> > > or
> > > get something from the axi-adc device but the link was backwards.  
> > 
> > The direction (for the binding) doesn't really matter. It doesn't 
> > dictate the direction in the OS. In the ad9467 driver, you can search 
> > the DT for 'adi,adc-dev' and find the node which matches your node's 
> > phandle. It's not exactly efficient, but you are doing it once. It would 
> > also prevent the DT ABI break you are introducing.
> >   
> 
> Hmm, I think I see your idea. So you mean something like
> devm_iio_backend_get_optional() and if not present, then we would look for nodes
> having the 'adi,adc-dev' property and look for the one pointing at us... Then, we
> would need another API in the backend to look for registered backends matching that
> fwnode. Right?
> 
> I mean, I guess this could work but we would already have to start a fresh framework
> with API's that are not really meant to be used anymore other than the ad9467 driver
> (not devm_iio_backend_get_optional() because sooner or later I think we'll need that
> one). We are already breaking ABI in the driver and I'm still fairly confident no one
> is really using the upstream driver because it's lacking support for devices and
> important features (not even in ADI fork we're using it).
> 
> Anyways, if you still insist on having something like this (and feel more comfortable
> in not breaking DT ABI), I can see how this would look like in the next version...

See how it looks. If it means removing the need to convince Rob then it's probably easier
to write the code than try and talk him around ;)  Can happily stick a bit deprecated
note next to the binding and the code.

> 
> >   
> > > > And if there's another consumer in the chain, then a node could 
> > > > certainly be both an io-channels consumer and producer.
> > > >   
> > > 
> > > This should also be possible with this architecture. A node can be both a backend
> > > (provider) and a consumer and we have an out of tree design that fits this (that
> > > I
> > > surely want to upstream after the foundations are done).
> > >   
> > > > The architecture of the drivers seems odd to me. It looks similar to 
> > > > making a phy driver handle all the state and protocol with the host 
> > > > controller being a backend.  
> > > 
> > > In this case, it's not really a controller. It's more like an extension of the
> > > device
> > > because we need a way to handle the high sample rates this ADCs can do. Then we
> > > can
> > > also do test tones with the backend which is useful for interface tuning (as
> > > mentioned above).
> > > 
> > > To give you a bit more context, I'm adding the generic property because we will
> > > have
> > > more users for it (from ADI - the next should be the axi-dac core) but STM is
> > > also
> > > interested in this (likely the next user).
> > > 
> > > Hope this makes it a bit more clear...  
> > 
> > Yes, thanks.
> > 
> > I generally ask for 2 users on new common bindings. I've accepted too 
> > many only to have the 2nd user come in a month later and need additions. 
> > An ack on the binding from the STM folks would be nice here. And 
> > Jonathan too.
> >   
> 
> Olivier, could we get an ack on the bindings patch? Do you also have any idea about
> how long it would take for you to send patches so we have another user of the schema?
> 
> On my side, it might very well take a month or so (given we have holidays nearby) as
> the axi-dac core is more complex than the axi-adc. Bah it might take less than a
> month to have the first version of it posted in the lists but I can't make any
> promises.

For the driver side of things I'd like at least 2, preferably 3 users before merging.
We have more flexibility to rework things as any issues will probably be internal
interfaces, but I'd rather wait if we are going to have 3 users within another month
or 2.

Jonathan
 
> 
> - Nuno Sá
> 

Re: [PATCH v3 0/8] iio: add new backend framework

[Nuno Sá]

On Sun, 2023-12-17 at 14:04 +0000, Jonathan Cameron wrote:
> On Fri, 15 Dec 2023 16:18:39 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Thu, 2023-12-14 at 11:03 -0600, Rob Herring wrote:
> > > On Thu, Dec 14, 2023 at 05:05:20PM +0100, Nuno Sá wrote:  
> > > > On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote:  
> > > > > On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:  
> > > > > > v1:
> > > > > >  
> > > > > > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517
> > > > > > 5273b81dbfe40b7f0daffcdc67d6cb8ff
> > > > > > 
> > > > > > v2:
> > > > > >  
> > > > > > https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.co
> > > > > > m
> > > > > > 
> > > > > > Changes in v3:
> > > > > > - Patch 1:
> > > > > >  * Use proposed generic schema [1]. Also make it a required
> > > > > > property;
> > > > > >  * Improved the commit message.
> > > > > > - Patch 2:
> > > > > >  * Improved commit message.
> > > > > > - Patch 4:
> > > > > >  * Namespace all IIO DMAENGINE buffer exports;
> > > > > >  * Removed unrelated new line removal change.
> > > > > > - Patch 5:
> > > > > >  * Namespace all IIO backend exports.
> > > > > > - Patch 6:
> > > > > >  * Set backend.h in alphabetical order;
> > > > > >  * Import IIO backend namespace.
> > > > > > - Patch 7:
> > > > > >  * Don't depend on OF in kbuild anymore;
> > > > > >  * Import IIO backend namespace.
> > > > > > 
> > > > > > For the bindings patches, I tried not to enter into much details
> > > > > > about
> > > > > > the IIO framework as I think specifics of the implementation don't
> > > > > > care
> > > > > > from the bindings perspective. Hopefully the commit messages are
> > > > > > good
> > > > > > enough.
> > > > > > 
> > > > > > I'm also aware that patch 1 is not backward compatible but we are
> > > > > > anyways doing it on the driver side (and on the driver the property
> > > > > > is
> > > > > > indeed required). Anyways, just let me know if making the property
> > > > > > required is not acceptable (I'm fairly confident no one was using
> > > > > > the
> > > > > > upstream version of the driver and so validating devicetrees for
> > > > > > it). 
> > > > > > 
> > > > > > Keeping the block diagram in v3's cover so we don't have to follow
> > > > > > links
> > > > > > to check the one of the typicals setups. 
> > > > > > 
> > > > > >                                            -------------------------
> > > > > > ---------
> > > > > > ----
> > > > > > -----------------
> > > > > >  ------------------                        | -----------         ---
> > > > > > ---------
> > > > > >       -------  FPGA |
> > > > > >  |     ADC        |------------------------| | AXI ADC |---------|
> > > > > > DMA CORE
> > > > > > > ----
> > > > > > --| RAM |       |
> > > > > >  | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------
> > > > > > |         
> > > > > > > ----
> > > > > > --|     |       |
> > > > > >  |                |------------------------| -----------         ---
> > > > > > ---------
> > > > > >       -------       |
> > > > > >  ------------------                        -------------------------
> > > > > > ---------
> > > > > > ----
> > > > > > -----------------  
> > > > > 
> > > > > Why doesn't axi-adc just have an io-channels property to adc? It's the
> > > > > opposite direction for the link, but it seems more logical to me that 
> > > > > axi-adc depends on adc rather than the other way around.
> > > > >   
> > > > 
> > > > We are not interested on a single channel but on the complete
> > > > device. >From the
> > > > axi-
> > > > adc point of view, there's not much we could do with the adc channel. I
> > > > mean,
> > > > maybe
> > > > we could still do something but it would be far from ideal (see below). 
> > > 
> > > Will this hold up for everyone? Could there be a backend device that 
> > > handles multiple ADCs? IOW, do you need #io-backend-cells? It's somewhat 
> > > better if we add that up front rather than later and have to treat 
> > > missing as 0 cells. It is also the only way to generically identify the 
> > > providers (well, there's also 'foo-controller' properties, but we've 
> > > gone away from those).
> > >   
> > 
> > For the axi-adc backend, it's very unlikely. The way the core connects to
> > the
> > converters is through a serial data interface (LVDS, CMOS or JESD in ADI
> > usecases).
> > This interface is not really a bus so it's kind of a 1:1 connection. Now, in
> > more
> > complicated devices (like highly integrated RF transceivers) what we have is
> > that we
> > have multiple of these cores (one per RX/TX port) connected to the frontend.
> > So,
> > effectively 1 frontend could have multiple backends. So, yes, your first
> > "doubts" are
> > not that "crazy" as this is also not the "typical" provider - consumer
> > relationship.
> > However, for all of what I've said in the previous email, even in these
> > cases,
> > thinking in these cores as the provider, makes things much more easier to
> > handle.
> > 
> > However, the above is just ADI usecases. In theory, yes, it can be very
> > possible for
> > another backend other than axi-adc to have multiple frontends connected so I
> > guess we
> > can make #io-backend-cells already available in the schema.
> 
> I'd like ultimately to consider making this work for new instances of
> dfsdm devices (separately front end ADC that spits out a modulated signal that
> a host
> controller then turns into something useful). In those cases we might well see
> a mix
> of front ends connected to one backend (at least in theory - not sure anyone
> would
> build such thing outside of an eval board).
> 
> Adding the flexibility from the start would be sensible. So I agree with Rob
> here.
> 

Agreed...

> > 
> > For the axi-adc bindings this would be 'const: 0', right?
> > 
> > >   
> > > > The opposite direction is exactly what we had (look at patch 2) just
> > > > with another
> > > > custom property. The problem with that is that we would then need a
> > > > bidirectional
> > > > link (we would need to callback into the provider and the provider would
> > > > need to
> > > > access the consumer) between consumers and providers and that would be
> > > > far from
> > > > optimal. The bidirectional link would exist because if we want to
> > > > support
> > > > fundamental
> > > > things like LVDS/CMOS interface tuning we need to set/get settings from
> > > > the axi-
> > > > adc
> > > > core. And as Jonathan suggested, the real data is captured or sent on
> > > > the
> > > > converters
> > > > (ADC or DACs) and that is why we have the IIO device/interface in there
> > > > and why
> > > > we
> > > > call them "frontends". In ADI usecases, backends are these FPGA cores
> > > > providing
> > > > "services" to the "real" high speed converters. To put it in another
> > > > way, the
> > > > real
> > > > converter is the one who knows how to use the axi-adc core and not the
> > > > other way
> > > > around. That's also one of the reasons why it would be way more
> > > > difficult to
> > > > handle
> > > > things with the opposite link. That's how we have done it so far and the
> > > > mess we
> > > > have
> > > > out of tree is massive :sweat_smile: We ended up doing raw writes and
> > > > reads on
> > > > the
> > > > axi-adc MMIO registers from the converter driver just because we had to
> > > > configure
> > > > or
> > > > get something from the axi-adc device but the link was backwards.  
> > > 
> > > The direction (for the binding) doesn't really matter. It doesn't 
> > > dictate the direction in the OS. In the ad9467 driver, you can search 
> > > the DT for 'adi,adc-dev' and find the node which matches your node's 
> > > phandle. It's not exactly efficient, but you are doing it once. It would 
> > > also prevent the DT ABI break you are introducing.
> > >   
> > 
> > Hmm, I think I see your idea. So you mean something like
> > devm_iio_backend_get_optional() and if not present, then we would look for
> > nodes
> > having the 'adi,adc-dev' property and look for the one pointing at us...
> > Then, we
> > would need another API in the backend to look for registered backends
> > matching that
> > fwnode. Right?
> > 
> > I mean, I guess this could work but we would already have to start a fresh
> > framework
> > with API's that are not really meant to be used anymore other than the
> > ad9467 driver
> > (not devm_iio_backend_get_optional() because sooner or later I think we'll
> > need that
> > one). We are already breaking ABI in the driver and I'm still fairly
> > confident no one
> > is really using the upstream driver because it's lacking support for devices
> > and
> > important features (not even in ADI fork we're using it).
> > 
> > Anyways, if you still insist on having something like this (and feel more
> > comfortable
> > in not breaking DT ABI), I can see how this would look like in the next
> > version...
> 
> See how it looks. If it means removing the need to convince Rob then it's
> probably easier
> to write the code than try and talk him around ;)  Can happily stick a bit
> deprecated
> note next to the binding and the code.
> 

Will do...

> > 
> > >   
> > > > > And if there's another consumer in the chain, then a node could 
> > > > > certainly be both an io-channels consumer and producer.
> > > > >   
> > > > 
> > > > This should also be possible with this architecture. A node can be both
> > > > a backend
> > > > (provider) and a consumer and we have an out of tree design that fits
> > > > this (that
> > > > I
> > > > surely want to upstream after the foundations are done).
> > > >   
> > > > > The architecture of the drivers seems odd to me. It looks similar to 
> > > > > making a phy driver handle all the state and protocol with the host 
> > > > > controller being a backend.  
> > > > 
> > > > In this case, it's not really a controller. It's more like an extension
> > > > of the
> > > > device
> > > > because we need a way to handle the high sample rates this ADCs can do.
> > > > Then we
> > > > can
> > > > also do test tones with the backend which is useful for interface tuning
> > > > (as
> > > > mentioned above).
> > > > 
> > > > To give you a bit more context, I'm adding the generic property because
> > > > we will
> > > > have
> > > > more users for it (from ADI - the next should be the axi-dac core) but
> > > > STM is
> > > > also
> > > > interested in this (likely the next user).
> > > > 
> > > > Hope this makes it a bit more clear...  
> > > 
> > > Yes, thanks.
> > > 
> > > I generally ask for 2 users on new common bindings. I've accepted too 
> > > many only to have the 2nd user come in a month later and need additions. 
> > > An ack on the binding from the STM folks would be nice here. And 
> > > Jonathan too.
> > >   
> > 
> > Olivier, could we get an ack on the bindings patch? Do you also have any
> > idea about
> > how long it would take for you to send patches so we have another user of
> > the schema?
> > 
> > On my side, it might very well take a month or so (given we have holidays
> > nearby) as
> > the axi-dac core is more complex than the axi-adc. Bah it might take less
> > than a
> > month to have the first version of it posted in the lists but I can't make
> > any
> > promises.
> 
> For the driver side of things I'd like at least 2, preferably 3 users before
> merging.
> We have more flexibility to rework things as any issues will probably be
> internal
> interfaces, but I'd rather wait if we are going to have 3 users within another
> month
> or 2.
> 

Totally fine by me. But how would this look like? Could we have an immutable
branch where we can send patches about this? Or maybe staging? I'm asking
because adding more stuff into these series might make it harder to review (the
axi-dac might have some fun ABI discussion :)). Ideally, we would have this
merged somewhere and then add users on top of it.

- Nuno Sá

Re: [PATCH v3 0/8] iio: add new backend framework

[Jonathan Cameron]

> > >   
> > > >     
> > > > > > And if there's another consumer in the chain, then a node could 
> > > > > > certainly be both an io-channels consumer and producer.
> > > > > >     
> > > > > 
> > > > > This should also be possible with this architecture. A node can be both
> > > > > a backend
> > > > > (provider) and a consumer and we have an out of tree design that fits
> > > > > this (that
> > > > > I
> > > > > surely want to upstream after the foundations are done).
> > > > >     
> > > > > > The architecture of the drivers seems odd to me. It looks similar to 
> > > > > > making a phy driver handle all the state and protocol with the host 
> > > > > > controller being a backend.    
> > > > > 
> > > > > In this case, it's not really a controller. It's more like an extension
> > > > > of the
> > > > > device
> > > > > because we need a way to handle the high sample rates this ADCs can do.
> > > > > Then we
> > > > > can
> > > > > also do test tones with the backend which is useful for interface tuning
> > > > > (as
> > > > > mentioned above).
> > > > > 
> > > > > To give you a bit more context, I'm adding the generic property because
> > > > > we will
> > > > > have
> > > > > more users for it (from ADI - the next should be the axi-dac core) but
> > > > > STM is
> > > > > also
> > > > > interested in this (likely the next user).
> > > > > 
> > > > > Hope this makes it a bit more clear...    
> > > > 
> > > > Yes, thanks.
> > > > 
> > > > I generally ask for 2 users on new common bindings. I've accepted too 
> > > > many only to have the 2nd user come in a month later and need additions. 
> > > > An ack on the binding from the STM folks would be nice here. And 
> > > > Jonathan too.
> > > >     
> > > 
> > > Olivier, could we get an ack on the bindings patch? Do you also have any
> > > idea about
> > > how long it would take for you to send patches so we have another user of
> > > the schema?
> > > 
> > > On my side, it might very well take a month or so (given we have holidays
> > > nearby) as
> > > the axi-dac core is more complex than the axi-adc. Bah it might take less
> > > than a
> > > month to have the first version of it posted in the lists but I can't make
> > > any
> > > promises.  
> > 
> > For the driver side of things I'd like at least 2, preferably 3 users before
> > merging.
> > We have more flexibility to rework things as any issues will probably be
> > internal
> > interfaces, but I'd rather wait if we are going to have 3 users within another
> > month
> > or 2.
> >   
> 
> Totally fine by me. But how would this look like? Could we have an immutable
> branch where we can send patches about this? Or maybe staging? I'm asking
> because adding more stuff into these series might make it harder to review (the
> axi-dac might have some fun ABI discussion :)). Ideally, we would have this
> merged somewhere and then add users on top of it.

It's fine to post a bunch of series with stated dependencies
(I've gotten 5 series + deep in the past :)

Obviously useful to have a git tree with them all on somewhere though
but if you host that it would be ideal given you are driving this
work in general.

Jonathan

> 
> - Nuno Sá

Re: [PATCH v3 0/8] iio: add new backend framework

[Rob Herring]

On Sun, Dec 17, 2023 at 02:04:12PM +0000, Jonathan Cameron wrote:
> On Fri, 15 Dec 2023 16:18:39 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Thu, 2023-12-14 at 11:03 -0600, Rob Herring wrote:
> > > On Thu, Dec 14, 2023 at 05:05:20PM +0100, Nuno Sá wrote:  
> > > > On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote:  
> > > > > On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:  
> > > > > > v1:
> > > > > >  
> > > > > > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517
> > > > > > 5273b81dbfe40b7f0daffcdc67d6cb8ff
> > > > > > 
> > > > > > v2:
> > > > > >  
> > > > > > https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.co
> > > > > > m
> > > > > > 
> > > > > > Changes in v3:
> > > > > > - Patch 1:
> > > > > >  * Use proposed generic schema [1]. Also make it a required property;
> > > > > >  * Improved the commit message.
> > > > > > - Patch 2:
> > > > > >  * Improved commit message.
> > > > > > - Patch 4:
> > > > > >  * Namespace all IIO DMAENGINE buffer exports;
> > > > > >  * Removed unrelated new line removal change.
> > > > > > - Patch 5:
> > > > > >  * Namespace all IIO backend exports.
> > > > > > - Patch 6:
> > > > > >  * Set backend.h in alphabetical order;
> > > > > >  * Import IIO backend namespace.
> > > > > > - Patch 7:
> > > > > >  * Don't depend on OF in kbuild anymore;
> > > > > >  * Import IIO backend namespace.
> > > > > > 
> > > > > > For the bindings patches, I tried not to enter into much details about
> > > > > > the IIO framework as I think specifics of the implementation don't care
> > > > > > from the bindings perspective. Hopefully the commit messages are good
> > > > > > enough.
> > > > > > 
> > > > > > I'm also aware that patch 1 is not backward compatible but we are
> > > > > > anyways doing it on the driver side (and on the driver the property is
> > > > > > indeed required). Anyways, just let me know if making the property
> > > > > > required is not acceptable (I'm fairly confident no one was using the
> > > > > > upstream version of the driver and so validating devicetrees for it). 
> > > > > > 
> > > > > > Keeping the block diagram in v3's cover so we don't have to follow links
> > > > > > to check the one of the typicals setups. 
> > > > > > 
> > > > > >                                            ----------------------------------
> > > > > > ----
> > > > > > -----------------
> > > > > >  ------------------                        | -----------         ------------
> > > > > >       -------  FPGA |
> > > > > >  |     ADC        |------------------------| | AXI ADC |---------| DMA CORE
> > > > > > |----
> > > > > > --| RAM |       |
> > > > > >  | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------|         
> > > > > > |----
> > > > > > --|     |       |
> > > > > >  |                |------------------------| -----------         ------------
> > > > > >       -------       |
> > > > > >  ------------------                        ----------------------------------
> > > > > > ----
> > > > > > -----------------  
> > > > > 
> > > > > Why doesn't axi-adc just have an io-channels property to adc? It's the 
> > > > > opposite direction for the link, but it seems more logical to me that 
> > > > > axi-adc depends on adc rather than the other way around.
> > > > >   
> > > > 
> > > > We are not interested on a single channel but on the complete device. >From the
> > > > axi-
> > > > adc point of view, there's not much we could do with the adc channel. I mean,
> > > > maybe
> > > > we could still do something but it would be far from ideal (see below).  
> > > 
> > > Will this hold up for everyone? Could there be a backend device that 
> > > handles multiple ADCs? IOW, do you need #io-backend-cells? It's somewhat 
> > > better if we add that up front rather than later and have to treat 
> > > missing as 0 cells. It is also the only way to generically identify the 
> > > providers (well, there's also 'foo-controller' properties, but we've 
> > > gone away from those).
> > >   
> > 
> > For the axi-adc backend, it's very unlikely. The way the core connects to the
> > converters is through a serial data interface (LVDS, CMOS or JESD in ADI usecases).
> > This interface is not really a bus so it's kind of a 1:1 connection. Now, in more
> > complicated devices (like highly integrated RF transceivers) what we have is that we
> > have multiple of these cores (one per RX/TX port) connected to the frontend. So,
> > effectively 1 frontend could have multiple backends. So, yes, your first "doubts" are
> > not that "crazy" as this is also not the "typical" provider - consumer relationship.
> > However, for all of what I've said in the previous email, even in these cases,
> > thinking in these cores as the provider, makes things much more easier to handle.
> > 
> > However, the above is just ADI usecases. In theory, yes, it can be very possible for
> > another backend other than axi-adc to have multiple frontends connected so I guess we
> > can make #io-backend-cells already available in the schema.
> 
> I'd like ultimately to consider making this work for new instances of
> dfsdm devices (separately front end ADC that spits out a modulated signal that a host
> controller then turns into something useful). In those cases we might well see a mix
> of front ends connected to one backend (at least in theory - not sure anyone would
> build such thing outside of an eval board).
> 
> Adding the flexibility from the start would be sensible. So I agree with Rob here.
> 
> > 
> > For the axi-adc bindings this would be 'const: 0', right?
> > 
> > >   
> > > > The opposite direction is exactly what we had (look at patch 2) just with another
> > > > custom property. The problem with that is that we would then need a bidirectional
> > > > link (we would need to callback into the provider and the provider would need to
> > > > access the consumer) between consumers and providers and that would be far from
> > > > optimal. The bidirectional link would exist because if we want to support
> > > > fundamental
> > > > things like LVDS/CMOS interface tuning we need to set/get settings from the axi-
> > > > adc
> > > > core. And as Jonathan suggested, the real data is captured or sent on the
> > > > converters
> > > > (ADC or DACs) and that is why we have the IIO device/interface in there and why
> > > > we
> > > > call them "frontends". In ADI usecases, backends are these FPGA cores providing
> > > > "services" to the "real" high speed converters. To put it in another way, the
> > > > real
> > > > converter is the one who knows how to use the axi-adc core and not the other way
> > > > around. That's also one of the reasons why it would be way more difficult to
> > > > handle
> > > > things with the opposite link. That's how we have done it so far and the mess we
> > > > have
> > > > out of tree is massive :sweat_smile: We ended up doing raw writes and reads on
> > > > the
> > > > axi-adc MMIO registers from the converter driver just because we had to configure
> > > > or
> > > > get something from the axi-adc device but the link was backwards.  
> > > 
> > > The direction (for the binding) doesn't really matter. It doesn't 
> > > dictate the direction in the OS. In the ad9467 driver, you can search 
> > > the DT for 'adi,adc-dev' and find the node which matches your node's 
> > > phandle. It's not exactly efficient, but you are doing it once. It would 
> > > also prevent the DT ABI break you are introducing.
> > >   
> > 
> > Hmm, I think I see your idea. So you mean something like
> > devm_iio_backend_get_optional() and if not present, then we would look for nodes
> > having the 'adi,adc-dev' property and look for the one pointing at us... Then, we
> > would need another API in the backend to look for registered backends matching that
> > fwnode. Right?
> > 
> > I mean, I guess this could work but we would already have to start a fresh framework
> > with API's that are not really meant to be used anymore other than the ad9467 driver
> > (not devm_iio_backend_get_optional() because sooner or later I think we'll need that
> > one). We are already breaking ABI in the driver and I'm still fairly confident no one
> > is really using the upstream driver because it's lacking support for devices and
> > important features (not even in ADI fork we're using it).
> > 
> > Anyways, if you still insist on having something like this (and feel more comfortable
> > in not breaking DT ABI), I can see how this would look like in the next version...
> 
> See how it looks. If it means removing the need to convince Rob then it's probably easier
> to write the code than try and talk him around ;)  Can happily stick a bit deprecated
> note next to the binding and the code.

I only point out ABI breaks and require they are justified in the commit 
message (basically stating what you state above). Otherwise, I don't 
care if I don't use the platform.

> > > > > And if there's another consumer in the chain, then a node could 
> > > > > certainly be both an io-channels consumer and producer.
> > > > >   
> > > > 
> > > > This should also be possible with this architecture. A node can be both a backend
> > > > (provider) and a consumer and we have an out of tree design that fits this (that
> > > > I
> > > > surely want to upstream after the foundations are done).
> > > >   
> > > > > The architecture of the drivers seems odd to me. It looks similar to 
> > > > > making a phy driver handle all the state and protocol with the host 
> > > > > controller being a backend.  
> > > > 
> > > > In this case, it's not really a controller. It's more like an extension of the
> > > > device
> > > > because we need a way to handle the high sample rates this ADCs can do. Then we
> > > > can
> > > > also do test tones with the backend which is useful for interface tuning (as
> > > > mentioned above).
> > > > 
> > > > To give you a bit more context, I'm adding the generic property because we will
> > > > have
> > > > more users for it (from ADI - the next should be the axi-dac core) but STM is
> > > > also
> > > > interested in this (likely the next user).
> > > > 
> > > > Hope this makes it a bit more clear...  
> > > 
> > > Yes, thanks.
> > > 
> > > I generally ask for 2 users on new common bindings. I've accepted too 
> > > many only to have the 2nd user come in a month later and need additions. 
> > > An ack on the binding from the STM folks would be nice here. And 
> > > Jonathan too.
> > >   
> > 
> > Olivier, could we get an ack on the bindings patch? Do you also have any idea about
> > how long it would take for you to send patches so we have another user of the schema?
> > 
> > On my side, it might very well take a month or so (given we have holidays nearby) as
> > the axi-dac core is more complex than the axi-adc. Bah it might take less than a
> > month to have the first version of it posted in the lists but I can't make any
> > promises.
> 
> For the driver side of things I'd like at least 2, preferably 3 users before merging.
> We have more flexibility to rework things as any issues will probably be internal
> interfaces, but I'd rather wait if we are going to have 3 users within another month
> or 2.

Really, for the binding, I'm fine with participation in the upstreaming 
of this (i.e. reviewing and/or testing on another platform) and a 'this 
works for us' on the binding.

Rob

Re: [PATCH v3 0/8] iio: add new backend framework

[Nuno Sá]

On Wed, 2023-12-20 at 08:17 -0600, Rob Herring wrote:
> On Sun, Dec 17, 2023 at 02:04:12PM +0000, Jonathan Cameron wrote:
> > On Fri, 15 Dec 2023 16:18:39 +0100
> > Nuno Sá <noname.nuno@gmail.com> wrote:
> > 
> > > On Thu, 2023-12-14 at 11:03 -0600, Rob Herring wrote:
> > > > On Thu, Dec 14, 2023 at 05:05:20PM +0100, Nuno Sá wrote:  
> > > > > On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote:  
> > > > > > On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:  
> > > > > > > v1:
> > > > > > >  
> > > > > > > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517
> > > > > > > 5273b81dbfe40b7f0daffcdc67d6cb8ff
> > > > > > > 
> > > > > > > v2:
> > > > > > >  
> > > > > > > https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.co
> > > > > > > m
> > > > > > > 
> > > > > > > Changes in v3:
> > > > > > > - Patch 1:
> > > > > > >  * Use proposed generic schema [1]. Also make it a required
> > > > > > > property;
> > > > > > >  * Improved the commit message.
> > > > > > > - Patch 2:
> > > > > > >  * Improved commit message.
> > > > > > > - Patch 4:
> > > > > > >  * Namespace all IIO DMAENGINE buffer exports;
> > > > > > >  * Removed unrelated new line removal change.
> > > > > > > - Patch 5:
> > > > > > >  * Namespace all IIO backend exports.
> > > > > > > - Patch 6:
> > > > > > >  * Set backend.h in alphabetical order;
> > > > > > >  * Import IIO backend namespace.
> > > > > > > - Patch 7:
> > > > > > >  * Don't depend on OF in kbuild anymore;
> > > > > > >  * Import IIO backend namespace.
> > > > > > > 
> > > > > > > For the bindings patches, I tried not to enter into much details
> > > > > > > about
> > > > > > > the IIO framework as I think specifics of the implementation don't
> > > > > > > care
> > > > > > > from the bindings perspective. Hopefully the commit messages are
> > > > > > > good
> > > > > > > enough.
> > > > > > > 
> > > > > > > I'm also aware that patch 1 is not backward compatible but we are
> > > > > > > anyways doing it on the driver side (and on the driver the
> > > > > > > property is
> > > > > > > indeed required). Anyways, just let me know if making the property
> > > > > > > required is not acceptable (I'm fairly confident no one was using
> > > > > > > the
> > > > > > > upstream version of the driver and so validating devicetrees for
> > > > > > > it). 
> > > > > > > 
> > > > > > > Keeping the block diagram in v3's cover so we don't have to follow
> > > > > > > links
> > > > > > > to check the one of the typicals setups. 
> > > > > > > 
> > > > > > >                                            -----------------------
> > > > > > > -----------
> > > > > > > ----
> > > > > > > -----------------
> > > > > > >  ------------------                        | -----------         -
> > > > > > > -----------
> > > > > > >       -------  FPGA |
> > > > > > >  |     ADC        |------------------------| | AXI ADC |---------|
> > > > > > > DMA CORE
> > > > > > > > ----
> > > > > > > --| RAM |       |
> > > > > > >  | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------
> > > > > > > |         
> > > > > > > > ----
> > > > > > > --|     |       |
> > > > > > >  |                |------------------------| -----------         -
> > > > > > > -----------
> > > > > > >       -------       |
> > > > > > >  ------------------                        -----------------------
> > > > > > > -----------
> > > > > > > ----
> > > > > > > -----------------  
> > > > > > 
> > > > > > Why doesn't axi-adc just have an io-channels property to adc? It's
> > > > > > the 
> > > > > > opposite direction for the link, but it seems more logical to me
> > > > > > that 
> > > > > > axi-adc depends on adc rather than the other way around.
> > > > > >   
> > > > > 
> > > > > We are not interested on a single channel but on the complete
> > > > > device. >From the
> > > > > axi-
> > > > > adc point of view, there's not much we could do with the adc channel.
> > > > > I mean,
> > > > > maybe
> > > > > we could still do something but it would be far from ideal (see
> > > > > below).  
> > > > 
> > > > Will this hold up for everyone? Could there be a backend device that 
> > > > handles multiple ADCs? IOW, do you need #io-backend-cells? It's somewhat
> > > > better if we add that up front rather than later and have to treat 
> > > > missing as 0 cells. It is also the only way to generically identify the 
> > > > providers (well, there's also 'foo-controller' properties, but we've 
> > > > gone away from those).
> > > >   
> > > 
> > > For the axi-adc backend, it's very unlikely. The way the core connects to
> > > the
> > > converters is through a serial data interface (LVDS, CMOS or JESD in ADI
> > > usecases).
> > > This interface is not really a bus so it's kind of a 1:1 connection. Now,
> > > in more
> > > complicated devices (like highly integrated RF transceivers) what we have
> > > is that we
> > > have multiple of these cores (one per RX/TX port) connected to the
> > > frontend. So,
> > > effectively 1 frontend could have multiple backends. So, yes, your first
> > > "doubts" are
> > > not that "crazy" as this is also not the "typical" provider - consumer
> > > relationship.
> > > However, for all of what I've said in the previous email, even in these
> > > cases,
> > > thinking in these cores as the provider, makes things much more easier to
> > > handle.
> > > 
> > > However, the above is just ADI usecases. In theory, yes, it can be very
> > > possible for
> > > another backend other than axi-adc to have multiple frontends connected so
> > > I guess we
> > > can make #io-backend-cells already available in the schema.
> > 
> > I'd like ultimately to consider making this work for new instances of
> > dfsdm devices (separately front end ADC that spits out a modulated signal
> > that a host
> > controller then turns into something useful). In those cases we might well
> > see a mix
> > of front ends connected to one backend (at least in theory - not sure anyone
> > would
> > build such thing outside of an eval board).
> > 
> > Adding the flexibility from the start would be sensible. So I agree with Rob
> > here.
> > 
> > > 
> > > For the axi-adc bindings this would be 'const: 0', right?
> > > 
> > > >   
> > > > > The opposite direction is exactly what we had (look at patch 2) just
> > > > > with another
> > > > > custom property. The problem with that is that we would then need a
> > > > > bidirectional
> > > > > link (we would need to callback into the provider and the provider
> > > > > would need to
> > > > > access the consumer) between consumers and providers and that would be
> > > > > far from
> > > > > optimal. The bidirectional link would exist because if we want to
> > > > > support
> > > > > fundamental
> > > > > things like LVDS/CMOS interface tuning we need to set/get settings
> > > > > from the axi-
> > > > > adc
> > > > > core. And as Jonathan suggested, the real data is captured or sent on
> > > > > the
> > > > > converters
> > > > > (ADC or DACs) and that is why we have the IIO device/interface in
> > > > > there and why
> > > > > we
> > > > > call them "frontends". In ADI usecases, backends are these FPGA cores
> > > > > providing
> > > > > "services" to the "real" high speed converters. To put it in another
> > > > > way, the
> > > > > real
> > > > > converter is the one who knows how to use the axi-adc core and not the
> > > > > other way
> > > > > around. That's also one of the reasons why it would be way more
> > > > > difficult to
> > > > > handle
> > > > > things with the opposite link. That's how we have done it so far and
> > > > > the mess we
> > > > > have
> > > > > out of tree is massive :sweat_smile: We ended up doing raw writes and
> > > > > reads on
> > > > > the
> > > > > axi-adc MMIO registers from the converter driver just because we had
> > > > > to configure
> > > > > or
> > > > > get something from the axi-adc device but the link was backwards.  
> > > > 
> > > > The direction (for the binding) doesn't really matter. It doesn't 
> > > > dictate the direction in the OS. In the ad9467 driver, you can search 
> > > > the DT for 'adi,adc-dev' and find the node which matches your node's 
> > > > phandle. It's not exactly efficient, but you are doing it once. It would
> > > > also prevent the DT ABI break you are introducing.
> > > >   
> > > 
> > > Hmm, I think I see your idea. So you mean something like
> > > devm_iio_backend_get_optional() and if not present, then we would look for
> > > nodes
> > > having the 'adi,adc-dev' property and look for the one pointing at us...
> > > Then, we
> > > would need another API in the backend to look for registered backends
> > > matching that
> > > fwnode. Right?
> > > 
> > > I mean, I guess this could work but we would already have to start a fresh
> > > framework
> > > with API's that are not really meant to be used anymore other than the
> > > ad9467 driver
> > > (not devm_iio_backend_get_optional() because sooner or later I think we'll
> > > need that
> > > one). We are already breaking ABI in the driver and I'm still fairly
> > > confident no one
> > > is really using the upstream driver because it's lacking support for
> > > devices and
> > > important features (not even in ADI fork we're using it).
> > > 
> > > Anyways, if you still insist on having something like this (and feel more
> > > comfortable
> > > in not breaking DT ABI), I can see how this would look like in the next
> > > version...
> > 
> > See how it looks. If it means removing the need to convince Rob then it's
> > probably easier
> > to write the code than try and talk him around ;)  Can happily stick a bit
> > deprecated
> > note next to the binding and the code.
> 
> I only point out ABI breaks and require they are justified in the commit 
> message (basically stating what you state above). Otherwise, I don't 
> care if I don't use the platform.
> 

Good to know. I already worked on a new series only with your suggestion so now
I'll send it anyways :). In the end it will be up to me and Jonathan to decide
if we want to add an API only meant to be used for this.

Thanks!
- Nuno Sá
> > > > > 

Re: [PATCH v3 0/8] iio: add new backend framework

[Olivier MOYSAN]

Hi Nuno,

On 12/15/23 16:18, Nuno Sá wrote:
> On Thu, 2023-12-14 at 11:03 -0600, Rob Herring wrote:
>> On Thu, Dec 14, 2023 at 05:05:20PM +0100, Nuno Sá wrote:
>>> On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote:
>>>> On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:
>>>>> v1:
>>>>>   
>>>>> https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517
>>>>> 5273b81dbfe40b7f0daffcdc67d6cb8ff
>>>>>
>>>>> v2:
>>>>>   
>>>>> https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@analog.co
>>>>> m
>>>>>
>>>>> Changes in v3:
>>>>> - Patch 1:
>>>>>   * Use proposed generic schema [1]. Also make it a required property;
>>>>>   * Improved the commit message.
>>>>> - Patch 2:
>>>>>   * Improved commit message.
>>>>> - Patch 4:
>>>>>   * Namespace all IIO DMAENGINE buffer exports;
>>>>>   * Removed unrelated new line removal change.
>>>>> - Patch 5:
>>>>>   * Namespace all IIO backend exports.
>>>>> - Patch 6:
>>>>>   * Set backend.h in alphabetical order;
>>>>>   * Import IIO backend namespace.
>>>>> - Patch 7:
>>>>>   * Don't depend on OF in kbuild anymore;
>>>>>   * Import IIO backend namespace.
>>>>>
>>>>> For the bindings patches, I tried not to enter into much details about
>>>>> the IIO framework as I think specifics of the implementation don't care
>>>>> from the bindings perspective. Hopefully the commit messages are good
>>>>> enough.
>>>>>
>>>>> I'm also aware that patch 1 is not backward compatible but we are
>>>>> anyways doing it on the driver side (and on the driver the property is
>>>>> indeed required). Anyways, just let me know if making the property
>>>>> required is not acceptable (I'm fairly confident no one was using the
>>>>> upstream version of the driver and so validating devicetrees for it).
>>>>>
>>>>> Keeping the block diagram in v3's cover so we don't have to follow links
>>>>> to check the one of the typicals setups.
>>>>>
>>>>>                                             ----------------------------------
>>>>> ----
>>>>> -----------------
>>>>>   ------------------                        | -----------         ------------
>>>>>        -------  FPGA |
>>>>>   |     ADC        |------------------------| | AXI ADC |---------| DMA CORE
>>>>> |----
>>>>> --| RAM |       |
>>>>>   | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------|
>>>>> |----
>>>>> --|     |       |
>>>>>   |                |------------------------| -----------         ------------
>>>>>        -------       |
>>>>>   ------------------                        ----------------------------------
>>>>> ----
>>>>> -----------------
>>>>
>>>> Why doesn't axi-adc just have an io-channels property to adc? It's the
>>>> opposite direction for the link, but it seems more logical to me that
>>>> axi-adc depends on adc rather than the other way around.
>>>>
>>>
>>> We are not interested on a single channel but on the complete device. >From the
>>> axi-
>>> adc point of view, there's not much we could do with the adc channel. I mean,
>>> maybe
>>> we could still do something but it would be far from ideal (see below).
>>
>> Will this hold up for everyone? Could there be a backend device that
>> handles multiple ADCs? IOW, do you need #io-backend-cells? It's somewhat
>> better if we add that up front rather than later and have to treat
>> missing as 0 cells. It is also the only way to generically identify the
>> providers (well, there's also 'foo-controller' properties, but we've
>> gone away from those).
>>
> 
> For the axi-adc backend, it's very unlikely. The way the core connects to the
> converters is through a serial data interface (LVDS, CMOS or JESD in ADI usecases).
> This interface is not really a bus so it's kind of a 1:1 connection. Now, in more
> complicated devices (like highly integrated RF transceivers) what we have is that we
> have multiple of these cores (one per RX/TX port) connected to the frontend. So,
> effectively 1 frontend could have multiple backends. So, yes, your first "doubts" are
> not that "crazy" as this is also not the "typical" provider - consumer relationship.
> However, for all of what I've said in the previous email, even in these cases,
> thinking in these cores as the provider, makes things much more easier to handle.
> 
> However, the above is just ADI usecases. In theory, yes, it can be very possible for
> another backend other than axi-adc to have multiple frontends connected so I guess we
> can make #io-backend-cells already available in the schema.
> 
> For the axi-adc bindings this would be 'const: 0', right?
> 
>>
>>> The opposite direction is exactly what we had (look at patch 2) just with another
>>> custom property. The problem with that is that we would then need a bidirectional
>>> link (we would need to callback into the provider and the provider would need to
>>> access the consumer) between consumers and providers and that would be far from
>>> optimal. The bidirectional link would exist because if we want to support
>>> fundamental
>>> things like LVDS/CMOS interface tuning we need to set/get settings from the axi-
>>> adc
>>> core. And as Jonathan suggested, the real data is captured or sent on the
>>> converters
>>> (ADC or DACs) and that is why we have the IIO device/interface in there and why
>>> we
>>> call them "frontends". In ADI usecases, backends are these FPGA cores providing
>>> "services" to the "real" high speed converters. To put it in another way, the
>>> real
>>> converter is the one who knows how to use the axi-adc core and not the other way
>>> around. That's also one of the reasons why it would be way more difficult to
>>> handle
>>> things with the opposite link. That's how we have done it so far and the mess we
>>> have
>>> out of tree is massive :sweat_smile: We ended up doing raw writes and reads on
>>> the
>>> axi-adc MMIO registers from the converter driver just because we had to configure
>>> or
>>> get something from the axi-adc device but the link was backwards.
>>
>> The direction (for the binding) doesn't really matter. It doesn't
>> dictate the direction in the OS. In the ad9467 driver, you can search
>> the DT for 'adi,adc-dev' and find the node which matches your node's
>> phandle. It's not exactly efficient, but you are doing it once. It would
>> also prevent the DT ABI break you are introducing.
>>
> 
> Hmm, I think I see your idea. So you mean something like
> devm_iio_backend_get_optional() and if not present, then we would look for nodes
> having the 'adi,adc-dev' property and look for the one pointing at us... Then, we
> would need another API in the backend to look for registered backends matching that
> fwnode. Right?
> 
> I mean, I guess this could work but we would already have to start a fresh framework
> with API's that are not really meant to be used anymore other than the ad9467 driver
> (not devm_iio_backend_get_optional() because sooner or later I think we'll need that
> one). We are already breaking ABI in the driver and I'm still fairly confident no one
> is really using the upstream driver because it's lacking support for devices and
> important features (not even in ADI fork we're using it).
> 
> Anyways, if you still insist on having something like this (and feel more comfortable
> in not breaking DT ABI), I can see how this would look like in the next version...
> 
>>
>>>> And if there's another consumer in the chain, then a node could
>>>> certainly be both an io-channels consumer and producer.
>>>>
>>>
>>> This should also be possible with this architecture. A node can be both a backend
>>> (provider) and a consumer and we have an out of tree design that fits this (that
>>> I
>>> surely want to upstream after the foundations are done).
>>>
>>>> The architecture of the drivers seems odd to me. It looks similar to
>>>> making a phy driver handle all the state and protocol with the host
>>>> controller being a backend.
>>>
>>> In this case, it's not really a controller. It's more like an extension of the
>>> device
>>> because we need a way to handle the high sample rates this ADCs can do. Then we
>>> can
>>> also do test tones with the backend which is useful for interface tuning (as
>>> mentioned above).
>>>
>>> To give you a bit more context, I'm adding the generic property because we will
>>> have
>>> more users for it (from ADI - the next should be the axi-dac core) but STM is
>>> also
>>> interested in this (likely the next user).
>>>
>>> Hope this makes it a bit more clear...
>>
>> Yes, thanks.
>>
>> I generally ask for 2 users on new common bindings. I've accepted too
>> many only to have the 2nd user come in a month later and need additions.
>> An ack on the binding from the STM folks would be nice here. And
>> Jonathan too.
>>
> 
> Olivier, could we get an ack on the bindings patch? Do you also have any idea about
> how long it would take for you to send patches so we have another user of the schema?
> 
> On my side, it might very well take a month or so (given we have holidays nearby) as
> the axi-dac core is more complex than the axi-adc. Bah it might take less than a
> month to have the first version of it posted in the lists but I can't make any
> promises.
> 

Sorry for late answer.
Regarding bindings patch, I assume you refer to [1].
After adding the #io-backend-cells property in v5 you can add my
Ack-by Olivier Moysan <olivier.moysan@foss.st.com>

I will prepare a serie based on these bindings. Hopefully it should be 
possible this month.

BRs
Olivier

[1] https://github.com/devicetree-org/dt-schema/pull/120


> - Nuno Sá
>