Re: [PATCHv5 1/4] dt-bindings: wiegand: add Wiegand controller common properties

[Rob Herring]

On Thu, Aug 24, 2023 at 01:10:12PM +0200, Martin Zaťovič wrote:
> Wiegand 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).

How would multiple devices work? Seems like you'd need some sort of mux 
which would be another node. If there's not really any need, then I'd 
just say it's only a single device. Either way the binding needs to 
define this.

> 
> Wiegand controller needs to specify several attributes such as
> the pulse length in order to function properly. These attributes
> are documented here.
> 
> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> Signed-off-by: Martin Zaťovič <m.zatovic1@xxxxxxxxx>
> ---
>  .../bindings/wiegand/wiegand-controller.yaml  | 39 +++++++++++++++++++
>  MAINTAINERS                                   |  5 +++
>  2 files changed, 44 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..8f36287e4fed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/wiegand/wiegand-controller.yaml
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: (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 Generic Controller Common Properties
> +
> +maintainers:
> +  - Martin Zaťovič <m.zatovic1@xxxxxxxxx>
> +
> +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.

Some more detail on what Wiegand is would be useful. Link to spec? What 
it is used for?

> +
> +properties:
> +  $nodename:
> +    pattern: "^wiegand(@.*|-[0-9a-f])?$"

The '-[0-9a-f]' suffix should be decimal rather than hex.

> +
> +  pulse-len-us:
> +    description:
> +      Length of the low pulse in microseconds.
> +
> +  interval-len-us:
> +    description:
> +      Length of a whole bit (both the pulse and the high phase) in microseconds.
> +
> +  frame-gap-us:
> +    description:
> +      Length of the last bit of a frame (both the pulse and the high phase) in
> +      microseconds.

If you have multiple devices, you need to define the child node format. 
Specifically, you need addresses for multiple devices. So you need to 
define the unit-address format, #address-cells and #size-cells values, 
and any constraints on 'reg' such as max address and/or number of entries.

Rob