Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3e1ee336-1c78-7719-826c-2a093a20ee8e@kernel.org>
Date:   Mon, 21 Feb 2022 17:47:34 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH 2/3] dt-bindings: remoteproc: Add AVM WASP
Content-Language: en-US
To:     Daniel Kestrel <kestrelseventyfour@gmail.com>
Cc:     Bjorn Andersson <bjorn.andersson@linaro.org>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220221135351.GA7342@ubuntu>
From:   Krzysztof Kozlowski <krzk@kernel.org>
In-Reply-To: <20220221135351.GA7342@ubuntu>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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