Re: [PATCH v2 1/3] ASoC: dt-bindings: qcom,lpass-va-csr: Add HeartBeat pulse clock

[Krzysztof Kozlowski <krzk@kernel.org>]

On 16/07/2026 21:51, Sarath Ganapathiraju via B4 Relay wrote:
> From: Sarath Ganapathiraju <sarath.ganapathiraju@oss.qualcomm.com>
> 
> Add Qualcomm LPASS VA CSR rate generator node that exposes
> the lpass_heartbeat_pulse clock on hawi.
> 
> The HeartBeat Pulse (also known as RateGen Pulse) synchronizes the
> start of the DMAs and Codec Interfaces for the audio usecase
> and can serve as a periodic wakeup source for the DSP.
> 
> Signed-off-by: Sarath Ganapathiraju <sarath.ganapathiraju@oss.qualcomm.com>
> ---
>  .../bindings/sound/qcom,lpass-va-csr.yaml          | 52 ++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-va-csr.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-va-csr.yaml
> new file mode 100644
> index 0000000000..131debb04f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-va-csr.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/qcom,lpass-va-csr.yaml#

I do not understand why a clock controller is part of sound. This makes
little sense and desription does not help me.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm LPASS VA CSR heartbeat pulse clock provider
> +
> +maintainers:
> +  - Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
> +  - Sarath Ganapathiraju <sarath.ganapathiraju@oss.qualcomm.com>
> +
> +description:
> +  The LPASS VA CSR block contains the rate generator hardware which
> +  produces the heartbeatpulse consumed by the Bolero codec macros
> +  (rx, tx, wsa, va). It is modelled as a clock provider
> +  so consumers can reference it by name in their clocks list.

Drop last syntax, explain the hardware, not duplicate schema as text.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,hawi-lpass-va-csr
> +
> +  reg:
> +    maxItems: 1
> +    description: Must cover the full VA CSR block (base 0x7EE0000, size 0xE000).

Drop description, redundant. Why would not device MMIO address range
cover full block?

> +
> +  "#clock-cells":
> +    const: 0
> +    description:
> +      Single clock output "lpass_heartbeat_pulse". Consumers reference
> +      it by clock-name "heartbeatpulse" in their clocks list.

Drop description. Not even accurate. Consumers reference it however they
wish and it is not any business of the provider.

From where did you take such style?

> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +
> +unevaluatedProperties: false

And this...

> +
> +examples:
> +  - |
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      va-csr@7ee0000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

> +        compatible = "qcom,hawi-lpass-va-csr";
> +        reg = <0x0 0x07ee0000 0x0 0xe000>;
> +        #clock-cells = <0>;
> +      };
> +    };
> 


Best regards,
Krzysztof