Re: [PATCH 2/3] dt-bindings: remoteproc: Add AVM WASP

[Krzysztof Kozlowski <krzk@kernel.org>]

On 21/02/2022 14:53, Daniel Kestrel wrote:
> AVM Fritzbox router boards may contain an additional ATH79
> based SoC that has the wifi cards connected.
> This patch adds bindings for this remote processor.
> 
> Signed-off-by: Daniel Kestrel <kestrelseventyfour@gmail.com>
> ---
>  .../bindings/remoteproc/avm,wasp-rproc.yaml   | 93 +++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/avm,wasp-rproc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/avm,wasp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/avm,wasp-rproc.yaml
> new file mode 100644
> index 000000000000..21f3bbcc4202
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/avm,wasp-rproc.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/avm,wasp-rproc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AVM WASP processor controller bindings
> +
> +maintainers:
> +  - Daniel Kestrel <kestrelseventyfour@gmail.com>
> +
> +description: |
> +  This document defines the bindings for the remoteproc component that loads and
> +  boots firmwares on the AVM Wireless Assistent Support Processor (WASP) SoC
> +  that is attached to some AVM Fritzbox devices (3390, 3490, 5490, 5491, 7490).
> +
> +properties:
> +  compatible:
> +    const: avm,wasp
> +
> +  ath9k-firmware:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: |
> +      Should contain the name of the ath9k eeprom that is to be loaded from
> +      the lantiq host flash. Wifi on the WASP SoC does not work without it.
> +      The file should be located on the firmware search path.

Are you sure this is a property of hardware? It looks like runtime
configuration parameter.

> +
> +  ath10k-caldata:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: |
> +      Should contain the name of the ath10k caldata that is to be loaded from
> +      the lantiq host flash. Wifi on the WASP SoC does not work without it.
> +      The file should be located on the firmware search path.

Same.

> +
> +  wasp-netboot-firmware:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: |
> +      Should contain the name of the netboot firmware that is to be loaded
> +      and started on the WASP SoC using mdio in order to be able to load
> +      the initramfs image as a second stage.
> +      The file should be located on the firmware search path.

Same.

> +
> +  wasp-netboot-mdio:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Reference to the Lantiq GSWIP switch mdio.

Vendor prefix.

> +
> +  wasp-initramfs-port:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Reference to the network port, where the WASP SoC is connected to.

Vendor prefix.

> +
> +  wasp-initramfs-image:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: |
> +      Should contain the name of the initramfs linux image that is to be loaded
> +      and started on the WASP SoC.
> +      The file should be located on the firmware search path.

initramfs path looks even less like a property of hardware... If you
change initramfs from CPIO to initrd or GZ, hardware changes as well?

> +  reset-gpio:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: Reference and parameters for the reset gpio of the WASP SoC.

Wrong suffix, unneeded type. Did you run dt_binding_check?

"Reference and parameters" are obvious, so they should be skipped.

> +
> +  startup-gpio:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: Reference and parameters for the power switch gpio of the WASP SoC.

Same.

> +
> +required:
> +  - compatible
> +  - ath9k-firmware
> +  - ath10k-caldata
> +  - wasp-netboot-firmware
> +  - wasp-netboot-mdio
> +  - wasp-initramfs-port
> +  - wasp-initramfs-image
> +  - reset-gpio
> +  - startup-gpio
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    avm-wasp {

Generic node name describing class of a device. AVM is company, WASP is
product, so neither of them are generic.


Best regards,
Krzysztof