linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: george.moussalem@outlook.com,
	Johannes Berg <johannes@sipsolutions.net>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Jeff Johnson <jjohnson@kernel.org>
Cc: linux-wireless@vger.kernel.org, devicetree@vger.kernel.org,
	ath11k@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] dt: bindings: net: add bindings for QCN6122
Date: Wed, 29 Oct 2025 15:32:17 +0100	[thread overview]
Message-ID: <3dc712ae-b51f-4142-bbab-1eadbc27e60a@kernel.org> (raw)
In-Reply-To: <20251029-ath11k-qcn6122-v1-1-58ed68eba333@outlook.com>

On 29/10/2025 15:26, George Moussalem via B4 Relay wrote:
> From: George Moussalem <george.moussalem@outlook.com>
> 
> QCN6122 is a PCIe based solution that is attached to and enumerated
> by the WPSS (Wireless Processor SubSystem) Q6 processor.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

The prefix is never "dt:".


A nit, subject: drop second/last, redundant "bindings for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Though it is a PCIe device, since it is not attached to APSS processor
> (Application Processor SubSystem), APSS will be unaware of such a decice
> so it is registered to the APSS processor as a platform device(AHB).
> Because of this hybrid nature, it is called as a hybrid bus device as
> introduced by WCN6750. It has 5 CE and 8 DP rings.
> 
> QCN6122 is similar to WCN6750 and follows the same codepath as for
> WCN6750.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---
>  .../bindings/net/wireless/qcom,ath11k.yaml         | 57 +++++++++++++++++++++-
>  1 file changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
> index c089677702cf17f3016b054d21494d2a7706ce5d..4b0b282bb9231c8bc496fed42e0917b9d7d106d2 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
> @@ -21,12 +21,13 @@ properties:
>        - qcom,ipq6018-wifi
>        - qcom,wcn6750-wifi
>        - qcom,ipq5018-wifi
> +      - qcom,qcn6122-wifi

Why people keep adding to the end... previously ipq5018 added by
qualcom, did not even get any review.

Place it before wcn and let ipq5018 be outlier since this was broken
already.

>  
>    reg:
>      maxItems: 1
>  
>    interrupts:
> -    minItems: 32
> +    minItems: 13
>      maxItems: 52
>  
>    interrupt-names:
> @@ -87,6 +88,14 @@ properties:
>      items:
>        - const: wlan-smp2p-out
>  
> +  qcom,userpd:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [2, 3]
> +    description: instance ID of user PD (protection domain) in multi-PD
> +                 architectures to distinguish between multiple instances
> +                 of the same wifi chip used by QMI in its interface with
> +                 the firmware running on Q6.

Broken indentation. It is supposed to be two spaces. Look at this file -
why are you doing this completely different?

Anyway, please do not come with 2nd or 3rd property for this. We already
have such somewhere.

> +
>  required:
>    - compatible
>    - reg
> @@ -268,6 +277,31 @@ allOf:
>              - description: interrupt event for ring DP20
>              - description: interrupt event for ring DP21
>              - description: interrupt event for ring DP22
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,qcn6122-wifi
> +    then:
> +      required:
> +        - qcom,userpd
> +      properties:
> +        interrupts:
> +          items:
> +            - description: interrupt event for ring CE1
> +            - description: interrupt event for ring CE2
> +            - description: interrupt event for ring CE3
> +            - description: interrupt event for ring CE4
> +            - description: interrupt event for ring CE5
> +            - description: interrupt event for ring DP1
> +            - description: interrupt event for ring DP2
> +            - description: interrupt event for ring DP3
> +            - description: interrupt event for ring DP4
> +            - description: interrupt event for ring DP5
> +            - description: interrupt event for ring DP6
> +            - description: interrupt event for ring DP7
> +            - description: interrupt event for ring DP8
>  
>  examples:
>    - |
> @@ -467,3 +501,24 @@ examples:
>              iommus = <&apps_smmu 0x1c02 0x1>;
>          };
>      };
> +
> +  - |
> +    wifi1: wifi@b00a040 {
> +        reg = <0x0b00a040 0x0>;
> +        compatible = "qcom,qcn6122-wifi";

Don't add examples if they differ by one property. Drop.

BR

Best regards,
Krzysztof

  reply	other threads:[~2025-10-29 14:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-29 14:26 [PATCH 0/6] wifi: ath11k: Add support for QCN6122 George Moussalem via B4 Relay
2025-10-29 14:26 ` [PATCH 1/6] dt: bindings: net: add bindings " George Moussalem via B4 Relay
2025-10-29 14:32   ` Krzysztof Kozlowski [this message]
2025-10-29 14:42     ` Krzysztof Kozlowski
2025-10-29 16:12     ` George Moussalem
2025-10-30  5:47       ` Krzysztof Kozlowski
2025-10-29 14:26 ` [PATCH 2/6] wifi: ath11k: add hw params " George Moussalem via B4 Relay
2025-10-29 14:26 ` [PATCH 3/6] wifi: ath11k: add hw ring mask " George Moussalem via B4 Relay
2025-10-29 14:33   ` Krzysztof Kozlowski
2025-10-29 14:26 ` [PATCH 4/6] wifi: ath11k: update hif and pci ops " George Moussalem via B4 Relay
2025-10-29 14:26 ` [PATCH 5/6] wifi: ath11k: add multipd support " George Moussalem via B4 Relay
2025-10-29 14:43   ` Krzysztof Kozlowski
2025-10-29 17:41     ` George Moussalem
2025-10-29 14:26 ` [PATCH 6/6] wifi: ath11k: add QCN6122 device support George Moussalem via B4 Relay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3dc712ae-b51f-4142-bbab-1eadbc27e60a@kernel.org \
    --to=krzk@kernel.org \
    --cc=ath11k@lists.infradead.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=george.moussalem@outlook.com \
    --cc=jjohnson@kernel.org \
    --cc=johannes@sipsolutions.net \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).