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

[Krzysztof Kozlowski]

On 22/02/2022 16:27, Kestrel seventyfour wrote:
> Am Mo., 21. Feb. 2022 um 17:47 Uhr schrieb Krzysztof Kozlowski
> <krzk@xxxxxxxxxx>:
>>
>> 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@xxxxxxxxx>
>>> ---
>>>  .../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@xxxxxxxxx>
>>> +
>>> +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?
> 
> Hi Krzystof,
> 
> Sorry for missing the dt_binding_check.
> I have switched to use devm_gpiod_get and it does not work if the
> suffix is not -gpio
> or -gpios (see of_find_gpio method).
> Would avm,reset-gpio be ok to use here?

No, because the suffix must be -gpios.
These are the legacy, often deprecated properties:
git grep '\-gpio:' -- Documentation/devicetree/ | grep yaml

These are proper:
git grep '\-gpios:' -- Documentation/devicetree/ | grep yaml


> 
> Thanks.
>>
>> "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.
> Is avm,startup-gpio ok, like above?

avm,startup-gpios


Best regards,
Krzysztof