| Message ID | 201303120044.29499.heiko@sntech.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12 March 2013 05:14, Heiko Stübner <heiko@sntech.de> wrote: > This is needed to allow looking up previous created clocks when > adding separate aliases to them. > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/clk/samsung/clk.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c > index 1a5de69..7c943f8 100644 > --- a/drivers/clk/samsung/clk.c > +++ b/drivers/clk/samsung/clk.c > @@ -58,11 +58,11 @@ void __init samsung_clk_init(struct device_node *np, void __iomem *base, > { > reg_base = base; > > -#ifdef CONFIG_OF > clk_table = kzalloc(sizeof(struct clk *) * nr_clks, GFP_KERNEL); > if (!clk_table) > panic("could not allocate clock lookup table\n"); This change is fine but one point that should be considered is that on non-dt platforms, the memory allocation of clk_table cannot really be justified just because few clocks require two or more aliases. Instead, the optional alias passed for divider/mux register functions can actually be a list of alias names, the list being terminated by a zero-length string. The clock register helper functions can then loop through that list and register all the aliases. Thanks, Thomas. > > +#ifdef CONFIG_OF > clk_data.clks = clk_table; > clk_data.clk_num = nr_clks; > of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data); > -- > 1.7.2.3 >
Am Dienstag, 12. März 2013, 10:54:47 schrieb Thomas Abraham: > On 12 March 2013 05:14, Heiko Stübner <heiko@sntech.de> wrote: > > This is needed to allow looking up previous created clocks when > > adding separate aliases to them. > > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > --- > > > > drivers/clk/samsung/clk.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c > > index 1a5de69..7c943f8 100644 > > --- a/drivers/clk/samsung/clk.c > > +++ b/drivers/clk/samsung/clk.c > > @@ -58,11 +58,11 @@ void __init samsung_clk_init(struct device_node *np, > > void __iomem *base, > > > > { > > > > reg_base = base; > > > > -#ifdef CONFIG_OF > > > > clk_table = kzalloc(sizeof(struct clk *) * nr_clks, GFP_KERNEL); > > if (!clk_table) > > > > panic("could not allocate clock lookup table\n"); > > This change is fine but one point that should be considered is that on > non-dt platforms, the memory allocation of clk_table cannot really be > justified just because few clocks require two or more aliases. hmm, at least on s3c24xx there are quite a lot of the clocks requiring more than one alias. Also the clk_table is allocated on all dt platforms so these platforms have to handle the (small) memory consumption in any case and non-dt platforms are supposed to die out in the not to distant future. I.e. I'm working on s3c24xx dt support, Tomasz is working on s3c64xx dt support, exynos5 already only has dt support and exynos4 will probably lose non-dt support at some point. And if someone converts the other s5p SoCs to the common clock framework they should already have the means to go to dt directly by then. So the non-dt common-clock path is merely a "short" intermediate step to support smooth transitions to dt making the memory argument in my opinion a moot point :-) Handling the aliases directly also requires adding quit a lot more {MUX/DIV/...}_*A aliases for all the necessary combinations. And in general I also find the readability of separate aliases a lot better - as shown in the s3c2443 clk driver in the second series. And the removal once we only support dt will be a lot cleaner too. Heiko > Instead, the optional alias passed for divider/mux register functions > can actually be a list of alias names, the list being terminated by a > zero-length string. The clock register helper functions can then loop > through that list and register all the aliases. > > +#ifdef CONFIG_OF > > > > clk_data.clks = clk_table; > > clk_data.clk_num = nr_clks; > > of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data); > > > > -- > > 1.7.2.3
Hi, On 03/12/2013 10:54 AM, Thomas Abraham wrote: > This change is fine but one point that should be considered is that on > non-dt platforms, the memory allocation of clk_table cannot really be > justified just because few clocks require two or more aliases. > > Instead, the optional alias passed for divider/mux register functions > can actually be a list of alias names, the list being terminated by a > zero-length string. The clock register helper functions can then loop > through that list and register all the aliases. Heiko's approach look much more clean to me and even for Exynos platform which has currently about 350 clocks and is going to be dt-only starting from 3.10 we are talking about 1.5kB of memory. Other (s3c64/24xx) platforms have much less clocks (70...150). IMHO all clocks aliases should be registered as it is done in Heiko's patch series. And the clocks table could just be freed on non-dt platforms if it is a real issue and anyone is concerned about it. Regards, Sylwester
On 12 March 2013 16:20, Heiko Stübner <heiko@sntech.de> wrote: > Am Dienstag, 12. März 2013, 10:54:47 schrieb Thomas Abraham: >> On 12 March 2013 05:14, Heiko Stübner <heiko@sntech.de> wrote: >> > This is needed to allow looking up previous created clocks when >> > adding separate aliases to them. >> > >> > Signed-off-by: Heiko Stuebner <heiko@sntech.de> >> > --- >> > >> > drivers/clk/samsung/clk.c | 2 +- >> > 1 files changed, 1 insertions(+), 1 deletions(-) >> > >> > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c >> > index 1a5de69..7c943f8 100644 >> > --- a/drivers/clk/samsung/clk.c >> > +++ b/drivers/clk/samsung/clk.c >> > @@ -58,11 +58,11 @@ void __init samsung_clk_init(struct device_node *np, >> > void __iomem *base, >> > >> > { >> > >> > reg_base = base; >> > >> > -#ifdef CONFIG_OF >> > >> > clk_table = kzalloc(sizeof(struct clk *) * nr_clks, GFP_KERNEL); >> > if (!clk_table) >> > >> > panic("could not allocate clock lookup table\n"); >> >> This change is fine but one point that should be considered is that on >> non-dt platforms, the memory allocation of clk_table cannot really be >> justified just because few clocks require two or more aliases. > > hmm, at least on s3c24xx there are quite a lot of the clocks requiring more > than one alias. Also the clk_table is allocated on all dt platforms so these > platforms have to handle the (small) memory consumption in any case and non-dt > platforms are supposed to die out in the not to distant future. > > I.e. I'm working on s3c24xx dt support, Tomasz is working on s3c64xx dt > support, exynos5 already only has dt support and exynos4 will probably lose > non-dt support at some point. And if someone converts the other s5p SoCs to > the common clock framework they should already have the means to go to dt > directly by then. > > So the non-dt common-clock path is merely a "short" intermediate step to > support smooth transitions to dt making the memory argument in my opinion a > moot point :-) > > > Handling the aliases directly also requires adding quit a lot more > {MUX/DIV/...}_*A aliases for all the necessary combinations. > > And in general I also find the readability of separate aliases a lot better - > as shown in the s3c2443 clk driver in the second series. And the removal once > we only support dt will be a lot cleaner too. True, I agree on your comments. For this patch, Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org> > > > Heiko > >> Instead, the optional alias passed for divider/mux register functions >> can actually be a list of alias names, the list being terminated by a >> zero-length string. The clock register helper functions can then loop >> through that list and register all the aliases. > > >> > +#ifdef CONFIG_OF >> > >> > clk_data.clks = clk_table; >> > clk_data.clk_num = nr_clks; >> > of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data); >> > >> > -- >> > 1.7.2.3 >
On 12 March 2013 16:53, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > Hi, > > On 03/12/2013 10:54 AM, Thomas Abraham wrote: >> This change is fine but one point that should be considered is that on >> non-dt platforms, the memory allocation of clk_table cannot really be >> justified just because few clocks require two or more aliases. >> >> Instead, the optional alias passed for divider/mux register functions >> can actually be a list of alias names, the list being terminated by a >> zero-length string. The clock register helper functions can then loop >> through that list and register all the aliases. > > Heiko's approach look much more clean to me and even for Exynos platform > which has currently about 350 clocks and is going to be dt-only starting > from 3.10 we are talking about 1.5kB of memory. Other (s3c64/24xx) > platforms have much less clocks (70...150). > > IMHO all clocks aliases should be registered as it is done in Heiko's > patch series. And the clocks table could just be freed on non-dt > platforms if it is a real issue and anyone is concerned about it. Ok, thanks. And, you mentioned Exynos4 will be dt-only from 3.10. Does that mean we just drop support for universal and nuri non-dt board support? Or, will there be a equivalent dt support added for these boards? Thanks, Thomas. > > Regards, > Sylwester
On 03/12/2013 12:46 PM, Thomas Abraham wrote: > And, you mentioned Exynos4 will be dt-only from 3.10. Does that mean > we just drop support for universal and nuri non-dt board support? Or, > will there be a equivalent dt support added for these boards? I think Tomasz has some initial dts files for Universal_c210 ready, and Nuri could probably just be dropped. But we need Kyungmin's opinion on that. I'm not sure about other boards, they look pretty basic though. So it shouldn't be difficult to replace them with corresponding dts files. Currently there are: arch/arm/mach-exynos/mach-smdk4x12.c arch/arm/mach-exynos/mach-origen.c arch/arm/mach-exynos/mach-nuri.c arch/arm/mach-exynos/mach-universal_c210.c arch/arm/mach-exynos/mach-armlex4210.c arch/arm/mach-exynos/mach-smdkv310.c And there are dts files for: arch/arm/boot/dts/exynos4210-smdkv310.dts arch/arm/boot/dts/exynos4210-origen.dts arch/arm/boot/dts/exynos5250-smdk5250.dts arch/arm/boot/dts/exynos5250-snow.dts arch/arm/boot/dts/exynos4210-trats.dts arch/arm/boot/dts/exynos5440-ssdk5440.dts arch/arm/boot/dts/exynos4412-smdk4412.dts So except Universal_c210 and Nuri the only ones not supporting booting from DT are these two simple boards, which seem to be maintained by Samsung: arch/arm/mach-exynos/mach-smdk4x12.c arch/arm/mach-exynos/mach-armlex4210.c It would be nice to make Exynos DT only in this kernel release. I guess there was enough time to get all boards converted to DT already. -- Regards, Sylwester
On 12 March 2013 19:18, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > On 03/12/2013 12:46 PM, Thomas Abraham wrote: >> And, you mentioned Exynos4 will be dt-only from 3.10. Does that mean >> we just drop support for universal and nuri non-dt board support? Or, >> will there be a equivalent dt support added for these boards? > > I think Tomasz has some initial dts files for Universal_c210 ready, and > Nuri could probably just be dropped. But we need Kyungmin's opinion on > that. > > I'm not sure about other boards, they look pretty basic though. > So it shouldn't be difficult to replace them with corresponding dts > files. Currently there are: > > arch/arm/mach-exynos/mach-smdk4x12.c > arch/arm/mach-exynos/mach-origen.c > arch/arm/mach-exynos/mach-nuri.c > arch/arm/mach-exynos/mach-universal_c210.c > arch/arm/mach-exynos/mach-armlex4210.c > arch/arm/mach-exynos/mach-smdkv310.c > > And there are dts files for: > > arch/arm/boot/dts/exynos4210-smdkv310.dts > arch/arm/boot/dts/exynos4210-origen.dts > arch/arm/boot/dts/exynos5250-smdk5250.dts > arch/arm/boot/dts/exynos5250-snow.dts > arch/arm/boot/dts/exynos4210-trats.dts > arch/arm/boot/dts/exynos5440-ssdk5440.dts > arch/arm/boot/dts/exynos4412-smdk4412.dts > > So except Universal_c210 and Nuri the only ones not supporting booting > from DT are these two simple boards, which seem to be maintained by > Samsung: > > arch/arm/mach-exynos/mach-smdk4x12.c > arch/arm/mach-exynos/mach-armlex4210.c There has not been much board support development for these boards for couple of kernel releases now. > > It would be nice to make Exynos DT only in this kernel release. > I guess there was enough time to get all boards converted to DT > already. Thanks for sharing your thoughts on this. Regards, Thomas. > > -- > > Regards, > Sylwester
Hi All On Tue, Mar 12, 2013 at 7:54 PM, Thomas Abraham <thomas.abraham@linaro.org> wrote: > On 12 March 2013 19:18, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: >> On 03/12/2013 12:46 PM, Thomas Abraham wrote: >>> And, you mentioned Exynos4 will be dt-only from 3.10. Does that mean >>> we just drop support for universal and nuri non-dt board support? Or, >>> will there be a equivalent dt support added for these boards? >> >> I think Tomasz has some initial dts files for Universal_c210 ready, and >> Nuri could probably just be dropped. But we need Kyungmin's opinion on >> that. >> >> I'm not sure about other boards, they look pretty basic though. >> So it shouldn't be difficult to replace them with corresponding dts >> files. Currently there are: >> >> arch/arm/mach-exynos/mach-smdk4x12.c >> arch/arm/mach-exynos/mach-origen.c >> arch/arm/mach-exynos/mach-nuri.c >> arch/arm/mach-exynos/mach-universal_c210.c >> arch/arm/mach-exynos/mach-armlex4210.c >> arch/arm/mach-exynos/mach-smdkv310.c >> >> And there are dts files for: >> >> arch/arm/boot/dts/exynos4210-smdkv310.dts >> arch/arm/boot/dts/exynos4210-origen.dts >> arch/arm/boot/dts/exynos5250-smdk5250.dts >> arch/arm/boot/dts/exynos5250-snow.dts >> arch/arm/boot/dts/exynos4210-trats.dts >> arch/arm/boot/dts/exynos5440-ssdk5440.dts >> arch/arm/boot/dts/exynos4412-smdk4412.dts >> >> So except Universal_c210 and Nuri the only ones not supporting booting >> from DT are these two simple boards, which seem to be maintained by >> Samsung: >> >> arch/arm/mach-exynos/mach-smdk4x12.c >> arch/arm/mach-exynos/mach-armlex4210.c > > There has not been much board support development for these boards for > couple of kernel releases now. > Right, Armlex4210 can be removed as there is no much development happening. And I do not think any one else using it now. I do not know about smdk4x12.c though, >> >> It would be nice to make Exynos DT only in this kernel release. >> I guess there was enough time to get all boards converted to DT >> already. > > Thanks for sharing your thoughts on this. > > Regards, > Thomas. > >> >> -- >> >> 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
> > So except Universal_c210 and Nuri the only ones not supporting booting > from DT are these two simple boards, which seem to be maintained by > Samsung: > > arch/arm/mach-exynos/mach-smdk4x12.c SMDK4412 is already supported through DT (exynos4412-smdk4412.dts) as you already pointed out. I am not sure if anyone is using SMDK4212. Even if it is required, one can easily add a dts file for that board as DT support for 4x12 already exists. arch/arm/boot/dts/exynos4x12.dtsi arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
On Tue, Mar 12, 2013 at 10:48 PM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > On 03/12/2013 12:46 PM, Thomas Abraham wrote: >> And, you mentioned Exynos4 will be dt-only from 3.10. Does that mean >> we just drop support for universal and nuri non-dt board support? Or, >> will there be a equivalent dt support added for these boards? > > I think Tomasz has some initial dts files for Universal_c210 ready, and > Nuri could probably just be dropped. But we need Kyungmin's opinion on > that. Agree. Tomasz will send unviersal DT files soon. Thank you, Kyungmin Park > > I'm not sure about other boards, they look pretty basic though. > So it shouldn't be difficult to replace them with corresponding dts > files. Currently there are: > > arch/arm/mach-exynos/mach-smdk4x12.c > arch/arm/mach-exynos/mach-origen.c > arch/arm/mach-exynos/mach-nuri.c > arch/arm/mach-exynos/mach-universal_c210.c > arch/arm/mach-exynos/mach-armlex4210.c > arch/arm/mach-exynos/mach-smdkv310.c > > And there are dts files for: > > arch/arm/boot/dts/exynos4210-smdkv310.dts > arch/arm/boot/dts/exynos4210-origen.dts > arch/arm/boot/dts/exynos5250-smdk5250.dts > arch/arm/boot/dts/exynos5250-snow.dts > arch/arm/boot/dts/exynos4210-trats.dts > arch/arm/boot/dts/exynos5440-ssdk5440.dts > arch/arm/boot/dts/exynos4412-smdk4412.dts > > So except Universal_c210 and Nuri the only ones not supporting booting > from DT are these two simple boards, which seem to be maintained by > Samsung: > > arch/arm/mach-exynos/mach-smdk4x12.c > arch/arm/mach-exynos/mach-armlex4210.c > > It would be nice to make Exynos DT only in this kernel release. > I guess there was enough time to get all boards converted to DT > already. > > -- > > Regards, > Sylwester > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c index 1a5de69..7c943f8 100644 --- a/drivers/clk/samsung/clk.c +++ b/drivers/clk/samsung/clk.c @@ -58,11 +58,11 @@ void __init samsung_clk_init(struct device_node *np, void __iomem *base, { reg_base = base; -#ifdef CONFIG_OF clk_table = kzalloc(sizeof(struct clk *) * nr_clks, GFP_KERNEL); if (!clk_table) panic("could not allocate clock lookup table\n"); +#ifdef CONFIG_OF clk_data.clks = clk_table; clk_data.clk_num = nr_clks; of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
This is needed to allow looking up previous created clocks when adding separate aliases to them. Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- drivers/clk/samsung/clk.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)