diff mbox series

[1/4] dt-bindings: iio: resolver: add devicetree bindings for ad2s1210

Message ID 20230920170253.203395-2-dlechner@baylibre.com
State Changes Requested
Headers show
Series iio: resolver: move ad2s1210 out of staging | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dt-meta-schema fail build log

Commit Message

David Lechner Sept. 20, 2023, 5:02 p.m. UTC
This adds new DeviceTree bindings for the Analog Devices, Inc. AD2S1210
resolver-to-digital converter.

Signed-off-by: Apelete Seketeli <aseketeli@baylibre.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 .../bindings/iio/resolver/adi,ad2s1210.yaml   | 150 ++++++++++++++++++
 1 file changed, 150 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml

Comments

Rob Herring (Arm) Sept. 20, 2023, 10:33 p.m. UTC | #1
On Wed, 20 Sep 2023 12:02:50 -0500, David Lechner wrote:
> This adds new DeviceTree bindings for the Analog Devices, Inc. AD2S1210
> resolver-to-digital converter.
> 
> Signed-off-by: Apelete Seketeli <aseketeli@baylibre.com>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  .../bindings/iio/resolver/adi,ad2s1210.yaml   | 150 ++++++++++++++++++
>  1 file changed, 150 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml
> 

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:
./Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml:102:12: [error] string value is redundantly quoted with any quotes (quoted-strings)
./Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml:102:22: [error] string value is redundantly quoted with any quotes (quoted-strings)
./Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml:102:34: [error] string value is redundantly quoted with any quotes (quoted-strings)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230920170253.203395-2-dlechner@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.
Conor Dooley Sept. 21, 2023, 9:06 a.m. UTC | #2
Yo,

On Wed, Sep 20, 2023 at 12:02:50PM -0500, David Lechner wrote:
> This adds new DeviceTree bindings for the Analog Devices, Inc. AD2S1210
> resolver-to-digital converter.
> 
> Signed-off-by: Apelete Seketeli <aseketeli@baylibre.com>
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Missing a From: or co-developed-by?

> ---
>  .../bindings/iio/resolver/adi,ad2s1210.yaml   | 150 ++++++++++++++++++
>  1 file changed, 150 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml b/Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml
> new file mode 100644
> index 000000000000..cf6838710e52
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml
> @@ -0,0 +1,150 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/resolver/adi,ad2s1210.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD2S1210 Resolver-to-Digital Converter
> +
> +maintainers:
> +  - Michael Hennerich <michael.hennerich@analog.com>

Why's this a chap from analog, rather than the authors?

> +description: |
> +  The AD2S1210 is a complete 10-bit to 16-bit resolution tracking
> +  resolver-to-digital converter, integrating an on-board programmable
> +  sinusoidal oscillator that provides sine wave excitation for
> +  resolvers.
> +
> +  The AD2S1210 allows the user to read the angular position or the
> +  angular velocity data directly from the parallel outputs or through
> +  the serial interface.
> +
> +    A1  A0  Result
> +     0   0  Normal mode - position output
> +     0   1  Normal mode - velocity output
> +     1   0  Reserved
> +     1   1  Configuration mode
> +
> +  In normal mode, the resolution of the digital output is selected using
> +  the RES0 and RES1 input pins. In configuration mode, the resolution is
> +  selected by setting the RES0 and RES1 bits in the control register.
> +
> +  RES1  RES0  Resolution (Bits)
> +     0     0  10
> +     0     1  12
> +     1     0  14
> +     1     1  16
> +
> +  Note on SPI connections: The CS line on the AD2S1210 should hard-wired to
> +  logic low and the WR/FSYNC line on the AD2S1210 should be connected to the
> +  SPI CSn output of the SPI controller.
> +
> +  Datasheet:
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad2s1210.pdf
> +
> +properties:
> +  compatible:
> +    const: adi,ad2s1210
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 25000000
> +
> +  spi-cpha: true
> +
> +  clocks:
> +    maxItems: 1
> +    description: External oscillator clock (CLKIN).
> +
> +  reset-gpios:
> +    description:
> +      GPIO connected to the /RESET pin. As the line needs to be low for the
> +      reset to be active, it should be configured as GPIO_ACTIVE_LOW.
> +    maxItems: 1
> +
> +  sample-gpios:
> +    description: |

What do you need the pipe for here? Don't see some formatting requiring
preservation. Ditto below.

> +      GPIO connected to the /SAMPLE pin. As the line needs to be low to trigger
> +      a sample, it should be configured as GPIO_ACTIVE_LOW.
> +    maxItems: 1
> +
> +  mode-gpios:
> +    description: |
> +      GPIO lines connected to the A0 and A1 pins. These pins select the data
> +      transfer mode.
> +    minItems: 2
> +    maxItems: 2
> +
> +  resolution-gpios:
> +    description: |
> +      GPIO lines connected to the RES0 and RES1 pins. These pins select the
> +      resolution of the digital output. If omitted, it is assumed that the
> +      RES0 and RES1 pins are hard-wired to match the assigned-resolution-bits
> +      property.
> +    minItems: 2
> +    maxItems: 2
> +
> +  fault-gpios:
> +    description: |
> +      GPIO lines connected to the LOT and DOS pins. These pins combined indicate
> +      the type of fault present, if any. As these pins a pulled low to indicate
> +      a fault condition, they should be configured as GPIO_ACTIVE_LOW.
> +    minItems: 2
> +    maxItems: 2
> +
> +  adi,fixed-mode:
> +    description: |
> +      This is used to indicate the selected mode if A0 and A1 are hard-wired
> +      instead of connected to GPIOS (i.e. mode-gpios is omitted).
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: ["config", "velocity", "position"]

The bot is complaining about these "s not being needed.

> +
> +  assigned-resolution-bits:

I figure the lack of a vendor prefix here is "inspired" by the same
property for stm32?

Cheers,
Conor.

> +    description: |
> +      Resolution of the digital output required by the application. This
> +      determines the precision of the angle and/or the maximum speed that can
> +      be measured. If resolution-gpios is omitted, it is assumed that RES0 and
> +      RES1 are hard-wired to match this value.
> +    enum: [10, 12, 14, 16]
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-cpha
> +  - clocks
> +  - sample-gpios
> +  - assigned-resolution-bits
> +
> +oneOf:
> +  - required:
> +      - mode-gpios
> +  - required:
> +      - adi,fixed-mode
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        resolver@0 {
> +            compatible = "adi,ad2s1210";
> +            reg = <0>;
> +            spi-max-frequency = <20000000>;
> +            spi-cpha;
> +            clocks = <&ext_osc>;
> +            sample-gpios = <&gpio0 90 GPIO_ACTIVE_LOW>;
> +            mode-gpios = <&gpio0 86 0>, <&gpio0 87 0>;
> +            resolution-gpios = <&gpio0 88 0>, <&gpio0 89 0>;
> +            assigned-resolution-bits = <16>;
> +        };
> +    };
> -- 
> 2.34.1
>
David Lechner Sept. 21, 2023, 9:40 a.m. UTC | #3
On Thu, Sep 21, 2023 at 11:06 AM Conor Dooley <conor@kernel.org> wrote:
>
> > Signed-off-by: Apelete Seketeli <aseketeli@baylibre.com>
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
>
> Missing a From: or co-developed-by?

Missing Co-developed-by:. Will fix.

> > +maintainers:
> > +  - Michael Hennerich <michael.hennerich@analog.com>
>
> Why's this a chap from analog, rather than the authors?

This was written by my codeveloper so not sure about the why. Michael is
on the CC so he can clarify if this is not correct.

> > +  sample-gpios:
> > +    description: |
>
> What do you need the pipe for here? Don't see some formatting requiring
> preservation. Ditto below.
>

It is just a habit to always do this for multi-line text. Will remove.

> > +
> > +  assigned-resolution-bits:
>
> I figure the lack of a vendor prefix here is "inspired" by the same
> property for stm32?
>

Correct.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml b/Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml
new file mode 100644
index 000000000000..cf6838710e52
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml
@@ -0,0 +1,150 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/resolver/adi,ad2s1210.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD2S1210 Resolver-to-Digital Converter
+
+maintainers:
+  - Michael Hennerich <michael.hennerich@analog.com>
+
+description: |
+  The AD2S1210 is a complete 10-bit to 16-bit resolution tracking
+  resolver-to-digital converter, integrating an on-board programmable
+  sinusoidal oscillator that provides sine wave excitation for
+  resolvers.
+
+  The AD2S1210 allows the user to read the angular position or the
+  angular velocity data directly from the parallel outputs or through
+  the serial interface.
+
+    A1  A0  Result
+     0   0  Normal mode - position output
+     0   1  Normal mode - velocity output
+     1   0  Reserved
+     1   1  Configuration mode
+
+  In normal mode, the resolution of the digital output is selected using
+  the RES0 and RES1 input pins. In configuration mode, the resolution is
+  selected by setting the RES0 and RES1 bits in the control register.
+
+  RES1  RES0  Resolution (Bits)
+     0     0  10
+     0     1  12
+     1     0  14
+     1     1  16
+
+  Note on SPI connections: The CS line on the AD2S1210 should hard-wired to
+  logic low and the WR/FSYNC line on the AD2S1210 should be connected to the
+  SPI CSn output of the SPI controller.
+
+  Datasheet:
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad2s1210.pdf
+
+properties:
+  compatible:
+    const: adi,ad2s1210
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 25000000
+
+  spi-cpha: true
+
+  clocks:
+    maxItems: 1
+    description: External oscillator clock (CLKIN).
+
+  reset-gpios:
+    description:
+      GPIO connected to the /RESET pin. As the line needs to be low for the
+      reset to be active, it should be configured as GPIO_ACTIVE_LOW.
+    maxItems: 1
+
+  sample-gpios:
+    description: |
+      GPIO connected to the /SAMPLE pin. As the line needs to be low to trigger
+      a sample, it should be configured as GPIO_ACTIVE_LOW.
+    maxItems: 1
+
+  mode-gpios:
+    description: |
+      GPIO lines connected to the A0 and A1 pins. These pins select the data
+      transfer mode.
+    minItems: 2
+    maxItems: 2
+
+  resolution-gpios:
+    description: |
+      GPIO lines connected to the RES0 and RES1 pins. These pins select the
+      resolution of the digital output. If omitted, it is assumed that the
+      RES0 and RES1 pins are hard-wired to match the assigned-resolution-bits
+      property.
+    minItems: 2
+    maxItems: 2
+
+  fault-gpios:
+    description: |
+      GPIO lines connected to the LOT and DOS pins. These pins combined indicate
+      the type of fault present, if any. As these pins a pulled low to indicate
+      a fault condition, they should be configured as GPIO_ACTIVE_LOW.
+    minItems: 2
+    maxItems: 2
+
+  adi,fixed-mode:
+    description: |
+      This is used to indicate the selected mode if A0 and A1 are hard-wired
+      instead of connected to GPIOS (i.e. mode-gpios is omitted).
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: ["config", "velocity", "position"]
+
+  assigned-resolution-bits:
+    description: |
+      Resolution of the digital output required by the application. This
+      determines the precision of the angle and/or the maximum speed that can
+      be measured. If resolution-gpios is omitted, it is assumed that RES0 and
+      RES1 are hard-wired to match this value.
+    enum: [10, 12, 14, 16]
+
+required:
+  - compatible
+  - reg
+  - spi-cpha
+  - clocks
+  - sample-gpios
+  - assigned-resolution-bits
+
+oneOf:
+  - required:
+      - mode-gpios
+  - required:
+      - adi,fixed-mode
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        resolver@0 {
+            compatible = "adi,ad2s1210";
+            reg = <0>;
+            spi-max-frequency = <20000000>;
+            spi-cpha;
+            clocks = <&ext_osc>;
+            sample-gpios = <&gpio0 90 GPIO_ACTIVE_LOW>;
+            mode-gpios = <&gpio0 86 0>, <&gpio0 87 0>;
+            resolution-gpios = <&gpio0 88 0>, <&gpio0 89 0>;
+            assigned-resolution-bits = <16>;
+        };
+    };