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

From: Krzysztof Kozlowski
Date: Thu Apr 06 2023 - 04:06:29 EST

Next message: Mukesh Ojha: "Re: [PATCH net-next] net: qrtr: correct types of trace event parameters"
Previous message: Leonardo Bras Soares Passos: "Re: [RFC PATCH v3 0/6] Deduplicating RISCV cmpxchg.h macros"
In reply to: Konrad Dybcio: "[PATCH 1/2] dt-bindings: net: Convert ATH10K to YAML"
Next in thread: Konrad Dybcio: "Re: [PATCH 1/2] dt-bindings: net: Convert ATH10K to YAML"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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.
>

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: Mukesh Ojha: "Re: [PATCH net-next] net: qrtr: correct types of trace event parameters"
Previous message: Leonardo Bras Soares Passos: "Re: [RFC PATCH v3 0/6] Deduplicating RISCV cmpxchg.h macros"
In reply to: Konrad Dybcio: "[PATCH 1/2] dt-bindings: net: Convert ATH10K to YAML"
Next in thread: Konrad Dybcio: "Re: [PATCH 1/2] dt-bindings: net: Convert ATH10K to YAML"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]