2023-01-04 14:14:41

by Martin Zaťovič

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: add Wiegand controller dt-binding documentation

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č <[email protected]>
---
.../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
+%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č <[email protected]>
+
+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: [email protected]
S: Maintained
F: drivers/hid/hid-wiimote*

+WIEGAND BUS DRIVER
+M: Martin Zaťovič <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
+
WILOCITY WIL6210 WIRELESS DRIVER
L: [email protected]
S: Orphan
--
2.38.1


2023-01-05 03:11:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: add Wiegand controller dt-binding documentation


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č <[email protected]>
> ---
> .../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/[email protected]

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.

2023-01-05 03:29:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: add Wiegand controller dt-binding documentation


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č <[email protected]>
> ---
> .../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/[email protected]

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.

2023-01-08 16:59:04

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: add Wiegand controller dt-binding documentation

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č <[email protected]>
> ---
> .../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č <[email protected]>
> +
> +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: [email protected]
> S: Maintained
> F: drivers/hid/hid-wiimote*
>
> +WIEGAND BUS DRIVER
> +M: Martin Zaťovič <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
> +
> WILOCITY WIL6210 WIRELESS DRIVER
> L: [email protected]
> S: Orphan
> --
> 2.38.1
>
>

2023-01-22 14:21:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: add Wiegand controller dt-binding documentation

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č <[email protected]>
> ---
> .../bindings/wiegand/wiegand-controller.yaml | 83 +++++++++++++++++++

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

Best regards,
Krzysztof