Add a node for the nss clock controller found on ipq9574 based devices.
Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
---
Changes in V2:
- Dropped the fixed clock node gcc_gpll0_out_aux and added
support for the same in gcc driver
- Updated the node name to clock-controller@39b00000
- Added clock-names to retrieve the nssnoc clocks and add them
to the list of pm clocks in nss driver
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 48 +++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 51aba071c1eb..903311547e96 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -10,6 +10,8 @@
#include <dt-bindings/clock/qcom,ipq9574-gcc.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/reset/qcom,ipq9574-gcc.h>
+#include <dt-bindings/clock/qcom,ipq9574-nsscc.h>
+#include <dt-bindings/reset/qcom,ipq9574-nsscc.h>
#include <dt-bindings/thermal/thermal.h>
/ {
@@ -18,6 +20,24 @@ / {
#size-cells = <2>;
clocks {
+ bias_pll_cc_clk: bias-pll-cc-clk {
+ compatible = "fixed-clock";
+ clock-frequency = <1200000000>;
+ #clock-cells = <0>;
+ };
+
+ bias_pll_nss_noc_clk: bias-pll-nss-noc-clk {
+ compatible = "fixed-clock";
+ clock-frequency = <461500000>;
+ #clock-cells = <0>;
+ };
+
+ bias_pll_ubi_nc_clk: bias-pll-ubi-nc-clk {
+ compatible = "fixed-clock";
+ clock-frequency = <353000000>;
+ #clock-cells = <0>;
+ };
+
sleep_clk: sleep-clk {
compatible = "fixed-clock";
#clock-cells = <0>;
@@ -722,6 +742,34 @@ frame@b128000 {
status = "disabled";
};
};
+
+ nsscc: clock-controller@39b00000 {
+ compatible = "qcom,ipq9574-nsscc";
+ reg = <0x39b00000 0x80000>;
+ clocks = <&gcc GCC_NSSNOC_NSSCC_CLK>,
+ <&gcc GCC_NSSNOC_SNOC_CLK>,
+ <&gcc GCC_NSSNOC_SNOC_1_CLK>,
+ <&bias_pll_cc_clk>,
+ <&bias_pll_nss_noc_clk>,
+ <&bias_pll_ubi_nc_clk>,
+ <&gcc GPLL0_OUT_AUX>,
+ <0>,
+ <0>,
+ <0>,
+ <0>,
+ <0>,
+ <0>,
+ <&xo_board_clk>;
+ clock-names = "nssnoc_nsscc", "nssnoc_snoc", "nssnoc_snoc_1",
+ "bias_pll_cc_clk", "bias_pll_nss_noc_clk",
+ "bias_pll_ubi_nc_clk", "gpll0_out_aux", "uniphy0_nss_rx_clk",
+ "uniphy0_nss_tx_clk", "uniphy1_nss_rx_clk",
+ "uniphy1_nss_tx_clk", "uniphy2_nss_rx_clk",
+ "uniphy2_nss_tx_clk", "xo_board_clk";
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ #power-domain-cells = <1>;
+ };
};
thermal-zones {
--
2.34.1On Fri, 25 Aug 2023 at 12:15, Devi Priya <quic_devipriy@quicinc.com> wrote:
>
> Add a node for the nss clock controller found on ipq9574 based devices.
>
> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> ---
> Changes in V2:
> - Dropped the fixed clock node gcc_gpll0_out_aux and added
> support for the same in gcc driver
> - Updated the node name to clock-controller@39b00000
> - Added clock-names to retrieve the nssnoc clocks and add them
> to the list of pm clocks in nss driver
>
> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 48 +++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 51aba071c1eb..903311547e96 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> @@ -10,6 +10,8 @@
> #include <dt-bindings/clock/qcom,ipq9574-gcc.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/reset/qcom,ipq9574-gcc.h>
> +#include <dt-bindings/clock/qcom,ipq9574-nsscc.h>
> +#include <dt-bindings/reset/qcom,ipq9574-nsscc.h>
> #include <dt-bindings/thermal/thermal.h>
>
> / {
> @@ -18,6 +20,24 @@ / {
> #size-cells = <2>;
>
> clocks {
> + bias_pll_cc_clk: bias-pll-cc-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <1200000000>;
> + #clock-cells = <0>;
> + };
> +
> + bias_pll_nss_noc_clk: bias-pll-nss-noc-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <461500000>;
> + #clock-cells = <0>;
> + };
> +
> + bias_pll_ubi_nc_clk: bias-pll-ubi-nc-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <353000000>;
> + #clock-cells = <0>;
> + };
Which part provides these clocks?
> +
> sleep_clk: sleep-clk {
> compatible = "fixed-clock";
> #clock-cells = <0>;
> @@ -722,6 +742,34 @@ frame@b128000 {
> status = "disabled";
> };
> };
> +
> + nsscc: clock-controller@39b00000 {
> + compatible = "qcom,ipq9574-nsscc";
> + reg = <0x39b00000 0x80000>;
> + clocks = <&gcc GCC_NSSNOC_NSSCC_CLK>,
> + <&gcc GCC_NSSNOC_SNOC_CLK>,
> + <&gcc GCC_NSSNOC_SNOC_1_CLK>,
> + <&bias_pll_cc_clk>,
> + <&bias_pll_nss_noc_clk>,
> + <&bias_pll_ubi_nc_clk>,
> + <&gcc GPLL0_OUT_AUX>,
> + <0>,
> + <0>,
> + <0>,
> + <0>,
> + <0>,
> + <0>,
> + <&xo_board_clk>;
If you move xo_board closer to the start of the list, it will be
slightly easier to review.
> + clock-names = "nssnoc_nsscc", "nssnoc_snoc", "nssnoc_snoc_1",
> + "bias_pll_cc_clk", "bias_pll_nss_noc_clk",
> + "bias_pll_ubi_nc_clk", "gpll0_out_aux", "uniphy0_nss_rx_clk",
> + "uniphy0_nss_tx_clk", "uniphy1_nss_rx_clk",
> + "uniphy1_nss_tx_clk", "uniphy2_nss_rx_clk",
> + "uniphy2_nss_tx_clk", "xo_board_clk";
You are using clock indices. Please drop clock-names.
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + #power-domain-cells = <1>;
> + };
> };
>
> thermal-zones {
> --
> 2.34.1
>
--
With best wishes
Dmitry
Hi Dmitry,
On Fri, Aug 25, 2023 at 1:28 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
> On Fri, 25 Aug 2023 at 12:15, Devi Priya <quic_devipriy@quicinc.com> wrote:
> > Add a node for the nss clock controller found on ipq9574 based devices.
> >
> > Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> > ---
> > Changes in V2:
> > - Dropped the fixed clock node gcc_gpll0_out_aux and added
> > support for the same in gcc driver
> > - Updated the node name to clock-controller@39b00000
> > - Added clock-names to retrieve the nssnoc clocks and add them
> > to the list of pm clocks in nss driver
> >
> > arch/arm64/boot/dts/qcom/ipq9574.dtsi | 48 +++++++++++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> > index 51aba071c1eb..903311547e96 100644
> > --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> > @@ -722,6 +742,34 @@ frame@b128000 {
> > status = "disabled";
> > };
> > };
> > +
> > + nsscc: clock-controller@39b00000 {
> > + compatible = "qcom,ipq9574-nsscc";
> > + reg = <0x39b00000 0x80000>;
> > + clocks = <&gcc GCC_NSSNOC_NSSCC_CLK>,
> > + <&gcc GCC_NSSNOC_SNOC_CLK>,
> > + <&gcc GCC_NSSNOC_SNOC_1_CLK>,
> > + <&bias_pll_cc_clk>,
> > + <&bias_pll_nss_noc_clk>,
> > + <&bias_pll_ubi_nc_clk>,
> > + <&gcc GPLL0_OUT_AUX>,
> > + <0>,
> > + <0>,
> > + <0>,
> > + <0>,
> > + <0>,
> > + <0>,
> > + <&xo_board_clk>;
>
> If you move xo_board closer to the start of the list, it will be
> slightly easier to review.
>
> > + clock-names = "nssnoc_nsscc", "nssnoc_snoc", "nssnoc_snoc_1",
> > + "bias_pll_cc_clk", "bias_pll_nss_noc_clk",
> > + "bias_pll_ubi_nc_clk", "gpll0_out_aux", "uniphy0_nss_rx_clk",
> > + "uniphy0_nss_tx_clk", "uniphy1_nss_rx_clk",
> > + "uniphy1_nss_tx_clk", "uniphy2_nss_rx_clk",
> > + "uniphy2_nss_tx_clk", "xo_board_clk";
>
> You are using clock indices. Please drop clock-names.
What do you mean by "using clock indices"?
Note that the "clock-names" property is required according to the DT bindings.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 13/09/2023 10:23, Geert Uytterhoeven wrote: >> >>> + clock-names = "nssnoc_nsscc", "nssnoc_snoc", "nssnoc_snoc_1", >>> + "bias_pll_cc_clk", "bias_pll_nss_noc_clk", >>> + "bias_pll_ubi_nc_clk", "gpll0_out_aux", "uniphy0_nss_rx_clk", >>> + "uniphy0_nss_tx_clk", "uniphy1_nss_rx_clk", >>> + "uniphy1_nss_tx_clk", "uniphy2_nss_rx_clk", >>> + "uniphy2_nss_tx_clk", "xo_board_clk"; >> >> You are using clock indices. Please drop clock-names. > > What do you mean by "using clock indices"? > Note that the "clock-names" property is required according to the DT bindings. Indeed, thanks for pointing this out. Probably bindings should be changed. Best regards, Krzysztof
Hi Krzysztof,
On Wed, Sep 13, 2023 at 10:26 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 13/09/2023 10:23, Geert Uytterhoeven wrote:
> >>
> >>> + clock-names = "nssnoc_nsscc", "nssnoc_snoc", "nssnoc_snoc_1",
> >>> + "bias_pll_cc_clk", "bias_pll_nss_noc_clk",
> >>> + "bias_pll_ubi_nc_clk", "gpll0_out_aux", "uniphy0_nss_rx_clk",
> >>> + "uniphy0_nss_tx_clk", "uniphy1_nss_rx_clk",
> >>> + "uniphy1_nss_tx_clk", "uniphy2_nss_rx_clk",
> >>> + "uniphy2_nss_tx_clk", "xo_board_clk";
> >>
> >> You are using clock indices. Please drop clock-names.
> >
> > What do you mean by "using clock indices"?
> > Note that the "clock-names" property is required according to the DT bindings.
>
> Indeed, thanks for pointing this out. Probably bindings should be changed.
But what's so great about not having "clock-names"?
There are _14_ input clocks.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 13/09/2023 10:38, Geert Uytterhoeven wrote: > Hi Krzysztof, > > On Wed, Sep 13, 2023 at 10:26 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> On 13/09/2023 10:23, Geert Uytterhoeven wrote: >>>> >>>>> + clock-names = "nssnoc_nsscc", "nssnoc_snoc", "nssnoc_snoc_1", >>>>> + "bias_pll_cc_clk", "bias_pll_nss_noc_clk", >>>>> + "bias_pll_ubi_nc_clk", "gpll0_out_aux", "uniphy0_nss_rx_clk", >>>>> + "uniphy0_nss_tx_clk", "uniphy1_nss_rx_clk", >>>>> + "uniphy1_nss_tx_clk", "uniphy2_nss_rx_clk", >>>>> + "uniphy2_nss_tx_clk", "xo_board_clk"; >>>> >>>> You are using clock indices. Please drop clock-names. >>> >>> What do you mean by "using clock indices"? >>> Note that the "clock-names" property is required according to the DT bindings. >> >> Indeed, thanks for pointing this out. Probably bindings should be changed. > > But what's so great about not having "clock-names"? > There are _14_ input clocks. There is nothing particularly wrong. They are just not used by Linux implementation and they confuse people into thinking items are not strictly ordered. Thus agreement long time ago for Qualcomm clock controllers was to drop the clock-names to avoid that confusion and make it explicit. Best regards, Krzysztof
On 13.09.2023 10:38, Geert Uytterhoeven wrote: > Hi Krzysztof, > > On Wed, Sep 13, 2023 at 10:26 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> On 13/09/2023 10:23, Geert Uytterhoeven wrote: >>>> >>>>> + clock-names = "nssnoc_nsscc", "nssnoc_snoc", "nssnoc_snoc_1", >>>>> + "bias_pll_cc_clk", "bias_pll_nss_noc_clk", >>>>> + "bias_pll_ubi_nc_clk", "gpll0_out_aux", "uniphy0_nss_rx_clk", >>>>> + "uniphy0_nss_tx_clk", "uniphy1_nss_rx_clk", >>>>> + "uniphy1_nss_tx_clk", "uniphy2_nss_rx_clk", >>>>> + "uniphy2_nss_tx_clk", "xo_board_clk"; >>>> >>>> You are using clock indices. Please drop clock-names. >>> >>> What do you mean by "using clock indices"? >>> Note that the "clock-names" property is required according to the DT bindings. >> >> Indeed, thanks for pointing this out. Probably bindings should be changed. > > But what's so great about not having "clock-names"? > There are _14_ input clocks. clk_parent_data has an "index" member, which lets us bind clocks[n] to parent[n]. With the DT properties being ABI, including the order of entries within, that lets us get rid of clock-names and the matching is marginally faster. Konrad
On 8/25/2023 4:58 PM, Dmitry Baryshkov wrote:
> On Fri, 25 Aug 2023 at 12:15, Devi Priya <quic_devipriy@quicinc.com> wrote:
>>
>> Add a node for the nss clock controller found on ipq9574 based devices.
>>
>> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
>> ---
>> Changes in V2:
>> - Dropped the fixed clock node gcc_gpll0_out_aux and added
>> support for the same in gcc driver
>> - Updated the node name to clock-controller@39b00000
>> - Added clock-names to retrieve the nssnoc clocks and add them
>> to the list of pm clocks in nss driver
>>
>> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 48 +++++++++++++++++++++++++++
>> 1 file changed, 48 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> index 51aba071c1eb..903311547e96 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> @@ -10,6 +10,8 @@
>> #include <dt-bindings/clock/qcom,ipq9574-gcc.h>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> #include <dt-bindings/reset/qcom,ipq9574-gcc.h>
>> +#include <dt-bindings/clock/qcom,ipq9574-nsscc.h>
>> +#include <dt-bindings/reset/qcom,ipq9574-nsscc.h>
>> #include <dt-bindings/thermal/thermal.h>
>>
>> / {
>> @@ -18,6 +20,24 @@ / {
>> #size-cells = <2>;
>>
>> clocks {
>> + bias_pll_cc_clk: bias-pll-cc-clk {
>> + compatible = "fixed-clock";
>> + clock-frequency = <1200000000>;
>> + #clock-cells = <0>;
>> + };
>> +
>> + bias_pll_nss_noc_clk: bias-pll-nss-noc-clk {
>> + compatible = "fixed-clock";
>> + clock-frequency = <461500000>;
>> + #clock-cells = <0>;
>> + };
>> +
>> + bias_pll_ubi_nc_clk: bias-pll-ubi-nc-clk {
>> + compatible = "fixed-clock";
>> + clock-frequency = <353000000>;
>> + #clock-cells = <0>;
>> + };
>
> Which part provides these clocks?
The Bias PLL generates these clocks based on the reference clock.
>
>> +
>> sleep_clk: sleep-clk {
>> compatible = "fixed-clock";
>> #clock-cells = <0>;
>> @@ -722,6 +742,34 @@ frame@b128000 {
>> status = "disabled";
>> };
>> };
>> +
>> + nsscc: clock-controller@39b00000 {
>> + compatible = "qcom,ipq9574-nsscc";
>> + reg = <0x39b00000 0x80000>;
>> + clocks = <&gcc GCC_NSSNOC_NSSCC_CLK>,
>> + <&gcc GCC_NSSNOC_SNOC_CLK>,
>> + <&gcc GCC_NSSNOC_SNOC_1_CLK>,
>> + <&bias_pll_cc_clk>,
>> + <&bias_pll_nss_noc_clk>,
>> + <&bias_pll_ubi_nc_clk>,
>> + <&gcc GPLL0_OUT_AUX>,
>> + <0>,
>> + <0>,
>> + <0>,
>> + <0>,
>> + <0>,
>> + <0>,
>> + <&xo_board_clk>;
>
> If you move xo_board closer to the start of the list, it will be
> slightly easier to review.
Sure okay
>
>> + clock-names = "nssnoc_nsscc", "nssnoc_snoc", "nssnoc_snoc_1",
>> + "bias_pll_cc_clk", "bias_pll_nss_noc_clk",
>> + "bias_pll_ubi_nc_clk", "gpll0_out_aux", "uniphy0_nss_rx_clk",
>> + "uniphy0_nss_tx_clk", "uniphy1_nss_rx_clk",
>> + "uniphy1_nss_tx_clk", "uniphy2_nss_rx_clk",
>> + "uniphy2_nss_tx_clk", "xo_board_clk";
>
> You are using clock indices. Please drop clock-names.
Sure okay
Thanks,
Devi Priya
>
>> + #clock-cells = <1>;
>> + #reset-cells = <1>;
>> + #power-domain-cells = <1>;
>> + };
>> };
>>
>> thermal-zones {
>> --
>> 2.34.1
>>
>
>
© 2016 - 2026 Red Hat, Inc.