diff mbox series

[v2,2/9] dt-bindings: iio: dac: adi-axi-adc: Add ad7606 variant

Message ID 20241210-ad7606_add_iio_backend_software_mode-v2-2-6619c3e50d81@baylibre.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for Software mode on AD7606's iio backend driver | expand

Commit Message

Guillaume Stols Dec. 10, 2024, 10:46 a.m. UTC
A new compatible is added to reflect the specialized version of the HDL.
We use the parallel interface to write the ADC's registers, and
accessing this interface requires to use
ADI_AXI_REG_CONFIG_RD,ADI_AXI_REG_CONFIG_WR and ADI_AXI_REG_CONFIG_CTRL
in a custom fashion.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 .../devicetree/bindings/iio/adc/adi,axi-adc.yaml   | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)

Comments

Rob Herring Dec. 10, 2024, 12:31 p.m. UTC | #1
On Tue, 10 Dec 2024 10:46:42 +0000, Guillaume Stols wrote:
> A new compatible is added to reflect the specialized version of the HDL.
> We use the parallel interface to write the ADC's registers, and
> accessing this interface requires to use
> ADI_AXI_REG_CONFIG_RD,ADI_AXI_REG_CONFIG_WR and ADI_AXI_REG_CONFIG_CTRL
> in a custom fashion.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,axi-adc.yaml   | 53 ++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.example.dtb: axi-adc@44a00000: '#address-cells', '#size-cells', 'adi_adc@0' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/iio/adc/adi,axi-adc.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.example.dtb: adi_adc@0: pwm-names:0: 'convst1' was expected
	from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad7606.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.example.dtb: adi_adc@0: pwm-names: ['cnvst_n'] is too short
	from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad7606.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.example.dtb: adi_adc@0: 'vdrive-supply' is a required property
	from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad7606.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241210-ad7606_add_iio_backend_software_mode-v2-2-6619c3e50d81@baylibre.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.
David Lechner Dec. 11, 2024, 10:01 p.m. UTC | #2
On 12/10/24 4:46 AM, Guillaume Stols wrote:
> A new compatible is added to reflect the specialized version of the HDL.
> We use the parallel interface to write the ADC's registers, and
> accessing this interface requires to use
> ADI_AXI_REG_CONFIG_RD,ADI_AXI_REG_CONFIG_WR and ADI_AXI_REG_CONFIG_CTRL
> in a custom fashion.
> 
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,axi-adc.yaml   | 53 ++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
> index e1f450b80db2..6c3fc44422cc 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
> @@ -17,13 +17,22 @@ description: |
>    interface for the actual ADC, while this IP core will interface
>    to the data-lines of the ADC and handle the streaming of data into
>    memory via DMA.
> +  In some cases, the AXI ADC interface is used to perform specialized
> +  operation to a particular ADC, e.g access the physical bus through
> +  specific registers to write ADC registers.
> +  In this case, we use a different compatible whch indicates the target

s/whch/which/

> +  chip(s)'s name.

s/chip(s)'s/IP core's/

> +  The following IP is currently supported:
> +    -axi_ad7606X: Backend for all the chips from the ad7606 family.

s/axi_ad7606X/AXI AD7606X/ # proper name of the IP core
s/Backend/Specialized version of the IP core/

>  
>    https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
> +  http://analogdevicesinc.github.io/hdl/library/axi_ad7606x/index.html
>  
>  properties:
>    compatible:
>      enum:
>        - adi,axi-adc-10.0.a
> +      - adi,axi-ad7606x
>  
>    reg:
>      maxItems: 1
> @@ -53,6 +62,24 @@ required:
>    - reg
>    - clocks
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: adi,axi-ad7606x
> +    then:
> +      patternProperties:
> +        "^adc@[0-9a-f]+$":
> +          type: object
> +          properties:
> +            reg:
> +              maxItems: 1
> +          additionalProperties: true
> +          required:
> +            - compatible
> +            - reg
> +

The preferred way to compatible-specific bindings is to add
everything at the top level and then disable ones that don't
apply using -if: blocks.

So under the top-level properties:, add

  '#address-cells':
    const: 1
  '#size-cells':
    const 0

and move the patternProperties: to the top level.

Then add not: to the -if: and make the then:

    properties:
      '#address-cells': false
      '#size-cells': false
    patternProperties:
      "^adc@[0-9a-f]+$": false

>  additionalProperties: false
>  
>  examples:
> @@ -65,4 +92,30 @@ examples:
>          clocks = <&axi_clk>;
>          #io-backend-cells = <0>;
>      };
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    axi-adc@44a00000 {

Shouldn't we have the label iio_backend: here?

Also, could use a more generic name like "parallel-bus-controller"
instead of "axi-adc".

> +        compatible = "adi,axi-ad7606x";
> +        reg = <0x44a00000 0x10000>;
> +        dmas = <&rx_dma 0>;
> +        dma-names = "rx";
> +        clocks = <&ext_clk>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adi_adc@0 {
> +            compatible = "adi,ad7606b";
> +            reg = <0>;
> +            pwms = <&axi_pwm_gen 0 0>;
> +            pwm-names = "cnvst_n";
> +            avcc-supply = <&adc_vref>;
> +            reset-gpios = <&gpio0 91 GPIO_ACTIVE_HIGH>;
> +            standby-gpios = <&gpio0 90 GPIO_ACTIVE_LOW>;
> +            adi,range-gpios = <&gpio0 89 GPIO_ACTIVE_HIGH>;
> +            adi,oversampling-ratio-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH
> +                            &gpio0 87 GPIO_ACTIVE_HIGH
> +                            &gpio0 86 GPIO_ACTIVE_HIGH>;
> +            io-backends = <&iio_backend>;
> +        };
> +    };
>  ...
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
index e1f450b80db2..6c3fc44422cc 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
@@ -17,13 +17,22 @@  description: |
   interface for the actual ADC, while this IP core will interface
   to the data-lines of the ADC and handle the streaming of data into
   memory via DMA.
+  In some cases, the AXI ADC interface is used to perform specialized
+  operation to a particular ADC, e.g access the physical bus through
+  specific registers to write ADC registers.
+  In this case, we use a different compatible whch indicates the target
+  chip(s)'s name.
+  The following IP is currently supported:
+    -axi_ad7606X: Backend for all the chips from the ad7606 family.
 
   https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
+  http://analogdevicesinc.github.io/hdl/library/axi_ad7606x/index.html
 
 properties:
   compatible:
     enum:
       - adi,axi-adc-10.0.a
+      - adi,axi-ad7606x
 
   reg:
     maxItems: 1
@@ -53,6 +62,24 @@  required:
   - reg
   - clocks
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: adi,axi-ad7606x
+    then:
+      patternProperties:
+        "^adc@[0-9a-f]+$":
+          type: object
+          properties:
+            reg:
+              maxItems: 1
+          additionalProperties: true
+          required:
+            - compatible
+            - reg
+
 additionalProperties: false
 
 examples:
@@ -65,4 +92,30 @@  examples:
         clocks = <&axi_clk>;
         #io-backend-cells = <0>;
     };
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    axi-adc@44a00000 {
+        compatible = "adi,axi-ad7606x";
+        reg = <0x44a00000 0x10000>;
+        dmas = <&rx_dma 0>;
+        dma-names = "rx";
+        clocks = <&ext_clk>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adi_adc@0 {
+            compatible = "adi,ad7606b";
+            reg = <0>;
+            pwms = <&axi_pwm_gen 0 0>;
+            pwm-names = "cnvst_n";
+            avcc-supply = <&adc_vref>;
+            reset-gpios = <&gpio0 91 GPIO_ACTIVE_HIGH>;
+            standby-gpios = <&gpio0 90 GPIO_ACTIVE_LOW>;
+            adi,range-gpios = <&gpio0 89 GPIO_ACTIVE_HIGH>;
+            adi,oversampling-ratio-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH
+                            &gpio0 87 GPIO_ACTIVE_HIGH
+                            &gpio0 86 GPIO_ACTIVE_HIGH>;
+            io-backends = <&iio_backend>;
+        };
+    };
 ...