diff mbox series

[1/3] dt-bindings: add Wiegand controller dt-binding documentation

Message ID 20230104133414.39305-2-m.zatovic1@gmail.com
State Changes Requested, archived
Headers show
Series Wiegand bus driver and GPIO controller driver | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 94 lines checked
robh/patch-applied success
robh/dt-meta-schema fail build log
robh/checkpatch warning total: 0 errors, 1 warnings, 94 lines checked
robh/patch-applied success
robh/dt-meta-schema fail build log

Commit Message

Martin Zaťovič Jan. 4, 2023, 1:34 p.m. UTC
A Weigand bus is defined by a Wiegand controller node. This node
can contain one or more device nodes for devices attached to
the controller(it is advised to only connect one device as Wiegand
is a point-to-point bus).

Wiegand controller needs to specify several attributes such as
the pulse length in order to function properly. These attributes
are documented here.

Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
---
 .../bindings/wiegand/wiegand-controller.yaml  | 83 +++++++++++++++++++
 MAINTAINERS                                   |  5 ++
 2 files changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml

Comments

Rob Herring (Arm) Jan. 5, 2023, 2:53 a.m. UTC | #1
On Wed, 04 Jan 2023 14:34:12 +0100, Martin Zaťovič wrote:
> A Weigand bus is defined by a Wiegand controller node. This node
> can contain one or more device nodes for devices attached to
> the controller(it is advised to only connect one device as Wiegand
> is a point-to-point bus).
> 
> Wiegand controller needs to specify several attributes such as
> the pulse length in order to function properly. These attributes
> are documented here.
> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
>  .../bindings/wiegand/wiegand-controller.yaml  | 83 +++++++++++++++++++
>  MAINTAINERS                                   |  5 ++
>  2 files changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/wiegand/wiegand-controller.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:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/wiegand/wiegand-controller.example.dts:20.40-41 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/wiegand/wiegand-controller.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1508: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230104133414.39305-2-m.zatovic1@gmail.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.
Rob Herring (Arm) Jan. 5, 2023, 2:53 a.m. UTC | #2
On Wed, 04 Jan 2023 14:34:12 +0100, Martin Zaťovič wrote:
> A Weigand bus is defined by a Wiegand controller node. This node
> can contain one or more device nodes for devices attached to
> the controller(it is advised to only connect one device as Wiegand
> is a point-to-point bus).
> 
> Wiegand controller needs to specify several attributes such as
> the pulse length in order to function properly. These attributes
> are documented here.
> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
>  .../bindings/wiegand/wiegand-controller.yaml  | 83 +++++++++++++++++++
>  MAINTAINERS                                   |  5 ++
>  2 files changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/wiegand/wiegand-controller.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:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/wiegand/wiegand-controller.example.dts:20.40-41 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/wiegand/wiegand-controller.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1508: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230104133414.39305-2-m.zatovic1@gmail.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.
Rob Herring (Arm) Jan. 8, 2023, 4:30 p.m. UTC | #3
On Wed, Jan 04, 2023 at 02:34:12PM +0100, Martin Zaťovič wrote:
> A Weigand bus is defined by a Wiegand controller node. This node
> can contain one or more device nodes for devices attached to
> the controller(it is advised to only connect one device as Wiegand
> is a point-to-point bus).
> 
> Wiegand controller needs to specify several attributes such as
> the pulse length in order to function properly. These attributes
> are documented here.

For the subject, drop 'dt-binding documentation'. That's already stated 
once.

> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
>  .../bindings/wiegand/wiegand-controller.yaml  | 83 +++++++++++++++++++
>  MAINTAINERS                                   |  5 ++
>  2 files changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
> 
> diff --git a/Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml b/Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
> new file mode 100644
> index 000000000000..645306c65d43
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: GPL-2.0

(GPL-2.0 OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/wiegand/wiegand-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wiegand Controller Generic Binding

s/Binding/Common Properties/

> +
> +maintainers:
> +  - Martin Zaťovič <martin.zatovic@tbs-biometrics.com>
> +
> +description: |

Don't need '|'.

> +  Wiegand busses can be described with a node for the Wiegand controller device
> +  and a set of child nodes for each SPI slave on the bus.
> +
> +properties:
> +  $nodename:
> +    pattern: "^wiegand(@.*|-[0-9a-f])*$"

'?' instead of '*' for the last '*'.

 
> +
> +  compatible:
> +    maxItems: 1

Drop 'compatible' as the specific controller has to list this anyways. 
Also, a controller can have as many compatible strings as it wants.

> +    description: Compatible of the Wiegand controller.
> +
> +  data-high-gpios:
> +    maxItems: 1
> +    description: GPIO used as Wiegands data-hi line. This line is initially
> +      pulled up to high value. Wiegand write of a bit of value 1 results in
> +      this line being pulled down for pulse length duration.
> +
> +  data-lo-gpios:
> +    maxItems: 1
> +    description: GPIO used as Wiegands data-lo line. This line is initially
> +      pulled up to high value. Wiegand write of a bit of value 0 results in
> +      this line being pulled down for pulse length duration.
> +
> +  pulse-len-us:
> +    maxItems: 1
> +    description: Length of the low pulse in microseconds.
> +
> +  interval-len-us:
> +    maxItems: 1
> +    description: Length of a whole bit (both the pulse and the high phase)
> +      in microseconds.
> +
> +  frame-gap-us:
> +    maxItems: 1
> +    description: Length of the last bit of a frame (both the pulse and the
> +      high phase) in microseconds.
> +
> +  slave-device:

A slave device should be named for the class of device it is, not 
'slave-device'. Just drop all of this as there's no way to match a node 
name.

> +    type: object
> +
> +    properties:
> +      compatible:
> +        description:
> +          Compatible of the Wiegand device.
> +
> +    required:
> +      - compatible
> +
> +required:
> +  - compatible
> +  - pulse-len-us
> +  - interval-len-us
> +  - frame-gap-us
> +
> +additionalProperties: false

Common bindings should be 'true'.

> +
> +examples:
> +  - |
> +    wiegand@f00 {
> +        compatible = "wiegand-gpio";
> +        data-hi-gpios = <&gpio2 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +        data-lo-gpios = <&gpio2 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +        pulse-len-us = <50>;
> +        interval-len-us = <2000>;
> +        frame-gap-us = <2000>;
> +        status = "okay";

Drop 'status'.

> +
> +        wiegand-foo-device {
> +                compatible = "wiegand-foo";
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7f86d02cb427..db9624d93af0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22428,6 +22428,11 @@ L:	linux-input@vger.kernel.org
>  S:	Maintained
>  F:	drivers/hid/hid-wiimote*
>  
> +WIEGAND BUS DRIVER
> +M:	Martin Zaťovič <m.zatovic1@gmail.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
> +
>  WILOCITY WIL6210 WIRELESS DRIVER
>  L:	linux-wireless@vger.kernel.org
>  S:	Orphan
> -- 
> 2.38.1
> 
>
Krzysztof Kozlowski Jan. 22, 2023, 2:21 p.m. UTC | #4
On 04/01/2023 14:34, Martin Zaťovič wrote:
> A Weigand bus is defined by a Wiegand controller node. This node
> can contain one or more device nodes for devices attached to
> the controller(it is advised to only connect one device as Wiegand
> is a point-to-point bus).
> 
> Wiegand controller needs to specify several attributes such as
> the pulse length in order to function properly. These attributes
> are documented here.
> 
> Signed-off-by: Martin Zaťovič <m.zatovic1@gmail.com>
> ---
>  .../bindings/wiegand/wiegand-controller.yaml  | 83 +++++++++++++++++++

You still miss bindings using this common properties which will document
"wiegand-gpio".

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml b/Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
new file mode 100644
index 000000000000..645306c65d43
--- /dev/null
+++ b/Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
@@ -0,0 +1,83 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/wiegand/wiegand-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Wiegand Controller Generic Binding
+
+maintainers:
+  - Martin Zaťovič <martin.zatovic@tbs-biometrics.com>
+
+description: |
+  Wiegand busses can be described with a node for the Wiegand controller device
+  and a set of child nodes for each SPI slave on the bus.
+
+properties:
+  $nodename:
+    pattern: "^wiegand(@.*|-[0-9a-f])*$"
+
+  compatible:
+    maxItems: 1
+    description: Compatible of the Wiegand controller.
+
+  data-high-gpios:
+    maxItems: 1
+    description: GPIO used as Wiegands data-hi line. This line is initially
+      pulled up to high value. Wiegand write of a bit of value 1 results in
+      this line being pulled down for pulse length duration.
+
+  data-lo-gpios:
+    maxItems: 1
+    description: GPIO used as Wiegands data-lo line. This line is initially
+      pulled up to high value. Wiegand write of a bit of value 0 results in
+      this line being pulled down for pulse length duration.
+
+  pulse-len-us:
+    maxItems: 1
+    description: Length of the low pulse in microseconds.
+
+  interval-len-us:
+    maxItems: 1
+    description: Length of a whole bit (both the pulse and the high phase)
+      in microseconds.
+
+  frame-gap-us:
+    maxItems: 1
+    description: Length of the last bit of a frame (both the pulse and the
+      high phase) in microseconds.
+
+  slave-device:
+    type: object
+
+    properties:
+      compatible:
+        description:
+          Compatible of the Wiegand device.
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - pulse-len-us
+  - interval-len-us
+  - frame-gap-us
+
+additionalProperties: false
+
+examples:
+  - |
+    wiegand@f00 {
+        compatible = "wiegand-gpio";
+        data-hi-gpios = <&gpio2 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+        data-lo-gpios = <&gpio2 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+        pulse-len-us = <50>;
+        interval-len-us = <2000>;
+        frame-gap-us = <2000>;
+        status = "okay";
+
+        wiegand-foo-device {
+                compatible = "wiegand-foo";
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 7f86d02cb427..db9624d93af0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22428,6 +22428,11 @@  L:	linux-input@vger.kernel.org
 S:	Maintained
 F:	drivers/hid/hid-wiimote*
 
+WIEGAND BUS DRIVER
+M:	Martin Zaťovič <m.zatovic1@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
+
 WILOCITY WIL6210 WIRELESS DRIVER
 L:	linux-wireless@vger.kernel.org
 S:	Orphan