2022-10-05 15:46:16

by Martin Zaťovič

[permalink] [raw]
Subject: [RFCv2 PATCH 1/4] dt-bindings: bus: add Wiegand bus dt documentation

This patch documents the devicetree entry for enabling Wiegand
bus driver. The drivers that will use Wiegand bus driver shall
create a sub-node of the documented node.

Signed-off-by: Martin Zaťovič <[email protected]>
---
Hello again,

this is the second round of RFC patches in an attempt to add Wiegand
driver to linux kernel. Thank you for all the issues you have pointed
out in the first round. I have tried to fix all of them and I have
also implemented a Wiegand bus driver, that is now used by the GPIO
driver itself - as suggested by Linus.

Any advice you have for me regarding the patches will be appreciated!

With regards,
Martin Zaťovič
---
.../devicetree/bindings/bus/wiegand.yaml | 50 +++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/bus/wiegand.yaml

diff --git a/Documentation/devicetree/bindings/bus/wiegand.yaml b/Documentation/devicetree/bindings/bus/wiegand.yaml
new file mode 100644
index 000000000000..1ed863ab925c
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/wiegand.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/wiegand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Wiegand Bus
+
+maintainers:
+ - Martin Zaťovič <[email protected]>
+
+description: |
+ Wiegand interface is a wiring standard popularized in the 1980s. To this day
+ many card readers, fingerprint readers, sensors, etc. use Wiegand interface
+ particularly for access control applications. It utilizes two wires to
+ transmit the data - D0 and D1.
+
+ Both data lines are initially pulled up. To send a bit of value 1, the D1
+ line is set low. Similarly to send a bit of value 0, the D0 line is set low.
+
+properties:
+ $nodename:
+ pattern: "^wiegand(@[0-9a-f]+)?$"
+
+ compatible:
+ contains:
+ const: wiegand
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ wiegand {
+ compatible = "wiegand";
+
+ wiegand-gpio {
+ compatible = "wiegand,wiegand-gpio";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart2_wiegand>;
+ data-hi-gpios = <&gpio 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+ data-lo-gpios = <&gpio 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+ };
+ };
+
+...
--
2.37.3


2022-10-06 02:19:58

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [RFCv2 PATCH 1/4] dt-bindings: bus: add Wiegand bus dt documentation

On Wed, 05 Oct 2022 16:57:43 +0200, Martin Zaťovič wrote:
> This patch documents the devicetree entry for enabling Wiegand
> bus driver. The drivers that will use Wiegand bus driver shall
> create a sub-node of the documented node.
>
> Signed-off-by: Martin Zaťovič <[email protected]>
> ---
> Hello again,
>
> this is the second round of RFC patches in an attempt to add Wiegand
> driver to linux kernel. Thank you for all the issues you have pointed
> out in the first round. I have tried to fix all of them and I have
> also implemented a Wiegand bus driver, that is now used by the GPIO
> driver itself - as suggested by Linus.
>
> Any advice you have for me regarding the patches will be appreciated!
>
> With regards,
> Martin Zaťovič
> ---
> .../devicetree/bindings/bus/wiegand.yaml | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/bus/wiegand.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:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/wiegand.example.dtb: wiegand: 'wiegand-gpio' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/wiegand.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/wiegand.example.dtb: wiegand: wiegand-gpio: {'compatible': ['wiegand,wiegand-gpio'], 'pinctrl-names': ['default'], 'pinctrl-0': [[4294967295]], 'data-hi-gpios': [[4294967295, 7, 6]], 'data-lo-gpios': [[4294967295, 6, 6]]} is not of type 'array'
From schema: /usr/local/lib/python3.10/dist-packages/dtschema/schemas/gpio/gpio-consumer.yaml
Documentation/devicetree/bindings/bus/wiegand.example.dtb:0:0: /example-0/wiegand/wiegand-gpio: failed to match any schema with compatible: ['wiegand,wiegand-gpio']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

2022-10-06 08:56:28

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFCv2 PATCH 1/4] dt-bindings: bus: add Wiegand bus dt documentation

On 05/10/2022 16:57, Martin Zaťovič wrote:
> This patch documents the devicetree entry for enabling Wiegand
> bus driver. The drivers that will use Wiegand bus driver shall
> create a sub-node of the documented node.
>
> Signed-off-by: Martin Zaťovič <[email protected]>
> ---
> Hello again,
>
> this is the second round of RFC patches in an attempt to add Wiegand
> driver to linux kernel. Thank you for all the issues you have pointed
> out in the first round. I have tried to fix all of them and I have
> also implemented a Wiegand bus driver, that is now used by the GPIO
> driver itself - as suggested by Linus.
>
> Any advice you have for me regarding the patches will be appreciated!
>
> With regards,
> Martin Zaťovič
> ---
> .../devicetree/bindings/bus/wiegand.yaml | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/bus/wiegand.yaml
>
> diff --git a/Documentation/devicetree/bindings/bus/wiegand.yaml b/Documentation/devicetree/bindings/bus/wiegand.yaml
> new file mode 100644
> index 000000000000..1ed863ab925c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/wiegand.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/wiegand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wiegand Bus
> +
> +maintainers:
> + - Martin Zaťovič <[email protected]>
> +
> +description: |
> + Wiegand interface is a wiring standard popularized in the 1980s. To this day
> + many card readers, fingerprint readers, sensors, etc. use Wiegand interface
> + particularly for access control applications. It utilizes two wires to
> + transmit the data - D0 and D1.
> +
> + Both data lines are initially pulled up. To send a bit of value 1, the D1
> + line is set low. Similarly to send a bit of value 0, the D0 line is set low.
> +
> +properties:
> + $nodename:
> + pattern: "^wiegand(@[0-9a-f]+)?$"
> +
> + compatible:
> + contains:
> + const: wiegand
> +

If the bus uses two wires, shouldn't you describe them here? Otherwise
what wires are you using?

> +required:
> + - compatible
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + wiegand {
> + compatible = "wiegand";
> +
> + wiegand-gpio {
> + compatible = "wiegand,wiegand-gpio";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart2_wiegand>;
> + data-hi-gpios = <&gpio 7 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> + data-lo-gpios = <&gpio 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> + };
> + };
> +
> +...

Best regards,
Krzysztof

2022-10-06 09:21:14

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFCv2 PATCH 1/4] dt-bindings: bus: add Wiegand bus dt documentation

On 05/10/2022 16:57, Martin Zaťovič wrote:
> This patch documents the devicetree entry for enabling Wiegand
> bus driver. The drivers that will use Wiegand bus driver shall
> create a sub-node of the documented node.
>
> Signed-off-by: Martin Zaťovič <[email protected]>
> ---
> Hello again,
>

Missing cover letter and changelog.

Best regards,
Krzysztof