From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BEED4C76196 for ; Thu, 6 Apr 2023 11:52:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238591AbjDFLwB (ORCPT ); Thu, 6 Apr 2023 07:52:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42092 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238427AbjDFLvo (ORCPT ); Thu, 6 Apr 2023 07:51:44 -0400 Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [IPv6:2a00:1450:4864:20::12c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EBB4FBBBF for ; Thu, 6 Apr 2023 04:47:51 -0700 (PDT) Received: by mail-lf1-x12c.google.com with SMTP id j11so50399022lfg.13 for ; Thu, 06 Apr 2023 04:47:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1680781625; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=ogJ/iJAqSaS/zSC8L1G8ImZfzxS+naCeVLjjGSutqCE=; b=U8Lif6LA08RsJIxJzKr9zxmGoIkFbKs+aUv+zLd1DzZO97BSuaV3QtGayaHfoI0UmO n8Kj2gFwZ7zHctV6yhYXHdzSqH9+jMEPV7oyYz4T4MKhS1GdiGML/wIP71cNWGmrLCyA FXgp/JTfnnEhp/hexWhwPT7XwikLQrBoeTEYLHFr5GWKLNRsLniI6MXUx60parJS41Xe mO/6B5qIzl3iMBdO8rkukEwoR6apucpDlr/KPPGSeI8oAxQVuzJmCQ8JEI9wGBw1Z+E1 2agnLBqV4WuW1Nc9vameX6JjO2f8YaCddLjb3PPP2X4lKrnFMHKEb+eLKp+j041qRab5 tAWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680781625; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ogJ/iJAqSaS/zSC8L1G8ImZfzxS+naCeVLjjGSutqCE=; b=TP8uVOSPbESayJPqr2JQmTX/TKakJfeva4c95JfxE9n4zrTkakRAbyCa52nNPR3svT Efsg2RRXFPKI3tM0D2AZ1UO/y4fuS5dRLQDujg9GNQhH82MgBtkjN9YMu940y50JpaS5 QtuLNLAuDp5GIex6vyVQ9sJ40Rj7JSV8iLDhf2sKmPCMCjo+0UL0wr24csCl5JGnX/kj /ssmobNAAqp72CCFusHNTxa1+Ho5EAoc7U73gwOHQHexu5TSeWwn595JD8S5988QO2J5 f1dsSvfLnjGSucwKQZhgUiW9mJWD4pcR29yamtnqPT3qpLF72N9+Ls0nixoZo47ENig8 gB2w== X-Gm-Message-State: AAQBX9e3I/2U9EBtiNil5SPK4cVLU81SX3jvf5dTSmRI4P1nExQqcL5V kP2/4rniwZ81E4vkF7NF/hVvFQ== X-Google-Smtp-Source: AKy350Zg8IqL+dbtmvUpVIsMu3tmChZ3XbiSvQNMcWk2himJk6XxZdUDJ6r/hlzL13FYXWIsVOPZXA== X-Received: by 2002:a05:6512:24d:b0:4eb:b28:373e with SMTP id b13-20020a056512024d00b004eb0b28373emr2702397lfo.61.1680781625099; Thu, 06 Apr 2023 04:47:05 -0700 (PDT) Received: from [192.168.1.101] (abxh37.neoplus.adsl.tpnet.pl. [83.9.1.37]) by smtp.gmail.com with ESMTPSA id 3-20020ac25683000000b004eb0d0e0207sm232426lfr.31.2023.04.06.04.47.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 06 Apr 2023 04:47:04 -0700 (PDT) Message-ID: <5f7f4201-0871-00b2-be0a-fe68f5de611c@linaro.org> Date: Thu, 6 Apr 2023 13:47:02 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: [PATCH 1/2] dt-bindings: net: Convert ATH10K to YAML Content-Language: en-US To: Krzysztof Kozlowski , Kalle Valo , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Andy Gross , Bjorn Andersson Cc: Marijn Suijten , ath10k@lists.infradead.org, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org References: <20230406-topic-ath10k_bindings-v1-0-1ef181c50236@linaro.org> <20230406-topic-ath10k_bindings-v1-1-1ef181c50236@linaro.org> <48a6b0a3-b441-a84d-e321-b9a773743878@linaro.org> From: Konrad Dybcio In-Reply-To: <48a6b0a3-b441-a84d-e321-b9a773743878@linaro.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> + >> +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 >> + #include >> + >> + 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 = , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + ; >> + 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 >> + >> + 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 = , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + ; >> + 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 >