| Message ID | 20241029173050.2188150-2-quic_rajkbhag@quicinc.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | wifi: ath12k: Add WSI node for QCN9274 in RDP433 for MLO | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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. > > 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. > > 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. > > reg: > maxItems: 1 > > + qcom,ath12k-calibration-variant: > + $ref: /schemas/types.yaml#/definitions/string > + description: | Do not need '|' unless you need to preserve formatting. > + 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. > + 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. > + > + qcom,wsi-master: > + type: boolean > + description: > + Indicates if this device is the WSI master. > + This copies property name. Why being master is important? Also, use some different name: see preferred names in kernel coding style. > + 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. > + > examples: > - | > #include <dt-bindings/clock/qcom,rpmh.h> > @@ -97,3 +184,139 @@ examples: > }; > }; > }; > + > + - | > + pcie1 { pcie { and keep all nodes here > + #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. > + 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. Best regards, Krzysztof
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? > + | | | | | | > + +----->| wsi |------->| wsi |------->| wsi |-----+ > + | | grp 0 | | grp 0 | | grp 2 | | s/grp 2/grp 0/??? ^ typo? > + | +-------+ +-------+ +-------+ | > + +------------------------------------------------------+ > + > + 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 > + | | | | | | | | > + +-->| 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: > }; > }; > }; > + > + - | 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? > + 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>; > + }; > + }; > + }; > + }; > + }; > + }; > + };
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
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(-)