Re: [PATCH 1/6] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5018 SoC

[George Moussalem]

On 5/4/25 05:49, Jie Luo wrote:
> On 5/2/2025 6:15 PM, George Moussalem via B4 Relay wrote:
> > From: George Moussalem <george.moussalem@xxxxxxxxxxx>
> >
> > The CMN PLL block in the IPQ5018 SoC takes 96 MHZ as the reference
> > input clock. Its output clocks are the XO (24Mhz), sleep (32Khz), and
> > ethernet (50Mhz) clocks.
> >
> > Unlike IPQ9574, the CMN PLL to the ethernet block needs to be enabled
> > first in IPQ5018. Hence, add optional phandle to TCSR register space
> > and offset to do so.
> >
> > Signed-off-by: George Moussalem <george.moussalem@xxxxxxxxxxx>
> > ---
> >    .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml  | 11 ++++++++---
> >    include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h         | 16 ++++++++++++++++
> >    2 files changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> > index cb6e09f4247f4b25105b25f4ae746c0b3ef47616..25006d65d30e20ef8e1f43537bcf3dca65bae73d 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
> > @@ -24,12 +24,10 @@ description:
> >    properties:
> >      compatible:
> >        enum:
> > +      - qcom,ipq5018-cmn-pll
> >          - qcom,ipq5424-cmn-pll
> >          - qcom,ipq9574-cmn-pll
> >    
> > -  reg:
> > -    maxItems: 1
> > -
>
> The property 'reg' should not be removed.

Moved it back, I unintentionally removed it. Thanks!

> >      clocks:
> >        items:
> >          - description: The reference clock. The supported clock rates include
> > @@ -50,6 +48,13 @@ properties:
> >      "#clock-cells":
> >        const: 1
> >    
> > +  qcom,cmn-pll-eth-enable:
> > +    description: Register in TCSR to enable CMN PLL to ethernet
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +        - description: phandle of TCSR syscon
> > +        - description: offset of TCSR register to enable CMN PLL to ethernet
> > +
>
> This TCSR should not be a part of CMN PLL, it is the LDO controller for
> the internal GEPHY in IPQ5018 SoC, which can be moved to a part of GEPHY
> device.

I'm preparing a patch for the ipq5018 GE PHY too so can move it there. 
Can you confirm the uniphy isn't dependent on this too? There are boards 
out there which don't use the GE PHY at all, so if the uniphy depends on 
it for SGMII/SGMII+, this approach wouldn't work.

> >    required:
> >      - compatible
> >      - reg
> > diff --git a/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..586d1c9b33b374331bef413f543c526212c18494
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +/*
> > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
> > +#define _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
> > +
> > +/* CMN PLL core clock. */
> > +#define IPQ5018_CMN_PLL_CLK			0
> > +
> > +/* The output clocks from CMN PLL of IPQ5018. */
> > +#define IPQ5018_XO_24MHZ_CLK			1
> > +#define IPQ5018_SLEEP_32KHZ_CLK			2
> > +#define IPQ5018_ETH_50MHZ_CLK			3
> > +#endif

Best regards,
George