Re: [RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274

[Krzysztof Kozlowski <krzk@kernel.org>]

On 23/10/2024 08:03, 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-index: It represents the identifier assigned to ath12k
>    device in the order of the WSI connection.
> 
>  - qcom,wsi-num-devices: Number of devices connected through WSI in
>    the same group ID.

You should have separate binding.

> 
> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
> ---
>  .../bindings/net/wireless/qcom,ath12k.yaml    | 61 +++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> index ecf38af747f7..6c8f97865075 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> @@ -19,6 +19,7 @@ properties:
>    compatible:
>      enum:
>        - pci17cb,1107  # WCN7850
> +      - pci17cb,1109  # QCN9274
>  
>    reg:
>      maxItems: 1
> @@ -50,6 +51,41 @@ 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.
> +
> +    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.

Why it cannot be implied by compatible?

> +
> +      qcom,wsi-index:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          It represents the identifier assigned to ath12k device in the
> +          order of the WSI connection.

No, we do not have indices in DTS.

> +
> +      qcom,wsi-num-devices:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Number of devices connected through WSI in the same group ID.

Wait, why? Number of devices is visible from DTS. You are missing some
diagram showing this but it looks like you stuff multiple nodes into one
node.



> +
> +    required:
> +      - qcom,wsi-group-id
> +      - qcom,wsi-index
> +      - qcom,wsi-num-devices
> +
> +    additionalProperties: false
> +
>  required:
>    - compatible
>    - reg
> @@ -108,3 +144,28 @@ examples:
>              };
>          };
>      };
> +
> +  - |
> +    pcie {
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        pcie@0 {
> +            device_type = "pci";
> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            ranges;
> +
> +            wifi@0 {
> +                compatible = "pci17cb,1109";
> +                reg = <0x0 0x0 0x0 0x0 0x0>;
> +
> +                wsi {
> +                    qcom,wsi-group-id = <0>;
> +                    qcom,wsi-index = <0>;
> +                    qcom,wsi-num-devices = <3>;

So what are the other 2 devices? Where are they documented?

Best regards,
Krzysztof