Re: [PATCH v5 6/9] ASoC: dt-bindings: qcom: Add Hawi LPASS macro codecs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dt-bindings: erroneous 'lpr' clock requirement for TX and VA macros
- [Low] dt-bindings: qcom: lpass-tx: copy-pasted clock prevents block sharing
--

commit 8329a2400381810d27d8768d8793aa64ce4220c0
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>

ASoC: dt-bindings: qcom: Add Hawi LPASS macro codecs

This commit documents the compatibles for the RX, TX, and VA macro codecs in
the Qualcomm Hawi SoC. It details the specific clock constraints for each
macro compared to the SM8550 generation.

> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-tx-macro.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-tx-macro.yaml
> index d8682ff2e82c5..2df72450d36f5 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-tx-macro.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-tx-macro.yaml

[ ... ]

> @@ -142,6 +143,24 @@ allOf:
>              - const: dcodec
>              - const: fsgen
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,hawi-lpass-tx-macro
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 4
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: mclk
> +            - const: lpr
> +            - const: dcodec
> +            - const: fsgen

[Severity: Low]
Does this erroneously expect the lpr clock instead of the macro clock?

The commit message explicitly states that only the RX macro uses the lpr
clock instead of macro, which suggests the TX macro should still use the
macro clock.

If this is a copy-paste error and is corrected to macro, the clock
constraints here become identical to the SM8550 TX macro. Should this new
compatible be added to the prior generation's if block rather than
duplicating the entire block, as per the dt-bindings guidelines?

> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-va-macro.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-va-macro.yaml
> index 683ebf702ba8a..7d56f9c360566 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-va-macro.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-va-macro.yaml

[ ... ]

> @@ -185,6 +186,24 @@ allOf:
>              - const: mclk
>              - const: npl
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,hawi-lpass-va-macro
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 4
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: mclk
> +            - const: lpr
> +            - const: dcodec
> +            - const: heartbeatpulse

[Severity: Low]
Similar to the TX macro above, does this schema improperly require the
lpr clock?

The commit message indicates that the VA macro simply takes an additional
heartbeatpulse clock, which implies it should retain the macro clock
rather than switching to lpr.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-add_shikra_support_v5-v5-0-fb9f0667e613@oss.qualcomm.com?part=6