| Message ID | 1417073716-22997-12-git-send-email-cw00.choi@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thursday 27 November 2014 16:35:08 Chanwoo Choi wrote: > + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" > + and "samsung,exynos5433-cmu-bus2" - clock controller compatible for CMU_BUS > + which generates global data buses clock and global peripheral buses clock. > > - reg: physical base address of the controller and length of memory mapped > region. > This looks like you are duplicating the bindings and the code, but it's really the same hardware multiple times with minor variations that you should be able to describe properly here. Why not make three nodes with the same compatible string and have them handled by the same code? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Arnd, On 11/27/2014 08:41 PM, Arnd Bergmann wrote: > On Thursday 27 November 2014 16:35:08 Chanwoo Choi wrote: >> + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" >> + and "samsung,exynos5433-cmu-bus2" - clock controller compatible for CMU_BUS >> + which generates global data buses clock and global peripheral buses clock. >> >> - reg: physical base address of the controller and length of memory mapped >> region. >> > > This looks like you are duplicating the bindings and the code, but > it's really the same hardware multiple times with minor variations > that you should be able to describe properly here. Why not make > three nodes with the same compatible string and have them handled > by the same code? Each CMU_BUSx domain of Exynos5433 have different base address as following: - CMU_BUS0's base address and range : 0x1360_0000 ~ 0x1360_0b04 - CMU_BUS1's base address and range : 0x1480_0000 ~ 0x1480_0b04 - CMU_BUS2's base address and range : 0x1340_0000 ~ 0x1340_0b04 So, I implement CMU_BUSx domain which has each compatible string. Best Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 27/11/14 12:56, Chanwoo Choi wrote: > On 11/27/2014 08:41 PM, Arnd Bergmann wrote: >> > On Thursday 27 November 2014 16:35:08 Chanwoo Choi wrote: >>> >> + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" >>> >> + and "samsung,exynos5433-cmu-bus2" - clock controller compatible for CMU_BUS >>> >> + which generates global data buses clock and global peripheral buses clock. >>> >> >>> >> - reg: physical base address of the controller and length of memory mapped >>> >> region. >>> >> >> > >> > This looks like you are duplicating the bindings and the code, but >> > it's really the same hardware multiple times with minor variations >> > that you should be able to describe properly here. Why not make >> > three nodes with the same compatible string and have them handled >> > by the same code? > > Each CMU_BUSx domain of Exynos5433 have different base address as following: > - CMU_BUS0's base address and range : 0x1360_0000 ~ 0x1360_0b04 > - CMU_BUS1's base address and range : 0x1480_0000 ~ 0x1480_0b04 > - CMU_BUS2's base address and range : 0x1340_0000 ~ 0x1340_0b04 > > So, I implement CMU_BUSx domain which has each compatible string. You can always have multiple entries in the reg property. I've done something like this for the exynos4415 CMU_ISPx units: cmu_isp: clock-controller@12060000 { compatible = "samsung,exynos4415-cmu-isp"; reg = <0x12060000 0xB10>, <0x12070000 0xB10>; #clock-cells = <1>; assigned-clocks = <&cmu CLK_FOUT_ISP_PLL>; assigned-clock-rates = <300000000>; }; -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, On 11/27/2014 09:12 PM, Sylwester Nawrocki wrote: > Hi, > > On 27/11/14 12:56, Chanwoo Choi wrote: >> On 11/27/2014 08:41 PM, Arnd Bergmann wrote: >>>> On Thursday 27 November 2014 16:35:08 Chanwoo Choi wrote: >>>>>> + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" >>>>>> + and "samsung,exynos5433-cmu-bus2" - clock controller compatible for CMU_BUS >>>>>> + which generates global data buses clock and global peripheral buses clock. >>>>>> >>>>>> - reg: physical base address of the controller and length of memory mapped >>>>>> region. >>>>>> >>>> >>>> This looks like you are duplicating the bindings and the code, but >>>> it's really the same hardware multiple times with minor variations >>>> that you should be able to describe properly here. Why not make >>>> three nodes with the same compatible string and have them handled >>>> by the same code? >> >> Each CMU_BUSx domain of Exynos5433 have different base address as following: >> - CMU_BUS0's base address and range : 0x1360_0000 ~ 0x1360_0b04 >> - CMU_BUS1's base address and range : 0x1480_0000 ~ 0x1480_0b04 >> - CMU_BUS2's base address and range : 0x1340_0000 ~ 0x1340_0b04 >> >> So, I implement CMU_BUSx domain which has each compatible string. > > You can always have multiple entries in the reg property. I've done > something like this for the exynos4415 CMU_ISPx units: > > cmu_isp: clock-controller@12060000 { > compatible = "samsung,exynos4415-cmu-isp"; > reg = <0x12060000 0xB10>, <0x12070000 0xB10>; > #clock-cells = <1>; > > assigned-clocks = <&cmu CLK_FOUT_ISP_PLL>; > assigned-clock-rates = <300000000>; > }; Thanks for your guide. I'll re-implment CMU_BUSx domain according to your guide. Best Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 27 November 2014 13:12:08 Sylwester Nawrocki wrote: > On 27/11/14 12:56, Chanwoo Choi wrote: > > On 11/27/2014 08:41 PM, Arnd Bergmann wrote: > >> > On Thursday 27 November 2014 16:35:08 Chanwoo Choi wrote: > >>> >> + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" > >>> >> + and "samsung,exynos5433-cmu-bus2" - clock controller compatible for CMU_BUS > >>> >> + which generates global data buses clock and global peripheral buses clock. > >>> >> > >>> >> - reg: physical base address of the controller and length of memory mapped > >>> >> region. > >>> >> > >> > > >> > This looks like you are duplicating the bindings and the code, but > >> > it's really the same hardware multiple times with minor variations > >> > that you should be able to describe properly here. Why not make > >> > three nodes with the same compatible string and have them handled > >> > by the same code? > > > > Each CMU_BUSx domain of Exynos5433 have different base address as following: > > - CMU_BUS0's base address and range : 0x1360_0000 ~ 0x1360_0b04 > > - CMU_BUS1's base address and range : 0x1480_0000 ~ 0x1480_0b04 > > - CMU_BUS2's base address and range : 0x1340_0000 ~ 0x1340_0b04 > > > > So, I implement CMU_BUSx domain which has each compatible string. But the base address is in the reg property, not in the compatible property. What I mean is to have multiple nodes like clock-controller@113600000 { reg = <0 0x113600000 0 0x1000>; compatible = "samsung,exynos5433-cmu"; #clock-cells = <1>; }; clock-controller@114800000 { reg = <0 0x114800000 0 0x1000>; compatible = "samsung,exynos5433-cmu"; #clock-cells = <1>; }; The code will just map the local registers for each instance and then provide the clocks of the right instance when asked for it. > You can always have multiple entries in the reg property. I've done > something like this for the exynos4415 CMU_ISPx units: > > cmu_isp: clock-controller@12060000 { > compatible = "samsung,exynos4415-cmu-isp"; > reg = <0x12060000 0xB10>, <0x12070000 0xB10>; > #clock-cells = <1>; > > assigned-clocks = <&cmu CLK_FOUT_ISP_PLL>; > assigned-clock-rates = <300000000>; > }; This is a different problem, this is a clock controller with multiple sets of registers that are all different. In case of the cmu, it seems that they are all the same, you just have multiple copies at different locations, and they are connected to different devices. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Arnd, On 11/27/2014 09:35 PM, Arnd Bergmann wrote: > On Thursday 27 November 2014 13:12:08 Sylwester Nawrocki wrote: >> On 27/11/14 12:56, Chanwoo Choi wrote: >>> On 11/27/2014 08:41 PM, Arnd Bergmann wrote: >>>>> On Thursday 27 November 2014 16:35:08 Chanwoo Choi wrote: >>>>>>> + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" >>>>>>> + and "samsung,exynos5433-cmu-bus2" - clock controller compatible for CMU_BUS >>>>>>> + which generates global data buses clock and global peripheral buses clock. >>>>>>> >>>>>>> - reg: physical base address of the controller and length of memory mapped >>>>>>> region. >>>>>>> >>>>> >>>>> This looks like you are duplicating the bindings and the code, but >>>>> it's really the same hardware multiple times with minor variations >>>>> that you should be able to describe properly here. Why not make >>>>> three nodes with the same compatible string and have them handled >>>>> by the same code? >>> >>> Each CMU_BUSx domain of Exynos5433 have different base address as following: >>> - CMU_BUS0's base address and range : 0x1360_0000 ~ 0x1360_0b04 >>> - CMU_BUS1's base address and range : 0x1480_0000 ~ 0x1480_0b04 >>> - CMU_BUS2's base address and range : 0x1340_0000 ~ 0x1340_0b04 >>> >>> So, I implement CMU_BUSx domain which has each compatible string. > > But the base address is in the reg property, not in the compatible > property. What I mean is to have multiple nodes like The merged clock driver in mainline have different compatible string if base addresss of clock domain is different. So, I implemented each CMU_BUSx domain with different compatible string. > > clock-controller@113600000 { > reg = <0 0x113600000 0 0x1000>; > compatible = "samsung,exynos5433-cmu"; > #clock-cells = <1>; > }; > > clock-controller@114800000 { > reg = <0 0x114800000 0 0x1000>; > compatible = "samsung,exynos5433-cmu"; > #clock-cells = <1>; > }; > > The code will just map the local registers for each instance and then > provide the clocks of the right instance when asked for it. Each clock domain has not the same mux/divider/clock. So, just one compatible string could not support all of clock domains. Best Regards, Chanwoo Choi > >> You can always have multiple entries in the reg property. I've done >> something like this for the exynos4415 CMU_ISPx units: >> >> cmu_isp: clock-controller@12060000 { >> compatible = "samsung,exynos4415-cmu-isp"; >> reg = <0x12060000 0xB10>, <0x12070000 0xB10>; >> #clock-cells = <1>; >> >> assigned-clocks = <&cmu CLK_FOUT_ISP_PLL>; >> assigned-clock-rates = <300000000>; >> }; > > This is a different problem, this is a clock controller with multiple > sets of registers that are all different. In case of the cmu, it seems > that they are all the same, you just have multiple copies at different > locations, and they are connected to different devices. > > Arnd > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 27 November 2014 21:58:53 Chanwoo Choi wrote: > Dear Arnd, > > On 11/27/2014 09:35 PM, Arnd Bergmann wrote: > > On Thursday 27 November 2014 13:12:08 Sylwester Nawrocki wrote: > >> On 27/11/14 12:56, Chanwoo Choi wrote: > >>> On 11/27/2014 08:41 PM, Arnd Bergmann wrote: > >>>>> On Thursday 27 November 2014 16:35:08 Chanwoo Choi wrote: > >>>>>>> + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" > >>>>>>> + and "samsung,exynos5433-cmu-bus2" - clock controller compatible for CMU_BUS > >>>>>>> + which generates global data buses clock and global peripheral buses clock. > >>>>>>> > >>>>>>> - reg: physical base address of the controller and length of memory mapped > >>>>>>> region. > >>>>>>> > >>>>> > >>>>> This looks like you are duplicating the bindings and the code, but > >>>>> it's really the same hardware multiple times with minor variations > >>>>> that you should be able to describe properly here. Why not make > >>>>> three nodes with the same compatible string and have them handled > >>>>> by the same code? > >>> > >>> Each CMU_BUSx domain of Exynos5433 have different base address as following: > >>> - CMU_BUS0's base address and range : 0x1360_0000 ~ 0x1360_0b04 > >>> - CMU_BUS1's base address and range : 0x1480_0000 ~ 0x1480_0b04 > >>> - CMU_BUS2's base address and range : 0x1340_0000 ~ 0x1340_0b04 > >>> > >>> So, I implement CMU_BUSx domain which has each compatible string. > > > > But the base address is in the reg property, not in the compatible > > property. What I mean is to have multiple nodes like > > The merged clock driver in mainline have different compatible string > if base addresss of clock domain is different. So, I implemented each CMU_BUSx domain > with different compatible string. Why? > > clock-controller@113600000 { > > reg = <0 0x113600000 0 0x1000>; > > compatible = "samsung,exynos5433-cmu"; > > #clock-cells = <1>; > > }; > > > > clock-controller@114800000 { > > reg = <0 0x114800000 0 0x1000>; > > compatible = "samsung,exynos5433-cmu"; > > #clock-cells = <1>; > > }; > > > > The code will just map the local registers for each instance and then > > provide the clocks of the right instance when asked for it. > > Each clock domain has not the same mux/divider/clock. So, just one compatible > string could not support all of clock domains. What are the specific differences? I saw that one of them has more outputs than the others but it seemed like a superset, so you just wouldn't be allowed to access the non-connected outputs. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 27 November 2014 22:41:49 Chanwoo Choi wrote: > 2014? 11? 27? ???, Arnd Bergmann<arnd@arndb.de>?? ??? ???: > > > On Thursday 27 November 2014 21:58:53 Chanwoo Choi wrote: > > > Dear Arnd, > > > > > > On 11/27/2014 09:35 PM, Arnd Bergmann wrote: > > > > On Thursday 27 November 2014 13:12:08 Sylwester Nawrocki wrote: > > > >> On 27/11/14 12:56, Chanwoo Choi wrote: > > > >>> On 11/27/2014 08:41 PM, Arnd Bergmann wrote: > > > >>>>> On Thursday 27 November 2014 16:35:08 Chanwoo Choi wrote: > > > >>>>>>> + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" > > > >>>>>>> + and "samsung,exynos5433-cmu-bus2" - clock controller > > compatible for CMU_BUS > > > >>>>>>> + which generates global data buses clock and global > > peripheral buses clock. > > > >>>>>>> > > > >>>>>>> - reg: physical base address of the controller and length of > > memory mapped > > > >>>>>>> region. > > > >>>>>>> > > > >>>>> > > > >>>>> This looks like you are duplicating the bindings and the code, but > > > >>>>> it's really the same hardware multiple times with minor variations > > > >>>>> that you should be able to describe properly here. Why not make > > > >>>>> three nodes with the same compatible string and have them handled > > > >>>>> by the same code? > > > >>> > > > >>> Each CMU_BUSx domain of Exynos5433 have different base address as > > following: > > > >>> - CMU_BUS0's base address and range : 0x1360_0000 ~ 0x1360_0b04 > > > >>> - CMU_BUS1's base address and range : 0x1480_0000 ~ 0x1480_0b04 > > > >>> - CMU_BUS2's base address and range : 0x1340_0000 ~ 0x1340_0b04 > > > >>> > > > >>> So, I implement CMU_BUSx domain which has each compatible string. > > > > > > > > But the base address is in the reg property, not in the compatible > > > > property. What I mean is to have multiple nodes like > > > > > > The merged clock driver in mainline have different compatible string > > > if base addresss of clock domain is different. So, I implemented each > > CMU_BUSx domain > > > with different compatible string. > > > > Why? > > > As I explained on below, each clock domain have different clocks. > So, clocks have unique clock name. > > If clock driver use only one compatible for various clock domain, clock > driver have to know the base address of each domain for distinction of > clock domain. I think It is stong dependency between device and driver. No, not at all. You can have lots of clock controllers with the same compatible string defining different instances of the same IP block, e.g. for compatible="fixed-clock". > > > > > > clock-controller@113600000 { > > > > reg = <0 0x113600000 0 0x1000>; > > > > compatible = "samsung,exynos5433-cmu"; > > > > #clock-cells = <1>; > > > > }; > > > > > > > > clock-controller@114800000 { > > > > reg = <0 0x114800000 0 0x1000>; > > > > compatible = "samsung,exynos5433-cmu"; > > > > #clock-cells = <1>; > > > > }; > > > > > > > > The code will just map the local registers for each instance and then > > > > provide the clocks of the right instance when asked for it. > > > > > > Each clock domain has not the same mux/divider/clock. So, just one > > compatible > > > string could not support all of clock domains. > > > > What are the specific differences? > > > > > I'm not sure that difference among clock domains because I think it is > dependent on the opinion of architect of SoC. > > cmu_bus0/1/2 are much similar. Just cmu_bus2 has one more mux/gate clock > than cmu_bus0/1. Yes, that's what I mean. You can simply model that extra mux/gate in the driver, as long as nothing ever tries to access the clock. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 27, 2014 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Thursday 27 November 2014 22:41:49 Chanwoo Choi wrote: >> 2014? 11? 27? ???, Arnd Bergmann<arnd@arndb.de>?? ??? ???: >> >> > On Thursday 27 November 2014 21:58:53 Chanwoo Choi wrote: >> > > Dear Arnd, >> > > >> > > On 11/27/2014 09:35 PM, Arnd Bergmann wrote: >> > > > On Thursday 27 November 2014 13:12:08 Sylwester Nawrocki wrote: >> > > >> On 27/11/14 12:56, Chanwoo Choi wrote: >> > > >>> On 11/27/2014 08:41 PM, Arnd Bergmann wrote: >> > > >>>>> On Thursday 27 November 2014 16:35:08 Chanwoo Choi wrote: >> > > >>>>>>> + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" >> > > >>>>>>> + and "samsung,exynos5433-cmu-bus2" - clock controller >> > compatible for CMU_BUS >> > > >>>>>>> + which generates global data buses clock and global >> > peripheral buses clock. >> > > >>>>>>> >> > > >>>>>>> - reg: physical base address of the controller and length of >> > memory mapped >> > > >>>>>>> region. >> > > >>>>>>> >> > > >>>>> >> > > >>>>> This looks like you are duplicating the bindings and the code, but >> > > >>>>> it's really the same hardware multiple times with minor variations >> > > >>>>> that you should be able to describe properly here. Why not make >> > > >>>>> three nodes with the same compatible string and have them handled >> > > >>>>> by the same code? >> > > >>> >> > > >>> Each CMU_BUSx domain of Exynos5433 have different base address as >> > following: >> > > >>> - CMU_BUS0's base address and range : 0x1360_0000 ~ 0x1360_0b04 >> > > >>> - CMU_BUS1's base address and range : 0x1480_0000 ~ 0x1480_0b04 >> > > >>> - CMU_BUS2's base address and range : 0x1340_0000 ~ 0x1340_0b04 >> > > >>> >> > > >>> So, I implement CMU_BUSx domain which has each compatible string. >> > > > >> > > > But the base address is in the reg property, not in the compatible >> > > > property. What I mean is to have multiple nodes like >> > > >> > > The merged clock driver in mainline have different compatible string >> > > if base addresss of clock domain is different. So, I implemented each >> > CMU_BUSx domain >> > > with different compatible string. >> > >> > Why? >> >> >> As I explained on below, each clock domain have different clocks. >> So, clocks have unique clock name. >> >> If clock driver use only one compatible for various clock domain, clock >> driver have to know the base address of each domain for distinction of >> clock domain. I think It is stong dependency between device and driver. > > No, not at all. You can have lots of clock controllers with the same > compatible string defining different instances of the same IP block, > e.g. for compatible="fixed-clock". But, "fixed-clock" pass all properties from dt file to driver/clk/clk-fixed-rate.c. and "fixed-clock" driver has not the data dependent on h/w. e.g., clock offset, parent clock. > >> > >> > > > clock-controller@113600000 { >> > > > reg = <0 0x113600000 0 0x1000>; >> > > > compatible = "samsung,exynos5433-cmu"; >> > > > #clock-cells = <1>; >> > > > }; >> > > > >> > > > clock-controller@114800000 { >> > > > reg = <0 0x114800000 0 0x1000>; >> > > > compatible = "samsung,exynos5433-cmu"; >> > > > #clock-cells = <1>; >> > > > }; >> > > > >> > > > The code will just map the local registers for each instance and then >> > > > provide the clocks of the right instance when asked for it. >> > > >> > > Each clock domain has not the same mux/divider/clock. So, just one >> > compatible >> > > string could not support all of clock domains. >> > >> > What are the specific differences? >> >> >> >> > I'm not sure that difference among clock domains because I think it is >> dependent on the opinion of architect of SoC. >> >> cmu_bus0/1/2 are much similar. Just cmu_bus2 has one more mux/gate clock >> than cmu_bus0/1. > > Yes, that's what I mean. You can simply model that extra mux/gate > in the driver, as long as nothing ever tries to access the clock. If only use one compatible to support CMU_BUSx domains, I would implement it as following with Sylwester's guide. To Sylwester, Tomaz, Are you agree following method to support CMU_BUSx domains by using one compatible string? +/* + * Register offset definitions for CMU_BUS{0|1} + */ +#define DIV_BUS 0x0600 +#define DIV_STAT_BUS 0x0700 +#define ENABLE_ACLK_BUS 0x0800 +#define ENABLE_PCLK_BUS 0x0900 +#define ENABLE_IP_BUS0 0x0b00 +#define ENABLE_IP_BUS1 0x0b04 + +#define bus_clk_regs(num) \ +static unsigned long bus##num_clk_regs[] __initdata = { \ + DIV_BUS, \ + DIV_STAT_BUS, \ + ENABLE_ACLK_BUS, \ + ENABLE_PCLK_BUS, \ + ENABLE_IP_BUS0, \ + ENABLE_IP_BUS1, \ +}; \ + +#define bus_div_clks(num) \ +static struct samsung_div_clock bus##num_div_clks[] __initdata = { \ + /* DIV_BUS */ \ + DIV(CLK_DIV_PCLK_BUS##num_133, "div_pclk_bus"#num"_133", \ + "aclk_bus"#num"_400", DIV_BUS##num, 0, 3), \ +}; \ + +#define bus_gate_clks(num) \ +static struct samsung_gate_clock bus##num_gate_clks[] __initdata = { \ + /* ENABLE_ACLK_BUS */ \ + GATE(CLK_ACLK_AHB2APB_BUS##num, "aclk_ahb2apb_bus"#num"p", \ + "div_pclk_bus"#num"_133", ENABLE_ACLK_BUS##num, \ + 4, CLK_IGNORE_UNUSED, 0), \ + GATE(CLK_ACLK_BUS##numNP_133, "aclk_bus"#num"np_133", \ + "div_pclk_bus"##num"_133", ENABLE_ACLK_BUS##num,\ + 2, CLK_IGNORE_UNUSED, 0), \ + GATE(CLK_ACLK_BUS##numND_400, "aclk_bus"#num"nd_400", \ + "aclk_bus"#num"_400", ENABLE_ACLK_BUS##num, \ + 0, CLK_IGNORE_UNUSED, 0), \ + \ + /* ENABLE_PCLK_BUS */ \ + GATE(CLK_PCLK_BUS##numSRVND_133, "pclk_bus"#num"srvnd_133", \ + "div_pclk_bus"#num"_133", ENABLE_PCLK_BUS##num, \ + 2, 0, 0), \ + GATE(CLK_PCLK_PMU_BUS##num, "pclk_pmu_bus"#num, \ + "div_pclk_bus"#num"_133", ENABLE_PCLK_BUS##num, \ + 1, CLK_IGNORE_UNUSED, 0), \ + GATE(CLK_PCLK_SYSREG_BUS##num, "pclk_sysreg_bus"#num, \ + "div_pclk_bus"#num"_133", ENABLE_PCLK_BUS##num, \ + 0, 0, 0), \ +}; \ + +#define bus_clk_regs(0) +#define bus_div_clks(0) +#define bus_gate_clks(0) + +#define bus_clk_regs(1) +#define bus_div_clks(1) +#define bus_gate_clks(1) + +static void __init exynos5433_cmu_bus_init(struct device_node *np) +{ + void __iomem *reg_base_bus0, *reg_base_bus1; + + reg_base_bus0 = of_iomap(np, 0); + reg_base_bus1 = of_iomap(np, 1); + + bus0_ctx = samsung_clk_init(np, reg_base_bus0, BUS0_NR_CLKS); + bus1_ctx = samsung_clk_init(np, reg_base_bus0, BUS0_NR_CLKS); + + samsung_clk_register_div(bus0_ctx, bus0_div_clks, + ARRAY_SIZE(bus0_div_clks)); + samsung_clk_register_gate(bus0_ctx, bus0_gate_clks, + ARRAY_SIZE(bus0_gate_clks)); + samsung_clk_register_div(bus1_ctx, bus1_div_clks, + ARRAY_SIZE(bus1_div_clks)); + samsung_clk_register_gate(bus1_ctx, bus1_gate_clks, + ARRAY_SIZE(bus1_gate_clks)); + + samsung_clk_of_provider(np, bus0_ctx); + samsung_clk_of_provider(np, bus1_ctx); + +} +CLK_OF_DECLARE(exynos5433_cmu_bus, "samsung,exynos5433-cmu-bus", + exynos5433_cmu_bus_init); Best Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 28 November 2014 00:17:40 Chanwoo Choi wrote: > > But, "fixed-clock" pass all properties from dt file to > driver/clk/clk-fixed-rate.c. > and "fixed-clock" driver has not the data dependent on h/w. e.g., > clock offset, parent clock. The parent clocks would obviously have to be provided in DT if you do this. I'm not sure what you mean with clock offsets. What would it take to describe that? > >> > > >> > > > clock-controller@113600000 { > >> > > > reg = <0 0x113600000 0 0x1000>; > >> > > > compatible = "samsung,exynos5433-cmu"; > >> > > > #clock-cells = <1>; > >> > > > }; > >> > > > > >> > > > clock-controller@114800000 { > >> > > > reg = <0 0x114800000 0 0x1000>; > >> > > > compatible = "samsung,exynos5433-cmu"; > >> > > > #clock-cells = <1>; > >> > > > }; > >> > > > > >> > > > The code will just map the local registers for each instance and then > >> > > > provide the clocks of the right instance when asked for it. > >> > > > >> > > Each clock domain has not the same mux/divider/clock. So, just one > >> > compatible > >> > > string could not support all of clock domains. > >> > > >> > What are the specific differences? > >> > >> > >> > >> > I'm not sure that difference among clock domains because I think it is > >> dependent on the opinion of architect of SoC. > >> > >> cmu_bus0/1/2 are much similar. Just cmu_bus2 has one more mux/gate clock > >> than cmu_bus0/1. > > > > Yes, that's what I mean. You can simply model that extra mux/gate > > in the driver, as long as nothing ever tries to access the clock. > > If only use one compatible to support CMU_BUSx domains, > I would implement it as following with Sylwester's guide. > > To Sylwester, Tomaz, > Are you agree following method to support CMU_BUSx domains > by using one compatible string? > +#define bus_clk_regs(num) \ > +static unsigned long bus##num_clk_regs[] __initdata = { \ > + DIV_BUS, \ > + DIV_STAT_BUS, \ > + ENABLE_ACLK_BUS, \ > + ENABLE_PCLK_BUS, \ > + ENABLE_IP_BUS0, \ > + ENABLE_IP_BUS1, \ > +}; \ I don't understand why you would need a macro here. Isn't this constant data that you can pass into multiple devices? The use of macros definitely makes it worse than the original patch. > +#define bus_div_clks(num) \ > +static struct samsung_div_clock bus##num_div_clks[] __initdata = { \ > + /* DIV_BUS */ \ > + DIV(CLK_DIV_PCLK_BUS##num_133, "div_pclk_bus"#num"_133", \ > + "aclk_bus"#num"_400", DIV_BUS##num, 0, 3), \ > +}; \ To illustrate my point further: CLK_DIV_PCLK_BUS0/1/2 are all the same, and so are DIV_BUS0/1/2, so you should not need to duplicate the definitions at all but just call them 'CLK_DIV_PCLK_BUS' and 'DIV_BUS'. For the "aclk_bus"#num"_400" and "div_pclk_bus"#num"_133" strings, I don't know what they refer to. Are you sure they have to be unique? > + > +#define bus_clk_regs(0) > +#define bus_div_clks(0) > +#define bus_gate_clks(0) > + > +#define bus_clk_regs(1) > +#define bus_div_clks(1) > +#define bus_gate_clks(1) > + > +static void __init exynos5433_cmu_bus_init(struct device_node *np) > +{ > + void __iomem *reg_base_bus0, *reg_base_bus1; > + > + reg_base_bus0 = of_iomap(np, 0); > + reg_base_bus1 = of_iomap(np, 1); > + > + bus0_ctx = samsung_clk_init(np, reg_base_bus0, BUS0_NR_CLKS); > + bus1_ctx = samsung_clk_init(np, reg_base_bus0, BUS0_NR_CLKS); > + > + samsung_clk_register_div(bus0_ctx, bus0_div_clks, > + ARRAY_SIZE(bus0_div_clks)); > + samsung_clk_register_gate(bus0_ctx, bus0_gate_clks, > + ARRAY_SIZE(bus0_gate_clks)); > + samsung_clk_register_div(bus1_ctx, bus1_div_clks, > + ARRAY_SIZE(bus1_div_clks)); > + samsung_clk_register_gate(bus1_ctx, bus1_gate_clks, > + ARRAY_SIZE(bus1_gate_clks)); > + > + samsung_clk_of_provider(np, bus0_ctx); > + samsung_clk_of_provider(np, bus1_ctx); > + > +} > +CLK_OF_DECLARE(exynos5433_cmu_bus, "samsung,exynos5433-cmu-bus", > + exynos5433_cmu_bus_init); This isn't helpful either: you really have two instances and should not merge them together into one device node. This should look like static void __init exynos5433_cmu_bus_init(struct device_node *np) { void __iomem *reg_base_bus; reg_base_bus = of_iomap(np, 0); bus_ctx = samsung_clk_init(np, reg_base_bus, BUS_NR_CLKS); samsung_clk_register_div(bus_ctx, bus_div_clks, ARRAY_SIZE(bus_div_clks)); samsung_clk_register_gate(bus_ctx, bus_gate_clks, ARRAY_SIZE(bus_gate_clks)); samsung_clk_of_provider(np, bus0_ctx); } and get called three times. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 28, 2014 at 12:33 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 28 November 2014 00:17:40 Chanwoo Choi wrote: >> >> But, "fixed-clock" pass all properties from dt file to >> driver/clk/clk-fixed-rate.c. >> and "fixed-clock" driver has not the data dependent on h/w. e.g., >> clock offset, parent clock. > > The parent clocks would obviously have to be provided in DT if you > do this. I'm not sure what you mean with clock offsets. What would > it take to describe that? > >> >> > >> >> > > > clock-controller@113600000 { >> >> > > > reg = <0 0x113600000 0 0x1000>; >> >> > > > compatible = "samsung,exynos5433-cmu"; >> >> > > > #clock-cells = <1>; >> >> > > > }; >> >> > > > >> >> > > > clock-controller@114800000 { >> >> > > > reg = <0 0x114800000 0 0x1000>; >> >> > > > compatible = "samsung,exynos5433-cmu"; >> >> > > > #clock-cells = <1>; >> >> > > > }; >> >> > > > >> >> > > > The code will just map the local registers for each instance and then >> >> > > > provide the clocks of the right instance when asked for it. >> >> > > >> >> > > Each clock domain has not the same mux/divider/clock. So, just one >> >> > compatible >> >> > > string could not support all of clock domains. >> >> > >> >> > What are the specific differences? >> >> >> >> >> >> >> >> > I'm not sure that difference among clock domains because I think it is >> >> dependent on the opinion of architect of SoC. >> >> >> >> cmu_bus0/1/2 are much similar. Just cmu_bus2 has one more mux/gate clock >> >> than cmu_bus0/1. >> > >> > Yes, that's what I mean. You can simply model that extra mux/gate >> > in the driver, as long as nothing ever tries to access the clock. >> >> If only use one compatible to support CMU_BUSx domains, >> I would implement it as following with Sylwester's guide. >> >> To Sylwester, Tomaz, >> Are you agree following method to support CMU_BUSx domains >> by using one compatible string? > > >> +#define bus_clk_regs(num) \ >> +static unsigned long bus##num_clk_regs[] __initdata = { \ >> + DIV_BUS, \ >> + DIV_STAT_BUS, \ >> + ENABLE_ACLK_BUS, \ >> + ENABLE_PCLK_BUS, \ >> + ENABLE_IP_BUS0, \ >> + ENABLE_IP_BUS1, \ >> +}; \ > > I don't understand why you would need a macro here. Isn't this constant > data that you can pass into multiple devices? The use of macros > definitely makes it worse than the original patch. > >> +#define bus_div_clks(num) \ >> +static struct samsung_div_clock bus##num_div_clks[] __initdata = { \ >> + /* DIV_BUS */ \ >> + DIV(CLK_DIV_PCLK_BUS##num_133, "div_pclk_bus"#num"_133", \ >> + "aclk_bus"#num"_400", DIV_BUS##num, 0, 3), \ >> +}; \ > > To illustrate my point further: CLK_DIV_PCLK_BUS0/1/2 are all the > same, and so are DIV_BUS0/1/2, so you should not need to duplicate > the definitions at all but just call them 'CLK_DIV_PCLK_BUS' > and 'DIV_BUS'. CLK_DIV_PCLK_BUS0/1/2 is not all the same. Each CLK_DIV_PCLK_BUS0/1/2 must need the unique clock number. Because some device may need some clocks by using unique clock number. > > For the "aclk_bus"#num"_400" and "div_pclk_bus"#num"_133" strings, > I don't know what they refer to. Are you sure they have to be unique? Sure. All clocks(mux/divider/gate) must need the unique clock number. Best Regards, Chanwoo Choi > >> + >> +#define bus_clk_regs(0) >> +#define bus_div_clks(0) >> +#define bus_gate_clks(0) >> + >> +#define bus_clk_regs(1) >> +#define bus_div_clks(1) >> +#define bus_gate_clks(1) >> + >> +static void __init exynos5433_cmu_bus_init(struct device_node *np) >> +{ >> + void __iomem *reg_base_bus0, *reg_base_bus1; >> + >> + reg_base_bus0 = of_iomap(np, 0); >> + reg_base_bus1 = of_iomap(np, 1); >> + >> + bus0_ctx = samsung_clk_init(np, reg_base_bus0, BUS0_NR_CLKS); >> + bus1_ctx = samsung_clk_init(np, reg_base_bus0, BUS0_NR_CLKS); >> + >> + samsung_clk_register_div(bus0_ctx, bus0_div_clks, >> + ARRAY_SIZE(bus0_div_clks)); >> + samsung_clk_register_gate(bus0_ctx, bus0_gate_clks, >> + ARRAY_SIZE(bus0_gate_clks)); >> + samsung_clk_register_div(bus1_ctx, bus1_div_clks, >> + ARRAY_SIZE(bus1_div_clks)); >> + samsung_clk_register_gate(bus1_ctx, bus1_gate_clks, >> + ARRAY_SIZE(bus1_gate_clks)); >> + >> + samsung_clk_of_provider(np, bus0_ctx); >> + samsung_clk_of_provider(np, bus1_ctx); >> + >> +} >> +CLK_OF_DECLARE(exynos5433_cmu_bus, "samsung,exynos5433-cmu-bus", >> + exynos5433_cmu_bus_init); > > This isn't helpful either: you really have two instances and should > not merge them together into one device node. This should look like > > static void __init exynos5433_cmu_bus_init(struct device_node *np) > { > void __iomem *reg_base_bus; > > reg_base_bus = of_iomap(np, 0); > > bus_ctx = samsung_clk_init(np, reg_base_bus, BUS_NR_CLKS); > > samsung_clk_register_div(bus_ctx, bus_div_clks, > ARRAY_SIZE(bus_div_clks)); > samsung_clk_register_gate(bus_ctx, bus_gate_clks, > ARRAY_SIZE(bus_gate_clks)); > > samsung_clk_of_provider(np, bus0_ctx); > } > > and get called three times. > > Arnd > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 28 November 2014 00:44:07 Chanwoo Choi wrote: > > > >> +#define bus_div_clks(num) \ > >> +static struct samsung_div_clock bus##num_div_clks[] __initdata = { \ > >> + /* DIV_BUS */ \ > >> + DIV(CLK_DIV_PCLK_BUS##num_133, "div_pclk_bus"#num"_133", \ > >> + "aclk_bus"#num"_400", DIV_BUS##num, 0, 3), \ > >> +}; \ > > > > To illustrate my point further: CLK_DIV_PCLK_BUS0/1/2 are all the > > same, and so are DIV_BUS0/1/2, so you should not need to duplicate > > the definitions at all but just call them 'CLK_DIV_PCLK_BUS' > > and 'DIV_BUS'. > > CLK_DIV_PCLK_BUS0/1/2 is not all the same. > Each CLK_DIV_PCLK_BUS0/1/2 must need the unique clock number. > Because some device may need some clocks by using unique clock number. This is from your original patch: +/* CMU_BUS0 */ +#define CLK_DIV_PCLK_BUS0_133 1 + +#define CLK_ACLK_AHB2APB_BUS0P 2 +#define CLK_ACLK_BUS0NP_133 3 +#define CLK_ACLK_BUS0ND_400 4 +#define CLK_PCLK_BUS0SRVND_133 5 +#define CLK_PCLK_PMU_BUS0 6 +#define CLK_PCLK_SYSREG_BUS0 7 + +#define BUS0_NR_CLK 8 + +/* CMU_BUS1 */ +#define CLK_DIV_PCLK_BUS1_133 1 + +#define CLK_ACLK_AHB2APB_BUS1P 2 +#define CLK_ACLK_BUS1NP_133 3 +#define CLK_ACLK_BUS1ND_400 4 +#define CLK_PCLK_BUS1SRVND_133 5 +#define CLK_PCLK_PMU_BUS1 6 +#define CLK_PCLK_SYSREG_BUS1 7 + +#define BUS1_NR_CLK 8 + +/* CMU_BUS2 */ +#define CLK_MOUT_ACLK_BUS2_400_USER 1 + +#define CLK_DIV_PCLK_BUS2_133 2 + +#define CLK_ACLK_AHB2APB_BUS2P 3 +#define CLK_ACLK_BUS2NP_133 4 +#define CLK_ACLK_BUS2BEND_400 5 +#define CLK_ACLK_BUS2RTND_400 6 +#define CLK_PCLK_BUS2SRVND_133 7 +#define CLK_PCLK_PMU_BUS2 8 +#define CLK_PCLK_SYSREG_BUS2 9 + +#define BUS2_NR_CLK 10 The numbers are arbitrarily assigned, but for bus0 and bus1, they are all identical, while bus2 uses a lightly different numbering, which you could easily change, e.g. by using the numbers you have for bus2 on bus0 and bus1 as well. + * Register offset definitions for CMU_BUS0 + */ +#define DIV_BUS0 0x0600 +#define DIV_STAT_BUS0 0x0700 +#define ENABLE_ACLK_BUS0 0x0800 +#define ENABLE_PCLK_BUS0 0x0900 +#define ENABLE_IP_BUS0 0x0b00 +#define ENABLE_IP_BUS1 0x0b04 + +/* + * Register offset definitions for CMU_BUS1 + */ +#define DIV_BUS1 0x0600 +#define DIV_STAT_BUS1 0x0700 +#define ENABLE_ACLK_BUS1 0x0800 +#define ENABLE_PCLK_BUS1 0x0900 +#define ENABLE_IP_BUS10 0x0b00 +#define ENABLE_IP_BUS11 0x0b04 +/* + * Register offset definitions for CMU_BUS2 + */ +#define MUX_SEL_BUS2 0x0200 +#define MUX_ENABLE_BUS2 0x0300 +#define MUX_STAT_BUS2 0x0400 +#define DIV_BUS2 0x0600 +#define DIV_STAT_BUS2 0x0700 +#define ENABLE_ACLK_BUS2 0x0800 +#define ENABLE_PCLK_BUS2 0x0900 +#define ENABLE_IP_BUS20 0x0b00 +#define ENABLE_IP_BUS21 0x0b04 More importantly, the register offsets are all identical, except that bus2 has the additional MUX_SEL and MUX_ENABLE definitions. It's very obvious that this is the same hardware block in multiple instances. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 28, 2014 at 12:51 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 28 November 2014 00:44:07 Chanwoo Choi wrote: >> > >> >> +#define bus_div_clks(num) \ >> >> +static struct samsung_div_clock bus##num_div_clks[] __initdata = { \ >> >> + /* DIV_BUS */ \ >> >> + DIV(CLK_DIV_PCLK_BUS##num_133, "div_pclk_bus"#num"_133", \ >> >> + "aclk_bus"#num"_400", DIV_BUS##num, 0, 3), \ >> >> +}; \ >> > >> > To illustrate my point further: CLK_DIV_PCLK_BUS0/1/2 are all the >> > same, and so are DIV_BUS0/1/2, so you should not need to duplicate >> > the definitions at all but just call them 'CLK_DIV_PCLK_BUS' >> > and 'DIV_BUS'. >> >> CLK_DIV_PCLK_BUS0/1/2 is not all the same. >> Each CLK_DIV_PCLK_BUS0/1/2 must need the unique clock number. >> Because some device may need some clocks by using unique clock number. > > This is from your original patch: > > +/* CMU_BUS0 */ > +#define CLK_DIV_PCLK_BUS0_133 1 > + > +#define CLK_ACLK_AHB2APB_BUS0P 2 > +#define CLK_ACLK_BUS0NP_133 3 > +#define CLK_ACLK_BUS0ND_400 4 > +#define CLK_PCLK_BUS0SRVND_133 5 > +#define CLK_PCLK_PMU_BUS0 6 > +#define CLK_PCLK_SYSREG_BUS0 7 > + > +#define BUS0_NR_CLK 8 > + > +/* CMU_BUS1 */ > +#define CLK_DIV_PCLK_BUS1_133 1 > + > +#define CLK_ACLK_AHB2APB_BUS1P 2 > +#define CLK_ACLK_BUS1NP_133 3 > +#define CLK_ACLK_BUS1ND_400 4 > +#define CLK_PCLK_BUS1SRVND_133 5 > +#define CLK_PCLK_PMU_BUS1 6 > +#define CLK_PCLK_SYSREG_BUS1 7 > + > +#define BUS1_NR_CLK 8 > + > +/* CMU_BUS2 */ > +#define CLK_MOUT_ACLK_BUS2_400_USER 1 > + > +#define CLK_DIV_PCLK_BUS2_133 2 > + > +#define CLK_ACLK_AHB2APB_BUS2P 3 > +#define CLK_ACLK_BUS2NP_133 4 > +#define CLK_ACLK_BUS2BEND_400 5 > +#define CLK_ACLK_BUS2RTND_400 6 > +#define CLK_PCLK_BUS2SRVND_133 7 > +#define CLK_PCLK_PMU_BUS2 8 > +#define CLK_PCLK_SYSREG_BUS2 9 > + > +#define BUS2_NR_CLK 10 > > > The numbers are arbitrarily assigned, but for bus0 and bus1, > they are all identical, while bus2 uses a lightly different > numbering, which you could easily change, e.g. by using the > numbers you have for bus2 on bus0 and bus1 as well. OK, I'll remove duplicate clock number due to the same on other clock domain. If use only CLK_DIV_PCLK_BUS instead of CLK_DIV_PCLK_BUS0/1/2, I think I have to use three compatible string for each CMU_BUSx domain as following:. clocks = <&cmu_bus0 CLK_DIV_PCLK_BUS>; clocks = <&cmu_bus1 CLK_DIV_PCLK_BUS>; clocks = <&cmu_bus2 CLK_DIV_PCLK_BUS>; > > + * Register offset definitions for CMU_BUS0 > + */ > +#define DIV_BUS0 0x0600 > +#define DIV_STAT_BUS0 0x0700 > +#define ENABLE_ACLK_BUS0 0x0800 > +#define ENABLE_PCLK_BUS0 0x0900 > +#define ENABLE_IP_BUS0 0x0b00 > +#define ENABLE_IP_BUS1 0x0b04 > + > > +/* > + * Register offset definitions for CMU_BUS1 > + */ > +#define DIV_BUS1 0x0600 > +#define DIV_STAT_BUS1 0x0700 > +#define ENABLE_ACLK_BUS1 0x0800 > +#define ENABLE_PCLK_BUS1 0x0900 > +#define ENABLE_IP_BUS10 0x0b00 > +#define ENABLE_IP_BUS11 0x0b04 > > +/* > + * Register offset definitions for CMU_BUS2 > + */ > +#define MUX_SEL_BUS2 0x0200 > +#define MUX_ENABLE_BUS2 0x0300 > +#define MUX_STAT_BUS2 0x0400 > +#define DIV_BUS2 0x0600 > +#define DIV_STAT_BUS2 0x0700 > +#define ENABLE_ACLK_BUS2 0x0800 > +#define ENABLE_PCLK_BUS2 0x0900 > +#define ENABLE_IP_BUS20 0x0b00 > +#define ENABLE_IP_BUS21 0x0b04 > > More importantly, the register offsets are all identical, except that > bus2 has the additional MUX_SEL and MUX_ENABLE definitions. It's very > obvious that this is the same hardware block in multiple instances. You're right. I'll remove duplicate offset definition. Best Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 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/Documentation/devicetree/bindings/clock/exynos5433-clock.txt b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt index 9a6ae75..03ae40a 100644 --- a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt +++ b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt @@ -25,6 +25,9 @@ Required Properties: which generates clocks for Display (DECON/HDMI/DSIM/MIXER) IPs. - "samsung,exynos5433-cmu-aud" - clock controller compatible for CMU_AUD which generates clocks for Cortex-A5/BUS/AUDIO clocks. + - "samsung,exynos5433-cmu-bus0", "samsung,exynos5433-cmu-bus1" + and "samsung,exynos5433-cmu-bus2" - clock controller compatible for CMU_BUS + which generates global data buses clock and global peripheral buses clock. - reg: physical base address of the controller and length of memory mapped region. @@ -94,6 +97,24 @@ Example 1: Examples of clock controller nodes are listed below. #clock-cells = <1>; }; + cmu_bus0: clock-controller@0x13600000 { + compatible = "samsung,exynos5433-cmu-bus0"; + reg = <0x13600000 0x0b04>; + #clock-cells = <1>; + }; + + cmu_bus1: clock-controller@0x14800000 { + compatible = "samsung,exynos5433-cmu-bus1"; + reg = <0x14800000 0x0b04>; + #clock-cells = <1>; + }; + + cmu_bus2: clock-controller@0x13400000 { + compatible = "samsung,exynos5433-cmu-bus2"; + reg = <0x13400000 0x0b04>; + #clock-cells = <1>; + }; + Example 2: UART controller node that consumes the clock generated by the clock controller. diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c index 9f28672..f0975e1 100644 --- a/drivers/clk/samsung/clk-exynos5433.c +++ b/drivers/clk/samsung/clk-exynos5433.c @@ -428,7 +428,7 @@ static struct samsung_div_clock top_div_clks[] __initdata = { DIV_TOP2, 0, 3), /* DIV_TOP3 */ - DIV(CLK_DIV_ACLK_IMEM_SSSX, "div_aclk_imem_sssx", + DIV(CLK_DIV_ACLK_IMEM_SSSX_266, "div_aclk_imem_sssx_266", "mout_bus_pll_user", DIV_TOP3, 24, 3), DIV(CLK_DIV_ACLK_IMEM_200, "div_aclk_imem_200", "mout_bus_pll_user", DIV_TOP3, 20, 3), @@ -443,6 +443,14 @@ static struct samsung_div_clock top_div_clks[] __initdata = { DIV(CLK_DIV_ACLK_PERIS_66_A, "div_aclk_peris_66_a", "mout_bus_pll_user", DIV_TOP3, 0, 3), + /* DIV_TOP4 */ + DIV(CLK_DIV_ACLK_G3D_400, "div_aclk_g3d_400", "mout_bus_pll_user", + DIV_TOP4, 8, 3), + DIV(CLK_DIV_ACLK_BUS0_400, "div_aclk_bus0_400", "mout_aclk_bus0_400", + DIV_TOP4, 4, 3), + DIV(CLK_DIV_ACLK_BUS1_400, "div_aclk_bus1_400", "mout_bus_pll_user", + DIV_TOP4, 0, 3), + /* DIV_TOP_FSYS0 */ DIV(CLK_DIV_SCLK_MMC1_B, "div_sclk_mmc1_b", "div_sclk_mmc1_a", DIV_TOP_FSYS0, 16, 8), @@ -506,6 +514,19 @@ static struct samsung_div_clock top_div_clks[] __initdata = { static struct samsung_gate_clock top_gate_clks[] __initdata = { /* ENABLE_ACLK_TOP */ + GATE(CLK_ACLK_G3D_400, "aclk_g3d_400", "div_aclk_g3d_400", + ENABLE_ACLK_TOP, 30, 0, 0), + GATE(CLK_ACLK_IMEM_SSX_266, "aclk_imem_ssx_266", + "div_aclk_imem_sssx_266", ENABLE_ACLK_TOP, + 29, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_BUS0_400, "aclk_bus0_400", "div_aclk_bus0_400", + ENABLE_ACLK_TOP, 26, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_BUS1_400, "aclk_bus1_400", "div_aclk_bus1_400", + ENABLE_ACLK_TOP, 25, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_IMEM_200, "aclk_imem_200", "div_aclk_imem_266", + ENABLE_ACLK_TOP, 24, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_IMEM_266, "aclk_imem_266", "div_aclk_imem_200", + ENABLE_ACLK_TOP, 23, CLK_IGNORE_UNUSED, 0), GATE(CLK_ACLK_PERIC_66, "aclk_peric_66", "div_aclk_peric_66_b", ENABLE_ACLK_TOP, 22, CLK_IGNORE_UNUSED, 0), GATE(CLK_ACLK_PERIS_66, "aclk_peris_66", "div_aclk_peris_66_b", @@ -2629,3 +2650,205 @@ static void __init exynos5433_cmu_aud_init(struct device_node *np) } CLK_OF_DECLARE(exynos5433_cmu_aud, "samsung,exynos5433-cmu-aud", exynos5433_cmu_aud_init); + + +/* + * Register offset definitions for CMU_BUS0 + */ +#define DIV_BUS0 0x0600 +#define DIV_STAT_BUS0 0x0700 +#define ENABLE_ACLK_BUS0 0x0800 +#define ENABLE_PCLK_BUS0 0x0900 +#define ENABLE_IP_BUS0 0x0b00 +#define ENABLE_IP_BUS1 0x0b04 + +static unsigned long bus0_clk_regs[] __initdata = { + DIV_BUS0, + DIV_STAT_BUS0, + ENABLE_ACLK_BUS0, + ENABLE_PCLK_BUS0, + ENABLE_IP_BUS0, + ENABLE_IP_BUS1, +}; + +static struct samsung_div_clock bus0_div_clks[] __initdata = { + /* DIV_BUS0 */ + DIV(CLK_DIV_PCLK_BUS0_133, "div_pclk_bus0_133", "aclk_bus0_400", + DIV_BUS0, 0, 3), +}; + +static struct samsung_gate_clock bus0_gate_clks[] __initdata = { + /* ENABLE_ACLK_BUS0 */ + GATE(CLK_ACLK_AHB2APB_BUS0P, "aclk_ahb2apb_bus0p", "div_pclk_bus0_133", + ENABLE_ACLK_BUS0, 4, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_BUS0NP_133, "aclk_bus0np_133", "div_pclk_bus0_133", + ENABLE_ACLK_BUS0, 2, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_BUS0ND_400, "aclk_bus0nd_400", "aclk_bus0_400", + ENABLE_ACLK_BUS0, 0, CLK_IGNORE_UNUSED, 0), + + /* ENABLE_PCLK_BUS0 */ + GATE(CLK_PCLK_BUS0SRVND_133, "pclk_bus0srvnd_133", "div_pclk_bus0_133", + ENABLE_PCLK_BUS0, 2, 0, 0), + GATE(CLK_PCLK_PMU_BUS0, "pclk_pmu_bus0", "div_pclk_bus0_133", + ENABLE_PCLK_BUS0, 1, CLK_IGNORE_UNUSED, 0), + GATE(CLK_PCLK_SYSREG_BUS0, "pclk_sysreg_bus0", "div_pclk_bus0_133", + ENABLE_PCLK_BUS0, 0, 0, 0), +}; + +static struct samsung_cmu_info bus0_cmu_info __initdata = { + .div_clks = bus0_div_clks, + .nr_div_clks = ARRAY_SIZE(bus0_div_clks), + .gate_clks = bus0_gate_clks, + .nr_gate_clks = ARRAY_SIZE(bus0_gate_clks), + .nr_clk_ids = BUS0_NR_CLK, + .clk_regs = bus0_clk_regs, + .nr_clk_regs = ARRAY_SIZE(bus0_clk_regs), +}; + +static void __init exynos5433_cmu_bus0_init(struct device_node *np) +{ + samsung_cmu_register_one(np, &bus0_cmu_info); +} +CLK_OF_DECLARE(exynos5433_cmu_bus0, "samsung,exynos5433-cmu-bus0", + exynos5433_cmu_bus0_init); + +/* + * Register offset definitions for CMU_BUS1 + */ +#define DIV_BUS1 0x0600 +#define DIV_STAT_BUS1 0x0700 +#define ENABLE_ACLK_BUS1 0x0800 +#define ENABLE_PCLK_BUS1 0x0900 +#define ENABLE_IP_BUS10 0x0b00 +#define ENABLE_IP_BUS11 0x0b04 + +static unsigned long bus1_clk_regs[] __initdata = { + DIV_BUS1, + DIV_STAT_BUS1, + ENABLE_ACLK_BUS1, + ENABLE_PCLK_BUS1, + ENABLE_IP_BUS10, + ENABLE_IP_BUS11, +}; + +static struct samsung_div_clock bus1_div_clks[] __initdata = { + /* DIV_BUS1 */ + DIV(CLK_DIV_PCLK_BUS1_133, "div_pclk_bus1_133", "aclk_bus1_400", + DIV_BUS1, 0, 3), +}; + +static struct samsung_gate_clock bus1_gate_clks[] __initdata = { + /* ENABLE_ACLK_BUS1 */ + GATE(CLK_ACLK_AHB2APB_BUS1P, "aclk_ahb2apb_bus1p", "div_pclk_bus1_133", + ENABLE_ACLK_BUS1, 4, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_BUS1NP_133, "aclk_bus1np_133", "div_pclk_bus1_133", + ENABLE_ACLK_BUS1, 2, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_BUS1ND_400, "aclk_bus1nd_400", "aclk_bus1_400", + ENABLE_ACLK_BUS1, 0, CLK_IGNORE_UNUSED, 0), + + /* ENABLE_PCLK_BUS1 */ + GATE(CLK_PCLK_BUS1SRVND_133, "pclk_bus1srvnd_133", "div_pclk_bus1_133", + ENABLE_PCLK_BUS1, 2, 0, 0), + GATE(CLK_PCLK_PMU_BUS1, "pclk_pmu_bus1", "div_pclk_bus1_133", + ENABLE_PCLK_BUS1, 1, CLK_IGNORE_UNUSED, 0), + GATE(CLK_PCLK_SYSREG_BUS1, "pclk_sysreg_bus1", "div_pclk_bus1_133", + ENABLE_PCLK_BUS1, 0, 0, 0), +}; + +static struct samsung_cmu_info bus1_cmu_info __initdata = { + .div_clks = bus1_div_clks, + .nr_div_clks = ARRAY_SIZE(bus1_div_clks), + .gate_clks = bus1_gate_clks, + .nr_gate_clks = ARRAY_SIZE(bus1_gate_clks), + .nr_clk_ids = BUS1_NR_CLK, + .clk_regs = bus1_clk_regs, + .nr_clk_regs = ARRAY_SIZE(bus1_clk_regs), +}; + +static void __init exynos5433_cmu_bus1_init(struct device_node *np) +{ + samsung_cmu_register_one(np, &bus1_cmu_info); +} +CLK_OF_DECLARE(exynos5433_cmu_bus1, "samsung,exynos5433-cmu-bus1", + exynos5433_cmu_bus1_init); + +/* + * Register offset definitions for CMU_BUS2 + */ +#define MUX_SEL_BUS2 0x0200 +#define MUX_ENABLE_BUS2 0x0300 +#define MUX_STAT_BUS2 0x0400 +#define DIV_BUS2 0x0600 +#define DIV_STAT_BUS2 0x0700 +#define ENABLE_ACLK_BUS2 0x0800 +#define ENABLE_PCLK_BUS2 0x0900 +#define ENABLE_IP_BUS20 0x0b00 +#define ENABLE_IP_BUS21 0x0b04 + +static unsigned long bus2_clk_regs[] __initdata = { + MUX_SEL_BUS2, + MUX_ENABLE_BUS2, + MUX_STAT_BUS2, + DIV_BUS2, + DIV_STAT_BUS2, + ENABLE_ACLK_BUS2, + ENABLE_PCLK_BUS2, + ENABLE_IP_BUS20, + ENABLE_IP_BUS21, +}; + +/* list of all parent clock list */ +PNAME(mout_aclk_bus2_400_p) = { "fin_pll", "aclk_bus2_400", }; + +static struct samsung_mux_clock bus2_mux_clks[] __initdata = { + /* MUX_SEL_BUS2 */ + MUX(CLK_MOUT_ACLK_BUS2_400_USER, "mout_aclk_bus2_400_user", + mout_aclk_bus2_400_p, MUX_SEL_BUS2, 0, 1), +}; + +static struct samsung_div_clock bus2_div_clks[] __initdata = { + /* DIV_BUS2 */ + DIV(CLK_DIV_PCLK_BUS2_133, "div_pclk_bus2_133", + "mout_aclk_bus2_400_user", DIV_BUS2, 0, 3), +}; + +static struct samsung_gate_clock bus2_gate_clks[] __initdata = { + /* ENABLE_ACLK_BUS2 */ + GATE(CLK_ACLK_AHB2APB_BUS2P, "aclk_ahb2apb_bus2p", "div_pclk_bus2_133", + ENABLE_ACLK_BUS2, 3, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_BUS2NP_133, "aclk_bus2np_133", "div_pclk_bus2_133", + ENABLE_ACLK_BUS2, 2, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_BUS2BEND_400, "aclk_bus2bend_400", + "mout_aclk_bus2_400_user", ENABLE_ACLK_BUS2, + 1, CLK_IGNORE_UNUSED, 0), + GATE(CLK_ACLK_BUS2RTND_400, "aclk_bus2rtnd_400", + "mout_aclk_bus2_400_user", ENABLE_ACLK_BUS2, + 0, CLK_IGNORE_UNUSED, 0), + + /* ENABLE_PCLK_BUS2 */ + GATE(CLK_PCLK_BUS2SRVND_133, "pclk_bus2srvnd_133", "div_pclk_bus2_133", + ENABLE_PCLK_BUS2, 2, 0, 0), + GATE(CLK_PCLK_PMU_BUS2, "pclk_pmu_bus2", "div_pclk_bus2_133", + ENABLE_PCLK_BUS2, 1, CLK_IGNORE_UNUSED, 0), + GATE(CLK_PCLK_SYSREG_BUS2, "pclk_sysreg_bus2", "div_pclk_bus2_133", + ENABLE_PCLK_BUS2, 0, 0, 0), +}; + +static struct samsung_cmu_info bus2_cmu_info __initdata = { + .mux_clks = bus2_mux_clks, + .nr_mux_clks = ARRAY_SIZE(bus2_mux_clks), + .div_clks = bus2_div_clks, + .nr_div_clks = ARRAY_SIZE(bus2_div_clks), + .gate_clks = bus2_gate_clks, + .nr_gate_clks = ARRAY_SIZE(bus2_gate_clks), + .nr_clk_ids = BUS2_NR_CLK, + .clk_regs = bus2_clk_regs, + .nr_clk_regs = ARRAY_SIZE(bus2_clk_regs), +}; + +static void __init exynos5433_cmu_bus2_init(struct device_node *np) +{ + samsung_cmu_register_one(np, &bus2_cmu_info); +} +CLK_OF_DECLARE(exynos5433_cmu_bus2, "samsung,exynos5433-cmu-bus2", + exynos5433_cmu_bus2_init); diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h index e1c848a..56eb8c8 100644 --- a/include/dt-bindings/clock/exynos5433.h +++ b/include/dt-bindings/clock/exynos5433.h @@ -72,7 +72,7 @@ #define CLK_MOUT_SCLK_HDMI_SPDIF 64 #define CLK_DIV_ACLK_FSYS_200 100 -#define CLK_DIV_ACLK_IMEM_SSSX 101 +#define CLK_DIV_ACLK_IMEM_SSSX_266 101 #define CLK_DIV_ACLK_IMEM_200 102 #define CLK_DIV_ACLK_IMEM_266 103 #define CLK_DIV_ACLK_PERIC_66_B 104 @@ -108,6 +108,9 @@ #define CLK_DIV_ACLK_MFC_400 134 #define CLK_DIV_ACLK_G2D_266 135 #define CLK_DIV_ACLK_G2D_400 136 +#define CLK_DIV_ACLK_G3D_400 137 +#define CLK_DIV_ACLK_BUS0_400 138 +#define CLK_DIV_ACLK_BUS1_400 139 #define CLK_ACLK_PERIC_66 200 #define CLK_ACLK_PERIS_66 201 @@ -131,8 +134,14 @@ #define CLK_SCLK_AUDIO0 219 #define CLK_ACLK_G2D_266 220 #define CLK_ACLK_G2D_400 221 +#define CLK_ACLK_G3D_400 222 +#define CLK_ACLK_IMEM_SSX_266 223 +#define CLK_ACLK_BUS0_400 224 +#define CLK_ACLK_BUS1_400 225 +#define CLK_ACLK_IMEM_200 226 +#define CLK_ACLK_IMEM_266 227 -#define TOP_NR_CLK 222 +#define TOP_NR_CLK 228 /* CMU_CPIF */ #define CLK_FOUT_MPHY_PLL 1 @@ -680,4 +689,43 @@ #define AUD_NR_CLK 48 +/* CMU_BUS0 */ +#define CLK_DIV_PCLK_BUS0_133 1 + +#define CLK_ACLK_AHB2APB_BUS0P 2 +#define CLK_ACLK_BUS0NP_133 3 +#define CLK_ACLK_BUS0ND_400 4 +#define CLK_PCLK_BUS0SRVND_133 5 +#define CLK_PCLK_PMU_BUS0 6 +#define CLK_PCLK_SYSREG_BUS0 7 + +#define BUS0_NR_CLK 8 + +/* CMU_BUS1 */ +#define CLK_DIV_PCLK_BUS1_133 1 + +#define CLK_ACLK_AHB2APB_BUS1P 2 +#define CLK_ACLK_BUS1NP_133 3 +#define CLK_ACLK_BUS1ND_400 4 +#define CLK_PCLK_BUS1SRVND_133 5 +#define CLK_PCLK_PMU_BUS1 6 +#define CLK_PCLK_SYSREG_BUS1 7 + +#define BUS1_NR_CLK 8 + +/* CMU_BUS2 */ +#define CLK_MOUT_ACLK_BUS2_400_USER 1 + +#define CLK_DIV_PCLK_BUS2_133 2 + +#define CLK_ACLK_AHB2APB_BUS2P 3 +#define CLK_ACLK_BUS2NP_133 4 +#define CLK_ACLK_BUS2BEND_400 5 +#define CLK_ACLK_BUS2RTND_400 6 +#define CLK_PCLK_BUS2SRVND_133 7 +#define CLK_PCLK_PMU_BUS2 8 +#define CLK_PCLK_SYSREG_BUS2 9 + +#define BUS2_NR_CLK 10 + #endif /* _DT_BINDINGS_CLOCK_EXYNOS5433_H */
This patch adds the mux/divider/gate clocks for CMU_BUS{0|1|2} domains which contain global data buses clocked at up the 400MHz. These blocks transfer data between DRAM and various sub-blocks. These clock domains also contain global peripheral buses clocked at 67/111/200/222/266/333/400 MHz and used for regiser accesses. Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> Cc: Tomasz Figa <tomasz.figa@gmail.com> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> Acked-by: Inki Dae <inki.dae@samsung.com> Acked-by: Geunsik Lim <geunsik.lim@samsung.com> --- .../devicetree/bindings/clock/exynos5433-clock.txt | 21 ++ drivers/clk/samsung/clk-exynos5433.c | 225 ++++++++++++++++++++- include/dt-bindings/clock/exynos5433.h | 52 ++++- 3 files changed, 295 insertions(+), 3 deletions(-)