| Message ID | 20241029173050.2188150-2-quic_rajkbhag@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: ath12k: Add WSI node for QCN9274 in RDP433 for MLO | expand |
On 10/30/2024 12:04 PM, Jeff Johnson wrote: > in the description above you have two different diagrams: > - one that shows 3 pcie* devices in a single group with apparently one port > per device > - one that shows 4 pcie* devices split into two groups of two, again with > apparently one port per device > > but in the representation that follows you describe three pcie* devices, each > with two distinct ports, all 6 of which are part of group 0. > > can we have diagrams that match the actual bindings. does the real product > actually have 6 ports in one group? After stepping away and then coming back and reading the dts change I now understand that each device has two ports, a tx and an rx port. /jeff
On 10/29/2024 11:22 PM, Krzysztof Kozlowski wrote: > On 29/10/2024 18:30, Raj Kumar Bhagat wrote: >> QCN9274 device has WSI support. WSI stands for WLAN Serial Interface. >> It is used for the exchange of specific control information across >> radios based on the doorbell mechanism. This WSI connection is >> essential to exchange control information among these devices >> >> Hence, describe WSI interface supported in QCN9274 with the following >> properties: >> >> - qcom,wsi-group-id: It represents the identifier assigned to the WSI >> connection. All the ath12k devices connected to same WSI connection >> have the same wsi-group-id. >> >> - qcom,wsi-master: Indicates if this device is the WSI master. >> >> - ports: This is a graph ports schema that has two ports: TX (port@0) >> and RX (port@1). This represents the actual WSI connection among >> multiple devices. > > Describe the hardware, not the contents of the patch/binding. We see it > easily, but what we do not see is the hardware. > sure will update the commit log. >> >> Also, describe the ath12k device property >> "qcom,ath12k-calibration-variant". This is a common property among >> ath12k devices. > > Why do you describe it? What you do is easily visible. We do not see why. > will remove this description in next version. >> >> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com> >> --- >> .../bindings/net/wireless/qcom,ath12k.yaml | 241 +++++++++++++++++- >> 1 file changed, 232 insertions(+), 9 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml >> index 1b5884015b15..42bcd73dd159 100644 >> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml >> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml >> @@ -1,5 +1,6 @@ >> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> # Copyright (c) 2024 Linaro Limited >> +# Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. >> %YAML 1.2 >> --- >> $id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml# >> @@ -18,10 +19,17 @@ properties: >> compatible: >> enum: >> - pci17cb,1107 # WCN7850 >> + - pci17cb,1109 # QCN9274 > > I asked for separate binding because it is quite a different device. > Unless it is not... but then commit msg is quite not precise here. > sure, will create a separate binding, may be "qcom,ath12k_wsi.yaml". This will be for ath21k PCI device with WSI interface. >> >> reg: >> maxItems: 1 >> >> + qcom,ath12k-calibration-variant: >> + $ref: /schemas/types.yaml#/definitions/string >> + description: | > > Do not need '|' unless you need to preserve formatting. > thanks will remove "|" >> + string to uniquely identify variant of the calibration data for designs >> + with colliding bus and device ids >> + >> vddaon-supply: >> description: VDD_AON supply regulator handle >> >> @@ -49,21 +57,100 @@ properties: >> vddpcie1p8-supply: >> description: VDD_PCIE_1P8 supply regulator handle >> >> + wsi: > > Not much improved here. I asked to drop the node. > In next version will remove "wsi". The properties under wsi (ports, qcom,wsi-master, etc) will be directly under ath12k device node. >> + type: object >> + description: | >> + The ath12k devices (QCN9274) feature WSI support. WSI stands for >> + WLAN Serial Interface. It is used for the exchange of specific >> + control information across radios based on the doorbell mechanism. >> + This WSI connection is essential to exchange control information >> + among these devices. >> + >> + Diagram to represent one WSI connection (one WSI group) among >> + three devices. >> + >> + +-------+ +-------+ +-------+ >> + | pcie2 | | pcie3 | | pcie1 | >> + | | | | | | >> + +----->| wsi |------->| wsi |------->| wsi |-----+ >> + | | grp 0 | | grp 0 | | grp 2 | | >> + | +-------+ +-------+ +-------+ | >> + +------------------------------------------------------+ >> + >> + Diagram to represent two WSI connections (two separate WSI groups) >> + among four devices. >> + >> + +-------+ +-------+ +-------+ +-------+ >> + | pcie2 | | pcie3 | | pcie1 | | pcie0 | >> + | | | | | | | | >> + +-->| wsi |--->| wsi |--+ +-->| wsi |--->| wsi |--+ >> + | | grp 0 | | grp 0 | | | | grp 1 | | grp 1 | | >> + | +-------+ +-------+ | | +-------+ +-------+ | >> + +---------------------------+ +---------------------------+ >> + >> + properties: >> + qcom,wsi-group-id: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + It represents the identifier assigned to the WSI connection. All >> + the ath12k devices connected to same WSI connection have the >> + same wsi-group-id. > > That's not needed according to description. Entire group is defined by > graph. > So this mean "qcom,wsi-group-id" to be dropped and we can assign the group ID (in ath12k driver implementation) by using the graph? >> + >> + qcom,wsi-master: >> + type: boolean >> + description: >> + Indicates if this device is the WSI master. >> + > > This copies property name. Why being master is important? > The master device in the WSI group aids (is capable) to synchronize the Timing Synchronization Function (TSF) clock across all devices in the group. Will include this information in next version. > Also, use some different name: see preferred names in kernel coding style. > Thanks for pointing out, will use "qcom,wsi-controller" >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + description: >> + These ports are used to connect multiple WSI supported devices to >> + form the WSI group. >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: >> + This is the TX port of WSI interface. It is attached to the RX >> + port of the next device in the WSI connection. >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: >> + This is the RX port of WSI interface. It is attached to the TX >> + port of the previous device in the WSI connection. >> + >> + required: >> + - qcom,wsi-group-id >> + - ports >> + >> + additionalProperties: false >> + >> required: >> - compatible >> - reg >> - - vddaon-supply >> - - vddwlcx-supply >> - - vddwlmx-supply >> - - vddrfacmn-supply >> - - vddrfa0p8-supply >> - - vddrfa1p2-supply >> - - vddrfa1p8-supply >> - - vddpcie0p9-supply >> - - vddpcie1p8-supply >> >> additionalProperties: false >> >> +allOf: >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - pci17cb,1107 >> + then: >> + required: >> + - vddaon-supply >> + - vddwlcx-supply >> + - vddwlmx-supply >> + - vddrfacmn-supply >> + - vddrfa0p8-supply >> + - vddrfa1p2-supply >> + - vddrfa1p8-supply >> + - vddpcie0p9-supply >> + - vddpcie1p8-supply > > Commit says WSI applies only to new variant, so properties should be > disallowed... or just follow my feedback last time: separate binding. > Sure, we will have separate binding in next version. >> + >> examples: >> - | >> #include <dt-bindings/clock/qcom,rpmh.h> >> @@ -97,3 +184,139 @@ examples: >> }; >> }; >> }; >> + >> + - | >> + pcie1 { > > pcie { > and keep all nodes here > sure >> + #address-cells = <3>; >> + #size-cells = <2>; >> + >> + pcie@0 { >> + device_type = "pci"; >> + reg = <0x0 0x0 0x0 0x0 0x0>; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + ranges; >> + >> + wifi1@0 { > > wifi@ > > Same in other places. > Thanks, will update. >> + compatible = "pci17cb,1109"; >> + reg = <0x0 0x0 0x0 0x0 0x0>; >> + >> + qcom,ath12k-calibration-variant = "RDP433_1"; >> + >> + wsi { > > No resources here? Not a bus? You already got comment about it. > sure will remove wsi node and directly define ports and other properties inside wifi.
On 10/31/2024 12:34 AM, Jeff Johnson wrote: > On 10/29/2024 10:30 AM, Raj Kumar Bhagat wrote: >> QCN9274 device has WSI support. WSI stands for WLAN Serial Interface. >> It is used for the exchange of specific control information across >> radios based on the doorbell mechanism. This WSI connection is >> essential to exchange control information among these devices >> >> Hence, describe WSI interface supported in QCN9274 with the following >> properties: >> >> - qcom,wsi-group-id: It represents the identifier assigned to the WSI >> connection. All the ath12k devices connected to same WSI connection >> have the same wsi-group-id. >> >> - qcom,wsi-master: Indicates if this device is the WSI master. >> >> - ports: This is a graph ports schema that has two ports: TX (port@0) >> and RX (port@1). This represents the actual WSI connection among >> multiple devices. >> >> Also, describe the ath12k device property >> "qcom,ath12k-calibration-variant". This is a common property among >> ath12k devices. >> >> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com> >> --- >> .../bindings/net/wireless/qcom,ath12k.yaml | 241 +++++++++++++++++- >> 1 file changed, 232 insertions(+), 9 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml >> index 1b5884015b15..42bcd73dd159 100644 >> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml >> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml >> @@ -1,5 +1,6 @@ >> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> # Copyright (c) 2024 Linaro Limited >> +# Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. >> %YAML 1.2 >> --- >> $id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml# >> @@ -18,10 +19,17 @@ properties: >> compatible: >> enum: >> - pci17cb,1107 # WCN7850 >> + - pci17cb,1109 # QCN9274 >> >> reg: >> maxItems: 1 >> >> + qcom,ath12k-calibration-variant: >> + $ref: /schemas/types.yaml#/definitions/string >> + description: | >> + string to uniquely identify variant of the calibration data for designs >> + with colliding bus and device ids >> + >> vddaon-supply: >> description: VDD_AON supply regulator handle >> >> @@ -49,21 +57,100 @@ properties: >> vddpcie1p8-supply: >> description: VDD_PCIE_1P8 supply regulator handle >> >> + wsi: >> + type: object >> + description: | >> + The ath12k devices (QCN9274) feature WSI support. WSI stands for >> + WLAN Serial Interface. It is used for the exchange of specific >> + control information across radios based on the doorbell mechanism. >> + This WSI connection is essential to exchange control information >> + among these devices. >> + >> + Diagram to represent one WSI connection (one WSI group) among >> + three devices. >> + >> + +-------+ +-------+ +-------+ >> + | pcie2 | | pcie3 | | pcie1 | > is there a reason to not have these in some order? > This could be made in same order. In next version will update. But in actual hardware the pcie and wsi connection may not be same order. >> + | | | | | | >> + +----->| wsi |------->| wsi |------->| wsi |-----+ >> + | | grp 0 | | grp 0 | | grp 2 | | > s/grp 2/grp 0/??? ^ typo? > Thanks for pointing out, this is a typo. will update in next version. >> + | +-------+ +-------+ +-------+ | >> + +------------------------------------------------------+ >> + >> + Diagram to represent two WSI connections (two separate WSI groups) >> + among four devices. >> + >> + +-------+ +-------+ +-------+ +-------+ >> + | pcie2 | | pcie3 | | pcie1 | | pcie0 | > again seems strange to not have any logical (to me) order Will keep this example diagram in pcie order in next version.
On 04/11/2024 19:44, Raj Kumar Bhagat wrote: >>> $id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml# >>> @@ -18,10 +19,17 @@ properties: >>> compatible: >>> enum: >>> - pci17cb,1107 # WCN7850 >>> + - pci17cb,1109 # QCN9274 >> >> I asked for separate binding because it is quite a different device. >> Unless it is not... but then commit msg is quite not precise here. >> > > sure, will create a separate binding, may be "qcom,ath12k_wsi.yaml". Underscores are not allowed in compatibles and the file name follows compatible name, so use hyphen. >>> + type: object >>> + description: | >>> + The ath12k devices (QCN9274) feature WSI support. WSI stands for >>> + WLAN Serial Interface. It is used for the exchange of specific >>> + control information across radios based on the doorbell mechanism. >>> + This WSI connection is essential to exchange control information >>> + among these devices. >>> + >>> + Diagram to represent one WSI connection (one WSI group) among >>> + three devices. >>> + >>> + +-------+ +-------+ +-------+ >>> + | pcie2 | | pcie3 | | pcie1 | >>> + | | | | | | >>> + +----->| wsi |------->| wsi |------->| wsi |-----+ >>> + | | grp 0 | | grp 0 | | grp 2 | | >>> + | +-------+ +-------+ +-------+ | >>> + +------------------------------------------------------+ >>> + >>> + Diagram to represent two WSI connections (two separate WSI groups) >>> + among four devices. >>> + >>> + +-------+ +-------+ +-------+ +-------+ >>> + | pcie2 | | pcie3 | | pcie1 | | pcie0 | >>> + | | | | | | | | >>> + +-->| wsi |--->| wsi |--+ +-->| wsi |--->| wsi |--+ >>> + | | grp 0 | | grp 0 | | | | grp 1 | | grp 1 | | >>> + | +-------+ +-------+ | | +-------+ +-------+ | >>> + +---------------------------+ +---------------------------+ >>> + >>> + properties: >>> + qcom,wsi-group-id: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + It represents the identifier assigned to the WSI connection. All >>> + the ath12k devices connected to same WSI connection have the >>> + same wsi-group-id. >> >> That's not needed according to description. Entire group is defined by >> graph. >> > > So this mean "qcom,wsi-group-id" to be dropped and we can assign the > group ID (in ath12k driver implementation) by using the graph? Yes > Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml index 1b5884015b15..42bcd73dd159 100644 --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml @@ -1,5 +1,6 @@ # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright (c) 2024 Linaro Limited +# Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. %YAML 1.2 --- $id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml# @@ -18,10 +19,17 @@ properties: compatible: enum: - pci17cb,1107 # WCN7850 + - pci17cb,1109 # QCN9274 reg: maxItems: 1 + qcom,ath12k-calibration-variant: + $ref: /schemas/types.yaml#/definitions/string + description: | + string to uniquely identify variant of the calibration data for designs + with colliding bus and device ids + vddaon-supply: description: VDD_AON supply regulator handle @@ -49,21 +57,100 @@ properties: vddpcie1p8-supply: description: VDD_PCIE_1P8 supply regulator handle + wsi: + type: object + description: | + The ath12k devices (QCN9274) feature WSI support. WSI stands for + WLAN Serial Interface. It is used for the exchange of specific + control information across radios based on the doorbell mechanism. + This WSI connection is essential to exchange control information + among these devices. + + Diagram to represent one WSI connection (one WSI group) among + three devices. + + +-------+ +-------+ +-------+ + | pcie2 | | pcie3 | | pcie1 | + | | | | | | + +----->| wsi |------->| wsi |------->| wsi |-----+ + | | grp 0 | | grp 0 | | grp 2 | | + | +-------+ +-------+ +-------+ | + +------------------------------------------------------+ + + Diagram to represent two WSI connections (two separate WSI groups) + among four devices. + + +-------+ +-------+ +-------+ +-------+ + | pcie2 | | pcie3 | | pcie1 | | pcie0 | + | | | | | | | | + +-->| wsi |--->| wsi |--+ +-->| wsi |--->| wsi |--+ + | | grp 0 | | grp 0 | | | | grp 1 | | grp 1 | | + | +-------+ +-------+ | | +-------+ +-------+ | + +---------------------------+ +---------------------------+ + + properties: + qcom,wsi-group-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + It represents the identifier assigned to the WSI connection. All + the ath12k devices connected to same WSI connection have the + same wsi-group-id. + + qcom,wsi-master: + type: boolean + description: + Indicates if this device is the WSI master. + + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + These ports are used to connect multiple WSI supported devices to + form the WSI group. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: + This is the TX port of WSI interface. It is attached to the RX + port of the next device in the WSI connection. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + This is the RX port of WSI interface. It is attached to the TX + port of the previous device in the WSI connection. + + required: + - qcom,wsi-group-id + - ports + + additionalProperties: false + required: - compatible - reg - - vddaon-supply - - vddwlcx-supply - - vddwlmx-supply - - vddrfacmn-supply - - vddrfa0p8-supply - - vddrfa1p2-supply - - vddrfa1p8-supply - - vddpcie0p9-supply - - vddpcie1p8-supply additionalProperties: false +allOf: + - if: + properties: + compatible: + contains: + enum: + - pci17cb,1107 + then: + required: + - vddaon-supply + - vddwlcx-supply + - vddwlmx-supply + - vddrfacmn-supply + - vddrfa0p8-supply + - vddrfa1p2-supply + - vddrfa1p8-supply + - vddpcie0p9-supply + - vddpcie1p8-supply + examples: - | #include <dt-bindings/clock/qcom,rpmh.h> @@ -97,3 +184,139 @@ examples: }; }; }; + + - | + pcie1 { + #address-cells = <3>; + #size-cells = <2>; + + pcie@0 { + device_type = "pci"; + reg = <0x0 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + ranges; + + wifi1@0 { + compatible = "pci17cb,1109"; + reg = <0x0 0x0 0x0 0x0 0x0>; + + qcom,ath12k-calibration-variant = "RDP433_1"; + + wsi { + qcom,wsi-group-id = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + wifi1_wsi_tx: endpoint { + remote-endpoint = <&wifi2_wsi_rx>; + }; + }; + + port@1 { + reg = <1>; + + wifi1_wsi_rx: endpoint { + remote-endpoint = <&wifi3_wsi_tx>; + }; + }; + }; + }; + }; + }; + }; + + pcie2 { + #address-cells = <3>; + #size-cells = <2>; + + pcie@0 { + device_type = "pci"; + reg = <0x0 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + ranges; + + wifi2@0 { + compatible = "pci17cb,1109"; + reg = <0x0 0x0 0x0 0x0 0x0>; + + qcom,ath12k-calibration-variant = "RDP433_2"; + + wsi { + qcom,wsi-group-id = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + wifi2_wsi_tx: endpoint { + remote-endpoint = <&wifi3_wsi_rx>; + }; + }; + + port@1 { + reg = <1>; + + wifi2_wsi_rx: endpoint { + remote-endpoint = <&wifi1_wsi_tx>; + }; + }; + }; + }; + }; + }; + }; + + pcie3 { + #address-cells = <3>; + #size-cells = <2>; + + pcie@0 { + device_type = "pci"; + reg = <0x0 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + ranges; + + wifi3@0 { + compatible = "pci17cb,1109"; + reg = <0x0 0x0 0x0 0x0 0x0>; + + qcom,ath12k-calibration-variant = "RDP433_3"; + + wsi { + qcom,wsi-group-id = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + wifi3_wsi_tx: endpoint { + remote-endpoint = <&wifi1_wsi_rx>; + }; + }; + + port@1 { + reg = <1>; + + wifi3_wsi_rx: endpoint { + remote-endpoint = <&wifi2_wsi_tx>; + }; + }; + }; + }; + }; + }; + };
QCN9274 device has WSI support. WSI stands for WLAN Serial Interface. It is used for the exchange of specific control information across radios based on the doorbell mechanism. This WSI connection is essential to exchange control information among these devices Hence, describe WSI interface supported in QCN9274 with the following properties: - qcom,wsi-group-id: It represents the identifier assigned to the WSI connection. All the ath12k devices connected to same WSI connection have the same wsi-group-id. - qcom,wsi-master: Indicates if this device is the WSI master. - ports: This is a graph ports schema that has two ports: TX (port@0) and RX (port@1). This represents the actual WSI connection among multiple devices. Also, describe the ath12k device property "qcom,ath12k-calibration-variant". This is a common property among ath12k devices. Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com> --- .../bindings/net/wireless/qcom,ath12k.yaml | 241 +++++++++++++++++- 1 file changed, 232 insertions(+), 9 deletions(-)