Re: [PATCH v5 01/13] dt-bindings: net: wireless: describe the ath12k AHB module

Krzysztof Kozlowski
2025-01-30T08:28:20Z

On Thu, Jan 30, 2025 at 10:04:56AM +0530, Raj Kumar Bhagat wrote:
> Add device-tree bindings for the ATH12K module found in the IPQ5332
> device.
>
> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
> ---
> .../net/wireless/qcom,ath12k-ahb.yaml | 319 ++++++++++++++++++
> 1 file changed, 319 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k-ahb.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k-ahb.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k-ahb.yaml
> new file mode 100644
> index 000000000000..bd953a028dc3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k-ahb.yaml

Filename should match compatible. This binding does not look like
supporting more devices, so there is no much benefit calling it by generic name.

> @@ -0,0 +1,319 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/wireless/qcom,ath12k-ahb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies ath12k wireless devices (AHB)
> +
> +maintainers:
> + - Kalle Valo <kvalo@kernel.org>
> + - Jeff Johnson <jjohnson@kernel.org>
> +
> +description:
> + Qualcomm Technologies IEEE 802.11be AHB devices.
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,ipq5332-wifi
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: XO clock used for copy engine
> +
> + clock-names:
> + items:
> + - const: xo
> +
> + interrupts:
> + items:
> + - description: Fatal interrupt
> + - description: Ready interrupt
> + - description: Spawn acknowledge interrupt
> + - description: Stop acknowledge interrupt
> + - description: misc-pulse1 interrupt events
> + - description: misc-latch interrupt events
> + - description: sw exception interrupt events
> + - description: interrupt event for ring CE0
> + - 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 CE6
> + - description: interrupt event for ring CE7
> + - description: interrupt event for ring CE8
> + - description: interrupt event for ring CE9
> + - description: interrupt event for ring CE10
> + - description: interrupt event for ring CE11
> + - description: interrupt event for ring host2wbm-desc-feed
> + - description: interrupt event for ring host2reo-re-injection
> + - description: interrupt event for ring host2reo-command
> + - description: interrupt event for ring host2rxdma-monitor-ring1
> + - description: interrupt event for ring reo2ost-exception
> + - description: interrupt event for ring wbm2host-rx-release
> + - description: interrupt event for ring reo2host-status
> + - description: interrupt event for ring reo2host-destination-ring4
> + - description: interrupt event for ring reo2host-destination-ring3
> + - description: interrupt event for ring reo2host-destination-ring2
> + - description: interrupt event for ring reo2host-destination-ring1
> + - description: interrupt event for ring rxdma2host-monitor-destination-mac3
> + - description: interrupt event for ring rxdma2host-monitor-destination-mac2
> + - description: interrupt event for ring rxdma2host-monitor-destination-mac1
> + - description: interrupt event for ring host2rxdma-host-buf-ring-mac3
> + - description: interrupt event for ring host2rxdma-host-buf-ring-mac2
> + - description: interrupt event for ring host2rxdma-host-buf-ring-mac1
> + - description: interrupt event for ring host2tcl-input-ring4
> + - description: interrupt event for ring host2tcl-input-ring3
> + - description: interrupt event for ring host2tcl-input-ring2
> + - description: interrupt event for ring host2tcl-input-ring1
> + - description: interrupt event for ring wbm2host-tx-completions-ring4
> + - description: interrupt event for ring wbm2host-tx-completions-ring3
> + - description: interrupt event for ring wbm2host-tx-completions-ring2
> + - description: interrupt event for ring wbm2host-tx-completions-ring1
> + - description: interrupt event for ring host2tx-monitor-ring1
> + - description: interrupt event for ring txmon2host-monitor-destination-mac3
> + - description: interrupt event for ring txmon2host-monitor-destination-mac2
> + - description: interrupt event for ring txmon2host-monitor-destination-mac1
> + - description: interrupt event for umac-reset
> +
> + interrupt-names:
> + items:
> + - const: fatal
> + - const: ready
> + - const: spawn
> + - const: stop-ack
> + - const: misc-pulse1
> + - const: misc-latch
> + - const: sw-exception
> + - const: ce0
> + - const: ce1
> + - const: ce2
> + - const: ce3
> + - const: ce4
> + - const: ce5
> + - const: ce6
> + - const: ce7
> + - const: ce8
> + - const: ce9
> + - const: ce10
> + - const: ce11
> + - const: host2wbm-desc-feed
> + - const: host2reo-re-injection
> + - const: host2reo-command
> + - const: host2rxdma-monitor-ring1
> + - const: reo2ost-exception
> + - const: wbm2host-rx-release
> + - const: reo2host-status
> + - const: reo2host-destination-ring4
> + - const: reo2host-destination-ring3
> + - const: reo2host-destination-ring2
> + - const: reo2host-destination-ring1
> + - const: rxdma2host-monitor-destination-mac3
> + - const: rxdma2host-monitor-destination-mac2
> + - const: rxdma2host-monitor-destination-mac1
> + - const: host2rxdma-host-buf-ring-mac3
> + - const: host2rxdma-host-buf-ring-mac2
> + - const: host2rxdma-host-buf-ring-mac1
> + - const: host2tcl-input-ring4
> + - const: host2tcl-input-ring3
> + - const: host2tcl-input-ring2
> + - const: host2tcl-input-ring1
> + - const: wbm2host-tx-completions-ring4
> + - const: wbm2host-tx-completions-ring3
> + - const: wbm2host-tx-completions-ring2
> + - const: wbm2host-tx-completions-ring1
> + - const: host2tx-monitor-ring1
> + - const: txmon2host-monitor-destination-mac3
> + - const: txmon2host-monitor-destination-mac2
> + - const: txmon2host-monitor-destination-mac1
> + - const: umac-reset
> +
> + memory-region:
> + description:
> + phandle to a node describing reserved memory (System RAM memory)
> + used by ath12k firmware (see bindings/reserved-memory/reserved-memory.txt)

Do not say what DT syntax is, so "phandle to a node" is redundant,
reserved-memory.txt is redundant. The only useful part here is "used by
ath12k firmware", so based on this none of below are used by the driver
and driver just passes them to the firmware?

> + items:
> + - description: Q6 memory region
> + - description: m3 dump memory region
> + - description: Q6 caldata memory region
> + - description: Multi Link Operation (MLO) Global memory region
> +
> + memory-region-names:
> + description:
> + Name of the reserved memory region used by ath12k firmware

Drop description.

> + items:
> + - const: q6-region
> + - const: m3-dump
> + - const: q6-caldb
> + - const: mlo-global-mem
> +
> + qcom,ath12k-calibration-variant:
> + $ref: /schemas/types.yaml#/definitions/string

Why this is named after ath12k? Why this is just not
"qcom,calibration-variant"? None of the other properties have ath12k in
their names, so why this one in the WSI schema was named like that?

> + description:
> + String to uniquely identify variant of the calibration data for designs
> + with colliding bus and device ids

I don't think this property is here possible. How could you have on the
same SoC different devices?

Best regards,
Krzysztof