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

[Qiang Yu]

On Mon, Jun 22, 2026 at 09:32:34AM +0200, Krzysztof Kozlowski wrote:
> 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.
>
Okay, will use  
	- enum:
	- qcom,glymur-tcsr
in the first patch in next version.

- Qiang Yu
> > +          - 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.
>
Okay, I will add the if:then: block in first patch and add some
description in commit msg. Then I also don't need to remove 'Mark the
relevant supplies as required per compatible using allOf/if/then
conditionals' in commit msg of first patch.

- Qiang Yu