Re: [PATCH v6 2/8] dt-bindings: clock: qcom,glymur-tcsr: Add mahua support

[Krzysztof Kozlowski]

On Sun, Jun 21, 2026 at 10:11:25PM -0700, Qiang Yu wrote:
> Mahua shares the same QREF TX/RPT/RX component naming as Glymur, but
> has a different topology: a single QREF block fed by REFGEN3 only,
> rather than the two independent blocks fed by REFGEN3 and REFGEN4 on
> Glymur.
> 
> Add qcom,mahua-tcsr compatible and document its required supply
> properties.
> 
> Signed-off-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
> ---
>  .../bindings/clock/qcom,glymur-tcsr.yaml           | 68 ++++++++++++++++------
>  1 file changed, 50 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,glymur-tcsr.yaml b/Documentation/devicetree/bindings/clock/qcom,glymur-tcsr.yaml
> index 16fc6ab87f9b..2b6422627165 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,glymur-tcsr.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,glymur-tcsr.yaml
> @@ -20,7 +20,9 @@ description: |
>  properties:
>    compatible:
>      items:
> -      - const: qcom,glymur-tcsr
> +      - enum:
> +          - qcom,glymur-tcsr

You could have made this enum in the first patch, so you would avoid
changing the same line twice. Usually adding line in patch #1 and then
removing it in patch #2 is indication of something to improve.

> +          - qcom,mahua-tcsr
>        - const: syscon
>  
>    clocks:
> @@ -41,9 +43,11 @@ properties:
>    vdda-qrefrpt2-0p9-supply: true
>    vdda-qrefrpt3-0p9-supply: true
>    vdda-qrefrpt4-0p9-supply: true
> +  vdda-qrefrpt5-0p9-supply: true
>    vdda-qrefrx0-0p9-supply: true
>    vdda-qrefrx1-0p9-supply: true
>    vdda-qrefrx2-0p9-supply: true
> +  vdda-qrefrx3-0p9-supply: true
>    vdda-qrefrx4-0p9-supply: true
>    vdda-qrefrx5-0p9-supply: true
>    vdda-qreftx0-0p9-supply: true
> @@ -54,26 +58,54 @@ properties:
>    vdda-refgen4-0p9-supply: true
>    vdda-refgen4-1p2-supply: true
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,glymur-tcsr
> +    then:
> +      required:
> +        - vdda-qrefrpt0-0p9-supply
> +        - vdda-qrefrpt1-0p9-supply
> +        - vdda-qrefrpt2-0p9-supply
> +        - vdda-qrefrpt3-0p9-supply
> +        - vdda-qrefrpt4-0p9-supply
> +        - vdda-qrefrx0-0p9-supply
> +        - vdda-qrefrx1-0p9-supply
> +        - vdda-qrefrx2-0p9-supply
> +        - vdda-qrefrx4-0p9-supply
> +        - vdda-qrefrx5-0p9-supply
> +        - vdda-qreftx0-0p9-supply
> +        - vdda-qreftx0-1p2-supply
> +        - vdda-qreftx1-0p9-supply
> +        - vdda-refgen3-0p9-supply
> +        - vdda-refgen3-1p2-supply
> +        - vdda-refgen4-0p9-supply
> +        - vdda-refgen4-1p2-supply

It is fine, although this you could as well keep like that in first
patch and mention in commit msg, that binding will grow, thus you
already define if:then: block to accommodate future changes.

Best regards,
Krzysztof