| Message ID | 20250321-qcom_ipq5424_cmnpll-v1-0-3ea8e5262da4@quicinc.com |
|---|---|
| Headers | show |
| Series | Add CMN PLL clock controller support for IPQ5424 | expand |
On 3/21/25 1:49 PM, Luo Jie wrote: > The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference > input clock. The output clocks are the same as IPQ9574 SoC, except > for the clock rate of output clocks to PPE and NSS. > > Also, add macros for clock rates that are applicable specifically > only for IPQ5424. > > Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > --- > .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml | 1 + > include/dt-bindings/clock/qcom,ipq-cmn-pll.h | 10 +++++++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml > index f869b3739be8..bbaf896ae908 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml > +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml > @@ -25,6 +25,7 @@ properties: > compatible: > enum: > - qcom,ipq9574-cmn-pll > + - qcom,ipq5424-cmn-pll > > reg: > maxItems: 1 > diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h > index 936e92b3b62c..e30d57001c38 100644 > --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h > +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > /* > - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. > */ > > #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H > @@ -19,4 +19,12 @@ > #define ETH1_50MHZ_CLK 7 > #define ETH2_50MHZ_CLK 8 > #define ETH_25MHZ_CLK 9 > + > +/* > + * The CMN PLL output clock rates that are specifically applicable for IPQ5424 > + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574. > + */ > +#define NSS_300MHZ_CLK 4 > +#define PPE_375MHZ_CLK 5 Not a huge fan of this, such differences are only relevant to the driver part in my view - bindings only let a consumer reference a specific piece of hardware Konrad
On 3/25/25 12:57 AM, Konrad Dybcio wrote: > On 3/21/25 1:49 PM, Luo Jie wrote: >> The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference >> input clock. The output clocks are the same as IPQ9574 SoC, except >> for the clock rate of output clocks to PPE and NSS. >> >> Also, add macros for clock rates that are applicable specifically >> only for IPQ5424. >> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com> >> --- >> .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml | 1 + >> include/dt-bindings/clock/qcom,ipq-cmn-pll.h | 10 +++++++++- >> 2 files changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml >> index f869b3739be8..bbaf896ae908 100644 >> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml >> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml >> @@ -25,6 +25,7 @@ properties: >> compatible: >> enum: >> - qcom,ipq9574-cmn-pll >> + - qcom,ipq5424-cmn-pll >> >> reg: >> maxItems: 1 >> diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h >> index 936e92b3b62c..e30d57001c38 100644 >> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h >> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h >> @@ -1,6 +1,6 @@ >> /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ >> /* >> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. >> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. >> */ >> >> #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H >> @@ -19,4 +19,12 @@ >> #define ETH1_50MHZ_CLK 7 >> #define ETH2_50MHZ_CLK 8 >> #define ETH_25MHZ_CLK 9 >> + >> +/* >> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424 >> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574. >> + */ >> +#define NSS_300MHZ_CLK 4 >> +#define PPE_375MHZ_CLK 5 > > Not a huge fan of this, such differences are only relevant to the driver > part in my view - bindings only let a consumer reference a specific piece > of hardware Oh I the bindings are stepping into the frequency department already, hmm.. Then I suppose it's fine if the dt-bindings maintainers don't have any concerns Konrad
On Fri, Mar 21, 2025 at 08:49:52PM +0800, Luo Jie wrote: > The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference > input clock. The output clocks are the same as IPQ9574 SoC, except > for the clock rate of output clocks to PPE and NSS. > > Also, add macros for clock rates that are applicable specifically > only for IPQ5424. > > Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > --- > .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml | 1 + > include/dt-bindings/clock/qcom,ipq-cmn-pll.h | 10 +++++++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml > index f869b3739be8..bbaf896ae908 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml > +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml > @@ -25,6 +25,7 @@ properties: > compatible: > enum: > - qcom,ipq9574-cmn-pll > + - qcom,ipq5424-cmn-pll Same comments as before... keep the order. > > reg: > maxItems: 1 > diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h > index 936e92b3b62c..e30d57001c38 100644 > --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h > +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > /* > - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. > */ > > #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H > @@ -19,4 +19,12 @@ > #define ETH1_50MHZ_CLK 7 > #define ETH2_50MHZ_CLK 8 > #define ETH_25MHZ_CLK 9 > + > +/* > + * The CMN PLL output clock rates that are specifically applicable for IPQ5424 > + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574. Just come with different header. Why was this called in generic way, since we ask to name the headers based on compatible? Best regards, Krzysztof
On Tue, Mar 25, 2025 at 12:59:49AM +0100, Konrad Dybcio wrote: > On 3/25/25 12:57 AM, Konrad Dybcio wrote: > > On 3/21/25 1:49 PM, Luo Jie wrote: > >> The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference > >> input clock. The output clocks are the same as IPQ9574 SoC, except > >> for the clock rate of output clocks to PPE and NSS. > >> > >> Also, add macros for clock rates that are applicable specifically > >> only for IPQ5424. > >> > >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > >> --- > >> .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml | 1 + > >> include/dt-bindings/clock/qcom,ipq-cmn-pll.h | 10 +++++++++- > >> 2 files changed, 10 insertions(+), 1 deletion(-) > >> > >> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml > >> index f869b3739be8..bbaf896ae908 100644 > >> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml > >> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml > >> @@ -25,6 +25,7 @@ properties: > >> compatible: > >> enum: > >> - qcom,ipq9574-cmn-pll > >> + - qcom,ipq5424-cmn-pll > >> > >> reg: > >> maxItems: 1 > >> diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h > >> index 936e92b3b62c..e30d57001c38 100644 > >> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h > >> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h > >> @@ -1,6 +1,6 @@ > >> /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > >> /* > >> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > >> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. > >> */ > >> > >> #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H > >> @@ -19,4 +19,12 @@ > >> #define ETH1_50MHZ_CLK 7 > >> #define ETH2_50MHZ_CLK 8 > >> #define ETH_25MHZ_CLK 9 > >> + > >> +/* > >> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424 > >> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574. > >> + */ > >> +#define NSS_300MHZ_CLK 4 > >> +#define PPE_375MHZ_CLK 5 > > > > Not a huge fan of this, such differences are only relevant to the driver > > part in my view - bindings only let a consumer reference a specific piece > > of hardware > > Oh I the bindings are stepping into the frequency department already, > hmm.. Then I suppose it's fine if the dt-bindings maintainers don't have any > concerns Nooooo, it was said these are output clocks, not rates. If these are rates, then NAK. Best regards, Krzysztof
On 3/25/2025 4:19 PM, Krzysztof Kozlowski wrote: > On Fri, Mar 21, 2025 at 08:49:52PM +0800, Luo Jie wrote: >> The CMN PLL block in the IPQ5424 SoC takes 48 MHZ as the reference >> input clock. The output clocks are the same as IPQ9574 SoC, except >> for the clock rate of output clocks to PPE and NSS. >> >> Also, add macros for clock rates that are applicable specifically >> only for IPQ5424. >> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com> >> --- >> .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml | 1 + >> include/dt-bindings/clock/qcom,ipq-cmn-pll.h | 10 +++++++++- >> 2 files changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml >> index f869b3739be8..bbaf896ae908 100644 >> --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml >> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml >> @@ -25,6 +25,7 @@ properties: >> compatible: >> enum: >> - qcom,ipq9574-cmn-pll >> + - qcom,ipq5424-cmn-pll > > Same comments as before... keep the order. OK, I will update to keep the compatible strings in alphanumerical order. > >> >> reg: >> maxItems: 1 >> diff --git a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h >> index 936e92b3b62c..e30d57001c38 100644 >> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h >> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h >> @@ -1,6 +1,6 @@ >> /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ >> /* >> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. >> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. >> */ >> >> #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H >> @@ -19,4 +19,12 @@ >> #define ETH1_50MHZ_CLK 7 >> #define ETH2_50MHZ_CLK 8 >> #define ETH_25MHZ_CLK 9 >> + >> +/* >> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424 >> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574. > > Just come with different header. Why was this called in generic way, > since we ask to name the headers based on compatible? > > Best regards, > Krzysztof > The DT binding file is named according to the compatible string, however I had incorrectly assumed that this recommendation was applicable only to the bindings file. OK, I will add a compatible specific header file to support the SoC IPQ5424. Thanks.
On 3/25/2025 4:22 PM, Krzysztof Kozlowski wrote: >>>> --- a/include/dt-bindings/clock/qcom,ipq-cmn-pll.h >>>> +++ b/include/dt-bindings/clock/qcom,ipq-cmn-pll.h >>>> @@ -1,6 +1,6 @@ >>>> /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ >>>> /* >>>> - * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. >>>> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. >>>> */ >>>> >>>> #ifndef _DT_BINDINGS_CLK_QCOM_IPQ_CMN_PLL_H >>>> @@ -19,4 +19,12 @@ >>>> #define ETH1_50MHZ_CLK 7 >>>> #define ETH2_50MHZ_CLK 8 >>>> #define ETH_25MHZ_CLK 9 >>>> + >>>> +/* >>>> + * The CMN PLL output clock rates that are specifically applicable for IPQ5424 >>>> + * SoC. For IPQ5424, the other output clocks and their rates are same as IPQ9574. >>>> + */ >>>> +#define NSS_300MHZ_CLK 4 >>>> +#define PPE_375MHZ_CLK 5 >>> Not a huge fan of this, such differences are only relevant to the driver >>> part in my view - bindings only let a consumer reference a specific piece >>> of hardware >> Oh I the bindings are stepping into the frequency department already, >> hmm.. Then I suppose it's fine if the dt-bindings maintainers don't have any >> concerns > > Nooooo, it was said these are output clocks, not rates. If these are > rates, then NAK. > > Best regards, > Krzysztof Yes, rates themselves are not defined here, they are defined in the driver data structure. The output clocks of CMN PLL are always fixed clock rate, so the clock frequency was added into the clock specifier macro names defined in this header file for clarity.
The CMN PLL block of IPQ5424 is almost same as that of IPQ9574 which is currently supported by the driver. The only difference is that the fixed output clocks to NSS and PPE from CMN PLL have a different clock rate. In IPQ5424, the output clocks are supplied to NSS at 300 MHZ and to PPE at 375 MHZ. Two related clock identifiers NSS_300MHZ_CLK and PPE_375MHZ_CLK are added for IPQ5424. The new table of output clocks is added for the CMN PLL of IPQ5424, which is acquired from the device according to the compatible. Signed-off-by: Luo Jie <quic_luoj@quicinc.com> --- Luo Jie (4): dt-bindings: clock: qcom: Add CMN PLL support for IPQ5424 SoC clk: qcom: cmnpll: Add IPQ5424 SoC support arm64: dts: ipq5424: Add CMN PLL node arm64: dts: qcom: Update IPQ5424 xo_board to use fixed factor clock .../bindings/clock/qcom,ipq9574-cmn-pll.yaml | 1 + arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts | 23 +++++++++++++-- arch/arm64/boot/dts/qcom/ipq5424.dtsi | 27 ++++++++++++++++- drivers/clk/qcom/ipq-cmn-pll.c | 34 ++++++++++++++++++---- include/dt-bindings/clock/qcom,ipq-cmn-pll.h | 10 ++++++- 5 files changed, 86 insertions(+), 9 deletions(-) --- base-commit: 9388ec571cb1adba59d1cded2300eeb11827679c change-id: 20250321-qcom_ipq5424_cmnpll-416f770f2f97 Best regards,