Re: [RFC PATCH v2 1/2] dt-bindings: wifi: Add support for Nordic nRF70

[Krzysztof Kozlowski]

On Tue, Apr 22, 2025 at 07:59:17PM GMT, Artur Rojek wrote:
> Add a documentation file to describe the Device Tree bindings for the
> Nordic Semiconductor nRF70 series wireless companion IC.
> 
> Signed-off-by: Artur Rojek <artur@xxxxxxxxxxxxx>
> ---
> 
> v2: - rename the patch subject to comply with DT submission rules 
>     - fix a typo in reg property name and correct its indentation
>     - replace all gpio based properties as follows:
>       - irq-gpios with interrupts/interrupt-names
>       - bucken-gpios/iovdd-gpios with vpwr-supply/vio-supply
>     - clarify usage of said properties in their descriptions
>     - add a reference to spi-peripheral-props.yaml#
>     - specify unevaluatedProperties
>     - drop unused voltage-ranges property
>     - update bindings example accordingly w/ above changes
> 
>  .../bindings/net/wireless/nordic,nrf70.yaml   | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/nordic,nrf70.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/nordic,nrf70.yaml b/Documentation/devicetree/bindings/net/wireless/nordic,nrf70.yaml
> new file mode 100644
> index 000000000000..c9a41b61c624
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/nordic,nrf70.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/wireless/nordic,nrf70.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nordic Semiconductor nRF70 series wireless companion IC

What is a wireless companion? You miss description - see example-schema.

> +
> +maintainers:
> +  - Artur Rojek <artur@xxxxxxxxxxxxxxx>
> +
> +properties:
> +  compatible:
> +    const: nordic,nrf70
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    description: HOST_IRQ line, used for host processor interrupt requests.

Drop description, obvious.

> +
> +  interrupt-names:
> +    description: Name for the HOST_IRQ line. This must be set to "host-irq".
> +    const: host-irq

Drop interrupt-names, not really useful for one entry.

> +
> +  vpwr-supply:
> +    description: BUCKEN line, used for PWR IP state control.
> +
> +  vio-supply:
> +    description: IOVDD line, used for I/O pins voltage control. Optional.

Drop "Optional". Don't repeat constraints in free form text.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - vpwr-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#

Missing ref to ieee802/wireless devices schema (wireless recently
merged), assuming this is WiFi.

Neither commit msg nor description explains me what this is. Both
or at least one should.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    reg_nrf70_buck: regulator-nrf70-buck {

Drop this node, not relevant.

> +        compatible = "regulator-fixed";
> +        regulator-name = "nrf70_buck";
> +        gpio = <&gpio2 24 GPIO_ACTIVE_HIGH>;
> +        enable-active-high;
> +    };
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        nrf7002@0 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

WiFi is usually "wifi".

Best regards,
Krzysztof