| Message ID | 20170618015855.27738-10-chunyan.zhang@spreadtrum.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Stephen Boyd |
| Headers | show |
On 06/18, Chunyan Zhang wrote: > diff --git a/arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi b/arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi > new file mode 100644 > index 0000000..e15bf2d > --- /dev/null > +++ b/arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi > @@ -0,0 +1,67 @@ > +/* > + * Spreadtrum SC9860 SoC CCU > + * > + * Copyright (C) 2017, Spreadtrum Communications Inc. > + * > + * SPDX-License-Identifier: (GPL-2.0+ OR MIT) > + */ > + > +&soc { > + ext_26m: ext-26m { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <26000000>; > + clock-output-names = "ext-26m"; > + }; > + > + ext_32m_sine0: ext-32m-sine0 { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32000000>; > + clock-output-names = "ext-32m-sine0"; > + }; > + > + ext_32m_sine1: ext-32m-sine1 { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32000000>; > + clock-output-names = "ext-32m-sine1"; > + }; > + > + ext_rco_100m: ext-rco-100m { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <100000000>; > + clock-output-names = "ext-rco-100m"; > + }; > + > + ext_32k: ext-32k { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32768>; > + clock-output-names = "ext-32k"; > + }; These should all be outside of the soc node as they're probably on the board and not the SoC? The hint is that they don't have a reg property. > + > + ccu: clk { clock-controller is a more standard node name. > + compatible = "sprd,sc9860-ccu"; > + #clock-cells = <1>; > + reg = <0 0x20000000 0 0x400>, > + <0 0x20210000 0 0x3000>, > + <0 0x402b0000 0 0x4000>, > + <0 0x402d0000 0 0x400>, > + <0 0x402e0000 0 0x4000>, > + <0 0x40400000 0 0x400>, > + <0 0x40880000 0 0x400>, > + <0 0x415e0000 0 0x400>, > + <0 0x60200000 0 0x400>, > + <0 0x61000000 0 0x400>, > + <0 0x61100000 0 0x3000>, > + <0 0x62000000 0 0x4000>, > + <0 0x62100000 0 0x4000>, > + <0 0x63000000 0 0x400>, > + <0 0x63100000 0 0x3000>, > + <0 0x70b00000 0 0x3000>; There are a lot of reg properties here. Perhaps there needs to be different nodes for the different clock controllers in this SoC? > + clocks = <&ext_26m>, <&ext_rco_100m>, <&ext_32k>; > + clock-names = "ext-26m", "ext-rco-100m", "ext-32k"; > + };
Hi Stephen, On 20 June 2017 at 09:24, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 06/18, Chunyan Zhang wrote: >> diff --git a/arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi b/arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi >> new file mode 100644 >> index 0000000..e15bf2d >> --- /dev/null >> +++ b/arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi >> @@ -0,0 +1,67 @@ >> +/* >> + * Spreadtrum SC9860 SoC CCU >> + * >> + * Copyright (C) 2017, Spreadtrum Communications Inc. >> + * >> + * SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> + */ >> + >> +&soc { >> + ext_26m: ext-26m { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <26000000>; >> + clock-output-names = "ext-26m"; >> + }; >> + >> + ext_32m_sine0: ext-32m-sine0 { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <32000000>; >> + clock-output-names = "ext-32m-sine0"; >> + }; >> + >> + ext_32m_sine1: ext-32m-sine1 { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <32000000>; >> + clock-output-names = "ext-32m-sine1"; >> + }; >> + >> + ext_rco_100m: ext-rco-100m { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <100000000>; >> + clock-output-names = "ext-rco-100m"; >> + }; >> + >> + ext_32k: ext-32k { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <32768>; >> + clock-output-names = "ext-32k"; >> + }; > > These should all be outside of the soc node as they're probably > on the board and not the SoC? The hint is that they don't have a > reg property. > >> + >> + ccu: clk { > > clock-controller is a more standard node name. OK, will address. > >> + compatible = "sprd,sc9860-ccu"; >> + #clock-cells = <1>; >> + reg = <0 0x20000000 0 0x400>, >> + <0 0x20210000 0 0x3000>, >> + <0 0x402b0000 0 0x4000>, >> + <0 0x402d0000 0 0x400>, >> + <0 0x402e0000 0 0x4000>, >> + <0 0x40400000 0 0x400>, >> + <0 0x40880000 0 0x400>, >> + <0 0x415e0000 0 0x400>, >> + <0 0x60200000 0 0x400>, >> + <0 0x61000000 0 0x400>, >> + <0 0x61100000 0 0x3000>, >> + <0 0x62000000 0 0x4000>, >> + <0 0x62100000 0 0x4000>, >> + <0 0x63000000 0 0x400>, >> + <0 0x63100000 0 0x3000>, >> + <0 0x70b00000 0 0x3000>; > > There are a lot of reg properties here. Perhaps there needs to be > different nodes for the different clock controllers in this SoC? > On Spreadtrum's platform, clocks are basically located in a few address areas due to some hardware design issue, that says there're more than one kinds of clocks in one address range, and one kind of clocks have more than one physical address bases, except ccu_pll and ccu_div in this patchset. We're planning to map the whole device area at one time before initializing each of them, once that has been done and upstreamed, I will remove these lists of addressed. Thanks for your review, Chunyan >> + clocks = <&ext_26m>, <&ext_rco_100m>, <&ext_32k>; >> + clock-names = "ext-26m", "ext-rco-100m", "ext-32k"; >> + }; > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/22, Chunyan Zhang wrote: > Hi Stephen, > > On 20 June 2017 at 09:24, Stephen Boyd <sboyd@codeaurora.org> wrote: > > On 06/18, Chunyan Zhang wrote: > > > > >> + compatible = "sprd,sc9860-ccu"; > >> + #clock-cells = <1>; > >> + reg = <0 0x20000000 0 0x400>, > >> + <0 0x20210000 0 0x3000>, > >> + <0 0x402b0000 0 0x4000>, > >> + <0 0x402d0000 0 0x400>, > >> + <0 0x402e0000 0 0x4000>, > >> + <0 0x40400000 0 0x400>, > >> + <0 0x40880000 0 0x400>, > >> + <0 0x415e0000 0 0x400>, > >> + <0 0x60200000 0 0x400>, > >> + <0 0x61000000 0 0x400>, > >> + <0 0x61100000 0 0x3000>, > >> + <0 0x62000000 0 0x4000>, > >> + <0 0x62100000 0 0x4000>, > >> + <0 0x63000000 0 0x400>, > >> + <0 0x63100000 0 0x3000>, > >> + <0 0x70b00000 0 0x3000>; > > > > There are a lot of reg properties here. Perhaps there needs to be > > different nodes for the different clock controllers in this SoC? > > > > On Spreadtrum's platform, clocks are basically located in a few > address areas due to some hardware design issue, that says there're > more than one kinds of clocks in one address range, and one kind of > clocks have more than one physical address bases, except ccu_pll and > ccu_div in this patchset. > > We're planning to map the whole device area at one time before > initializing each of them, once that has been done and upstreamed, I > will remove these lists of addressed. Ok. Does this mean we need to wait for those patches to be sent out for review? Is it more like certain clks are embedded inside other devices like display controllers, i2c controllers, etc? Is there any more information I can get on this SoC?
Hi Stephen, Thanks for your every so clear and detailed answer, thank you. On 30 June 2017 at 08:57, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 06/22, Chunyan Zhang wrote: >> Hi Stephen, >> >> On 20 June 2017 at 09:24, Stephen Boyd <sboyd@codeaurora.org> wrote: >> > On 06/18, Chunyan Zhang wrote: >> >> > >> >> + compatible = "sprd,sc9860-ccu"; >> >> + #clock-cells = <1>; >> >> + reg = <0 0x20000000 0 0x400>, >> >> + <0 0x20210000 0 0x3000>, >> >> + <0 0x402b0000 0 0x4000>, >> >> + <0 0x402d0000 0 0x400>, >> >> + <0 0x402e0000 0 0x4000>, >> >> + <0 0x40400000 0 0x400>, >> >> + <0 0x40880000 0 0x400>, >> >> + <0 0x415e0000 0 0x400>, >> >> + <0 0x60200000 0 0x400>, >> >> + <0 0x61000000 0 0x400>, >> >> + <0 0x61100000 0 0x3000>, >> >> + <0 0x62000000 0 0x4000>, >> >> + <0 0x62100000 0 0x4000>, >> >> + <0 0x63000000 0 0x400>, >> >> + <0 0x63100000 0 0x3000>, >> >> + <0 0x70b00000 0 0x3000>; >> > >> > There are a lot of reg properties here. Perhaps there needs to be >> > different nodes for the different clock controllers in this SoC? >> > >> >> On Spreadtrum's platform, clocks are basically located in a few >> address areas due to some hardware design issue, that says there're >> more than one kinds of clocks in one address range, and one kind of >> clocks have more than one physical address bases, except ccu_pll and >> ccu_div in this patchset. >> >> We're planning to map the whole device area at one time before >> initializing each of them, once that has been done and upstreamed, I >> will remove these lists of addressed. > > Ok. Does this mean we need to wait for those patches to be sent I don't think that would come out for review in the near future, so I have to keep these ranges of the address listed here for the time being. > out for review? Is it more like certain clks are embedded inside > other devices like display controllers, i2c controllers, etc? Is From what I understand, that's just something like you said. > there any more information I can get on this SoC? I think you may get more information from our dts files [1], if you would like to :) Thanks again, Chunyan [1] https://github.com/sprdlinux/kernel/blob/sp9860g-1h10/arch/arm64/boot/dts/sprd/whale.dtsi > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi b/arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi new file mode 100644 index 0000000..e15bf2d --- /dev/null +++ b/arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi @@ -0,0 +1,67 @@ +/* + * Spreadtrum SC9860 SoC CCU + * + * Copyright (C) 2017, Spreadtrum Communications Inc. + * + * SPDX-License-Identifier: (GPL-2.0+ OR MIT) + */ + +&soc { + ext_26m: ext-26m { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <26000000>; + clock-output-names = "ext-26m"; + }; + + ext_32m_sine0: ext-32m-sine0 { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <32000000>; + clock-output-names = "ext-32m-sine0"; + }; + + ext_32m_sine1: ext-32m-sine1 { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <32000000>; + clock-output-names = "ext-32m-sine1"; + }; + + ext_rco_100m: ext-rco-100m { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <100000000>; + clock-output-names = "ext-rco-100m"; + }; + + ext_32k: ext-32k { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <32768>; + clock-output-names = "ext-32k"; + }; + + ccu: clk { + compatible = "sprd,sc9860-ccu"; + #clock-cells = <1>; + reg = <0 0x20000000 0 0x400>, + <0 0x20210000 0 0x3000>, + <0 0x402b0000 0 0x4000>, + <0 0x402d0000 0 0x400>, + <0 0x402e0000 0 0x4000>, + <0 0x40400000 0 0x400>, + <0 0x40880000 0 0x400>, + <0 0x415e0000 0 0x400>, + <0 0x60200000 0 0x400>, + <0 0x61000000 0 0x400>, + <0 0x61100000 0 0x3000>, + <0 0x62000000 0 0x4000>, + <0 0x62100000 0 0x4000>, + <0 0x63000000 0 0x400>, + <0 0x63100000 0 0x3000>, + <0 0x70b00000 0 0x3000>; + clocks = <&ext_26m>, <&ext_rco_100m>, <&ext_32k>; + clock-names = "ext-26m", "ext-rco-100m", "ext-32k"; + }; +}; diff --git a/arch/arm64/boot/dts/sprd/sc9860.dtsi b/arch/arm64/boot/dts/sprd/sc9860.dtsi index 7b7d8ce..10ff7c6 100644 --- a/arch/arm64/boot/dts/sprd/sc9860.dtsi +++ b/arch/arm64/boot/dts/sprd/sc9860.dtsi @@ -7,7 +7,9 @@ */ #include <dt-bindings/interrupt-controller/arm-gic.h> +#include <dt-bindings/clock/sc9860-ccu.h> #include "whale2.dtsi" +#include "sc9860-ccu.dtsi" / { cpus { diff --git a/arch/arm64/boot/dts/sprd/whale2.dtsi b/arch/arm64/boot/dts/sprd/whale2.dtsi index 7c217c5..9d69b84 100644 --- a/arch/arm64/boot/dts/sprd/whale2.dtsi +++ b/arch/arm64/boot/dts/sprd/whale2.dtsi @@ -59,13 +59,5 @@ status = "disabled"; }; }; - - }; - - ext_26m: ext-26m { - compatible = "fixed-clock"; - #clock-cells = <0>; - clock-frequency = <26000000>; - clock-output-names = "ext_26m"; }; };
Now we have clock driver, so add clock dt for SC9860 platform. This patch also removed "ext-26m" from whale2.dtsi since it is described in sc9860-ccu.dtsi. Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com> --- arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi | 67 ++++++++++++++++++++++++++++++++ arch/arm64/boot/dts/sprd/sc9860.dtsi | 2 + arch/arm64/boot/dts/sprd/whale2.dtsi | 8 ---- 3 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 arch/arm64/boot/dts/sprd/sc9860-ccu.dtsi