Re: [PATCH 1/2] dt-bindings: net: Convert ATH10K to YAML

From: Konrad Dybcio
Date: Thu Apr 06 2023 - 07:52:03 EST

Next message: Javier Martinez Canillas: "Re: [PATCH 1/2] drm/vkms: Drop vkms_connector_destroy() wrapper"
Previous message: Andreas Kemnade: "Re: [PATCH 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c LED driver"
In reply to: Krzysztof Kozlowski: "Re: [PATCH 1/2] dt-bindings: net: Convert ATH10K to YAML"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 6.04.2023 10:06, Krzysztof Kozlowski wrote:
> On 06/04/2023 02:59, Konrad Dybcio wrote:
>> Convert the ATH10K bindings to YAML.
>>
>> Dropped properties that are absent at the current state of mainline:
>> - qcom,msi_addr
>> - qcom,msi_base
>>
>> qcom,coexist-support and qcom,coexist-gpio-pin do very little and should
>> be reconsidered on the driver side, especially the latter one.
>>
>> Somewhat based on the ath11k bindings.
>>
Ack to everything, thanks

Konrad
>
> Thank you for your patch. There is something to discuss/improve.
>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml
>> new file mode 100644
>> index 000000000000..2ff004e404d9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml
>> @@ -0,0 +1,357 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/wireless/qcom,ath10k.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Technologies ATH10K wireless devices
>> +
>> +maintainers:
>> + - Kalle Valo <kvalo@xxxxxxxxxx>
>> +
>> +description: |
>
> Do not need '|'.
>
>> + Qualcomm Technologies, Inc. IEEE 802.11ac devices.
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - qcom,ath10k # SDIO-based devices
>> + - qcom,ipq4019-wifi
>> + - qcom,wcn3990-wifi # SNoC-based devices
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + reg-names:
>> + items:
>> + - const: membase
>> +
>> + interrupts:
>> + minItems: 12
>> + maxItems: 17
>> +
>> + interrupt-names:
>> + minItems: 12
>> + maxItems: 17
>> +
>> + memory-region:
>> + maxItems: 1
>> + description:
>> + Reference to the MSA memory region used by the Wi-Fi firmware
>> + running on the Q6 core.
>> +
>> + iommus:
>> + minItems: 1
>> + maxItems: 2
>> +
>> + clocks:
>> + minItems: 1
>> + maxItems: 3
>> +
>> + clock-names:
>> + minItems: 1
>> + maxItems: 3
>> +
>> + resets:
>> + minItems: 6
>
> Drop minItems here.
>
>> + maxItems: 6
>> +
>> + reset-names:
>> + items:
>> + - const: wifi_cpu_init
>> + - const: wifi_radio_srif
>> + - const: wifi_radio_warm
>> + - const: wifi_radio_cold
>> + - const: wifi_core_warm
>> + - const: wifi_core_cold
>> +
>> + ext-fem-name:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + description: Name of external front end module used.
>> + items:
>
> Drop items (it's just enum)
>
>> + enum:
>> + - microsemi-lx5586
>> + - sky85703-11
>> + - sky85803
>> +
>> + wifi-firmware:
>> + type: object
>
> additionalProperties: false
>
>> + description: |
>> + The ATH10K Wi-Fi node can contain one optional firmware subnode.
>> + Firmware subnode is needed when the platform does not have Trustzone.
>
> properties:
> iommus:
> maxItems: 1
>
>> + required:
>> + - iommus
>> +
>> + qcom,ath10k-calibration-data:
>> + $ref: /schemas/types.yaml#/definitions/uint8-array
>> + description:
>> + Calibration data + board-specific data as a byte array. The length
>> + can vary between hardware versions.
>> +
>> + qcom,ath10k-calibration-variant:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + description:
>> + Unique variant identifier of the calibration data in board-2.bin
>> + for designs with colliding bus and device specific ids
>> +
>> + qcom,ath10k-pre-calibration-data:
>> + $ref: /schemas/types.yaml#/definitions/uint8-array
>> + description:
>> + Pre-calibration data as a byte array. The length can vary between
>> + hardware versions.
>> +
>> + qcom,coexist-support:
>> + $ref: /schemas/types.yaml#/definitions/uint8
>
> enum: [0, 1]
>
>> + description:
>> + 0 or 1 to indicate coex support by the hardware.
>> +
>> + qcom,coexist-gpio-pin:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + COEX GPIO number provided to the Wi-Fi firmware.
>> +
>> + qcom,msa-fixed-perm:
>> + type: boolean
>> + description:
>> + Whether to skip executing an SCM call that reassigns the memory
>> + region ownership.
>> +
>> + qcom,smem-states:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: State bits used by the AP to signal the WLAN Q6.
>> + items:
>> + - description: Signal bits used to enable/disable low power mode
>> + on WCN in the case of WoW (Wake on Wireless).
>> +
>> + qcom,smem-state-names:
>> + description: The names of the state bits used for SMP2P output.
>> + items:
>> + - const: wlan-smp2p-out
>> +
>> + qcom,snoc-host-cap-8bit-quirk:
>> + type: boolean
>> + description:
>> + Quirk specifying that the firmware expects the 8bit version
>> + of the host capability QMI request
>> +
>> + qcom,xo-cal-data:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + XO cal offset to be configured in XO trim register.
>> +
>> + vdd-0.8-cx-mx-supply:
>> + description: Main logic power rail
>> +
>> + vdd-1.8-xo-supply:
>> + description: Crystal oscillator supply
>> +
>> + vdd-1.3-rfa-supply:
>> + description: RFA supply
>> +
>> + vdd-3.3-ch0-supply:
>> + description: Primary Wi-Fi antenna supply
>> +
>> + vdd-3.3-ch1-supply:
>> + description: Secondary Wi-Fi antenna supply
>> +
>> +required:
>> + - compatible
>> + - reg
>> +
>> +additionalProperties: false
>> +
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,ipq4019-wifi
>> + then:
>> + properties:
>> + interrupts:
>> + minItems: 17
>> + maxItems: 17
>> +
>> + interrupt-names:
>> + minItems: 17
>> + items:
>> + - const: msi0
>> + - const: msi1
>> + - const: msi2
>> + - const: msi3
>> + - const: msi4
>> + - const: msi5
>> + - const: msi6
>> + - const: msi7
>> + - const: msi8
>> + - const: msi9
>> + - const: msi10
>> + - const: msi11
>> + - const: msi12
>> + - const: msi13
>> + - const: msi14
>> + - const: msi15
>> + - const: legacy
>> +
>> + clocks:
>> + items:
>> + - description: Wi-Fi command clock
>> + - description: Wi-Fi reference clock
>> + - description: Wi-Fi RTC clock
>> +
>> + clock-names:
>> + items:
>> + - const: wifi_wcss_cmd
>> + - const: wifi_wcss_ref
>> + - const: wifi_wcss_rtc
>> +
>> + required:
>> + - clocks
>> + - clock-names
>> + - interrupts
>> + - interrupt-names
>> + - resets
>> + - reset-names
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,wcn3990-wifi
>> +
>> + then:
>> + properties:
>> + clocks:
>> + minItems: 1
>> + items:
>> + - description: XO reference clock
>> + - description: Qualcomm Debug Subsystem clock
>> +
>> + clock-names:
>> + minItems: 1
>> + items:
>> + - const: cxo_ref_clk_pin
>> + - const: qdss
>> +
>> + interrupts:
>> + items:
>> + - description: CE0
>> + - description: CE1
>> + - description: CE2
>> + - description: CE3
>> + - description: CE4
>> + - description: CE5
>> + - description: CE6
>> + - description: CE7
>> + - description: CE8
>> + - description: CE9
>> + - description: CE10
>> + - description: CE11
>> +
>> + required:
>> + - interrupts
>> +
>> +examples:
>> + # SNoC
>> + - |
>> + #include <dt-bindings/clock/qcom,rpmcc.h>
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + wlan_msa_mem: memory@4cd000 {
>> + no-map;
>> + reg = <0x0 0x004cd000 0x0 0x1000>;
>> + };
>> + };
>
> Drop the reserved memory node, it's more-or-less obvious (same everywhere).
>
>> +
>> + wifi: wifi@18800000 {
>
> Drop label.
>
>> + compatible = "qcom,wcn3990-wifi";
>> + reg = <0x18800000 0x800000>;
>> + reg-names = "membase";
>> + memory-region = <&wlan_msa_mem>;
>> + clocks = <&rpmcc RPM_SMD_RF_CLK2_PIN>;
>> + clock-names = "cxo_ref_clk_pin";
>> + interrupts = <GIC_SPI 413 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 414 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 415 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>;
>> + iommus = <&anoc2_smmu 0x1900>,
>> + <&anoc2_smmu 0x1901>;
>> + qcom,snoc-host-cap-8bit-quirk;
>> + status = "disabled";
>
> Drop status from examples.
>
> The example looks a bit incomplete. Please add supplies and
> wifi-firmware node.
>
>> + };
>> +
>> + # AHB
>> + - |
>> + #include <dt-bindings/clock/qcom,gcc-ipq4019.h>
>> +
>> + wifi0: wifi@a000000 {
>
> Drop label.
>
>> + compatible = "qcom,ipq4019-wifi";
>> + reg = <0xa000000 0x200000>;
>> + resets = <&gcc WIFI0_CPU_INIT_RESET>,
>> + <&gcc WIFI0_RADIO_SRIF_RESET>,
>> + <&gcc WIFI0_RADIO_WARM_RESET>,
>> + <&gcc WIFI0_RADIO_COLD_RESET>,
>> + <&gcc WIFI0_CORE_WARM_RESET>,
>> + <&gcc WIFI0_CORE_COLD_RESET>;
>> + reset-names = "wifi_cpu_init",
>> + "wifi_radio_srif",
>> + "wifi_radio_warm",
>> + "wifi_radio_cold",
>> + "wifi_core_warm",
>> + "wifi_core_cold";
>> + clocks = <&gcc GCC_WCSS2G_CLK>,
>> + <&gcc GCC_WCSS2G_REF_CLK>,
>> + <&gcc GCC_WCSS2G_RTC_CLK>;
>> + clock-names = "wifi_wcss_cmd",
>> + "wifi_wcss_ref",
>> + "wifi_wcss_rtc";
>> + interrupts = <GIC_SPI 32 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 33 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 36 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 37 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 38 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 39 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 40 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 41 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 42 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 43 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 44 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 45 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 46 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 47 IRQ_TYPE_EDGE_RISING>,
>> + <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "msi0",
>> + "msi1",
>> + "msi2",
>> + "msi3",
>> + "msi4",
>> + "msi5",
>> + "msi6",
>> + "msi7",
>> + "msi8",
>> + "msi9",
>> + "msi10",
>> + "msi11",
>> + "msi12",
>> + "msi13",
>> + "msi14",
>> + "msi15",
>> + "legacy";
>> + status = "disabled";
>
> Ditto
>
>> + };
>>
>
> Best regards,
> Krzysztof
>

Next message: Javier Martinez Canillas: "Re: [PATCH 1/2] drm/vkms: Drop vkms_connector_destroy() wrapper"
Previous message: Andreas Kemnade: "Re: [PATCH 2/2] leds: bd2606mvv: Driver for the Rohm 6 Channel i2c LED driver"
In reply to: Krzysztof Kozlowski: "Re: [PATCH 1/2] dt-bindings: net: Convert ATH10K to YAML"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]