| Message ID | 5855371.5GIoOu8EnE@phil (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello. On 04/23/2014 11:34 PM, Heiko Stübner wrote: > The s3c24xx cpufreq driver needs to change the mpll speed and was doing > this by writing raw values from a translation table into the MPLLCON > register. > Change this to use a regular clk_set_rate call when using the common > clock framework and only write the raw value in the samsung_clock case. > To not needing to create additional infrastructure for this, the mpll clock > is requested at the first call to s3c2410_set_fvco(). > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > arch/arm/mach-s3c24xx/cpufreq-utils.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > diff --git a/arch/arm/mach-s3c24xx/cpufreq-utils.c b/arch/arm/mach-s3c24xx/cpufreq-utils.c > index 2a0aa56..d5e797b 100644 > --- a/arch/arm/mach-s3c24xx/cpufreq-utils.c > +++ b/arch/arm/mach-s3c24xx/cpufreq-utils.c [...] > @@ -60,5 +61,18 @@ void s3c2410_cpufreq_setrefresh(struct s3c_cpufreq_config *cfg) > */ > void s3c2410_set_fvco(struct s3c_cpufreq_config *cfg) > { > +#ifdef CONFIG_SAMSUNG_CLOCK > __raw_writel(cfg->pll.driver_data, S3C2410_MPLLCON); > +#endif > + > +#ifdef CONFIG_COMMON_CLK > + static struct clk *mpll; > + > + if (!mpll) You are testing uninitialized variable. This check wouldn't make much sense even if the variable was initialized. > + mpll = clk_get(NULL, "mpll") > + if (IS_ERR(mpll)) > + return; > + > + clk_set_rate(mpll, cfg->pll.frequency); > +#endif > } WBR, Sergei -- 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 23.04.2014 22:42, Sergei Shtylyov wrote: > Hello. > > On 04/23/2014 11:34 PM, Heiko Stübner wrote: > >> The s3c24xx cpufreq driver needs to change the mpll speed and was doing >> this by writing raw values from a translation table into the MPLLCON >> register. > >> Change this to use a regular clk_set_rate call when using the common >> clock framework and only write the raw value in the samsung_clock case. > >> To not needing to create additional infrastructure for this, the mpll >> clock >> is requested at the first call to s3c2410_set_fvco(). > >> Signed-off-by: Heiko Stuebner <heiko@sntech.de> >> --- >> arch/arm/mach-s3c24xx/cpufreq-utils.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) > >> diff --git a/arch/arm/mach-s3c24xx/cpufreq-utils.c >> b/arch/arm/mach-s3c24xx/cpufreq-utils.c >> index 2a0aa56..d5e797b 100644 >> --- a/arch/arm/mach-s3c24xx/cpufreq-utils.c >> +++ b/arch/arm/mach-s3c24xx/cpufreq-utils.c > [...] >> @@ -60,5 +61,18 @@ void s3c2410_cpufreq_setrefresh(struct >> s3c_cpufreq_config *cfg) >> */ >> void s3c2410_set_fvco(struct s3c_cpufreq_config *cfg) >> { >> +#ifdef CONFIG_SAMSUNG_CLOCK >> __raw_writel(cfg->pll.driver_data, S3C2410_MPLLCON); >> +#endif >> + >> +#ifdef CONFIG_COMMON_CLK >> + static struct clk *mpll; >> + >> + if (!mpll) > > You are testing uninitialized variable. This check wouldn't make > much sense even if the variable was initialized. I should probably add that NULL is considered a valid clock handle by Common Clock Framework. If there is really no way to pass the clock to this function then probably a global variable initialized by some code running earlier than this function could be called would be a better choice. Anyway, Heiko, thanks for working on this. I'll try to review rest of the series soon. (I'm attending the ELC next week, though, so I'm not sure if I find some time then, though.) Best regards, Tomasz -- 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 04/24/2014 12:42 AM, Sergei Shtylyov wrote: >> The s3c24xx cpufreq driver needs to change the mpll speed and was doing >> this by writing raw values from a translation table into the MPLLCON >> register. >> Change this to use a regular clk_set_rate call when using the common >> clock framework and only write the raw value in the samsung_clock case. >> To not needing to create additional infrastructure for this, the mpll clock >> is requested at the first call to s3c2410_set_fvco(). >> Signed-off-by: Heiko Stuebner <heiko@sntech.de> >> --- >> arch/arm/mach-s3c24xx/cpufreq-utils.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> diff --git a/arch/arm/mach-s3c24xx/cpufreq-utils.c >> b/arch/arm/mach-s3c24xx/cpufreq-utils.c >> index 2a0aa56..d5e797b 100644 >> --- a/arch/arm/mach-s3c24xx/cpufreq-utils.c >> +++ b/arch/arm/mach-s3c24xx/cpufreq-utils.c > [...] >> @@ -60,5 +61,18 @@ void s3c2410_cpufreq_setrefresh(struct s3c_cpufreq_config >> *cfg) >> */ >> void s3c2410_set_fvco(struct s3c_cpufreq_config *cfg) >> { >> +#ifdef CONFIG_SAMSUNG_CLOCK >> __raw_writel(cfg->pll.driver_data, S3C2410_MPLLCON); >> +#endif >> + >> +#ifdef CONFIG_COMMON_CLK >> + static struct clk *mpll; >> + >> + if (!mpll) > You are testing uninitialized variable. This check wouldn't make much > sense even if the variable was initialized. Oops, I didn't notice *static*... :-> The code makes more sense now. >> + mpll = clk_get(NULL, "mpll") >> + if (IS_ERR(mpll)) >> + return; >> + >> + clk_set_rate(mpll, cfg->pll.frequency); >> +#endif >> } WBR, Sergei -- 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
Am Mittwoch, 23. April 2014, 22:55:51 schrieb Tomasz Figa: > Hi, > > On 23.04.2014 22:42, Sergei Shtylyov wrote: > > Hello. > > > > On 04/23/2014 11:34 PM, Heiko Stübner wrote: > >> The s3c24xx cpufreq driver needs to change the mpll speed and was doing > >> this by writing raw values from a translation table into the MPLLCON > >> register. > >> > >> Change this to use a regular clk_set_rate call when using the common > >> clock framework and only write the raw value in the samsung_clock case. > >> > >> To not needing to create additional infrastructure for this, the mpll > >> clock > >> is requested at the first call to s3c2410_set_fvco(). > >> > >> Signed-off-by: Heiko Stuebner <heiko@sntech.de> > >> --- > >> > >> arch/arm/mach-s3c24xx/cpufreq-utils.c | 14 ++++++++++++++ > >> 1 file changed, 14 insertions(+) > >> > >> diff --git a/arch/arm/mach-s3c24xx/cpufreq-utils.c > >> b/arch/arm/mach-s3c24xx/cpufreq-utils.c > >> index 2a0aa56..d5e797b 100644 > >> --- a/arch/arm/mach-s3c24xx/cpufreq-utils.c > >> +++ b/arch/arm/mach-s3c24xx/cpufreq-utils.c > > > > [...] > > > >> @@ -60,5 +61,18 @@ void s3c2410_cpufreq_setrefresh(struct > >> s3c_cpufreq_config *cfg) > >> > >> */ > >> > >> void s3c2410_set_fvco(struct s3c_cpufreq_config *cfg) > >> { > >> > >> +#ifdef CONFIG_SAMSUNG_CLOCK > >> > >> __raw_writel(cfg->pll.driver_data, S3C2410_MPLLCON); > >> > >> +#endif > >> + > >> +#ifdef CONFIG_COMMON_CLK > >> + static struct clk *mpll; > >> + > >> + if (!mpll) > >> > > You are testing uninitialized variable. This check wouldn't make > > > > much sense even if the variable was initialized. > > I should probably add that NULL is considered a valid clock handle by > Common Clock Framework. > > If there is really no way to pass the clock to this function then > probably a global variable initialized by some code running earlier than > this function could be called would be a better choice. *grrr* :-) ... ok I'll try to find another way (again) to do this > Anyway, Heiko, thanks for working on this. I'll try to review rest of > the series soon. (I'm attending the ELC next week, though, so I'm not > sure if I find some time then, though.) Just as a reminder, there isn't this much to still review, as you Acked/Reviewed most of the series in v1 and only this patch as well as 2 and 5 still need a review/ack - and the only changes are fixes for your comments ;-) Heiko -- 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
Heiko Stübner wrote: > [...] > > Anyway, Heiko, thanks for working on this. I'll try to review rest of > > the series soon. (I'm attending the ELC next week, though, so I'm not > > sure if I find some time then, though.) > Yeah, this series is really good to us. > Just as a reminder, there isn't this much to still review, as you > Acked/Reviewed most of the series in v1 and only this patch as well as 2 > and 5 > still need a review/ack - and the only changes are fixes for your > comments ;-) > I think so. Since I will apply this series in this week, if any big comments, please let us know. Heiko, thanks for the v2.1 as well. - Kukjin -- 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 06.05.2014 06:27, Kukjin Kim wrote: > Heiko Stübner wrote: >> > > [...] > >>> Anyway, Heiko, thanks for working on this. I'll try to review rest of >>> the series soon. (I'm attending the ELC next week, though, so I'm not >>> sure if I find some time then, though.) >> > Yeah, this series is really good to us. > >> Just as a reminder, there isn't this much to still review, as you >> Acked/Reviewed most of the series in v1 and only this patch as well as 2 >> and 5 >> still need a review/ack - and the only changes are fixes for your >> comments ;-) >> > I think so. Since I will apply this series in this week, if any big > comments, please let us know. > > Heiko, thanks for the v2.1 as well. Just found time to look through this series and it looks fine. So if not applied yet, feel free to add Acked-by: Tomasz Figa <t.figa@samsung.com> to clk: patches. Also we still need to ping DT maintainers, as they should ask patch 4/9 before it is merged. Best regards, Tomasz -- 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 Kukjin, On 06.05.2014 06:27, Kukjin Kim wrote: > Heiko Stübner wrote: >> > > [...] > >>> Anyway, Heiko, thanks for working on this. I'll try to review rest of >>> the series soon. (I'm attending the ELC next week, though, so I'm not >>> sure if I find some time then, though.) >> > Yeah, this series is really good to us. > >> Just as a reminder, there isn't this much to still review, as you >> Acked/Reviewed most of the series in v1 and only this patch as well as 2 >> and 5 >> still need a review/ack - and the only changes are fixes for your >> comments ;-) >> > I think so. Since I will apply this series in this week, if any big > comments, please let us know. This needs to be synchronized with series for Exynos5260 and support for multiple clock providers. Let me prepare a branch for you to pull from my tree with all necessary dependencies. Best regards, Tomasz -- 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/arch/arm/mach-s3c24xx/cpufreq-utils.c b/arch/arm/mach-s3c24xx/cpufreq-utils.c index 2a0aa56..d5e797b 100644 --- a/arch/arm/mach-s3c24xx/cpufreq-utils.c +++ b/arch/arm/mach-s3c24xx/cpufreq-utils.c @@ -14,6 +14,7 @@ #include <linux/errno.h> #include <linux/cpufreq.h> #include <linux/io.h> +#include <linux/clk.h> #include <mach/map.h> #include <mach/regs-clock.h> @@ -60,5 +61,18 @@ void s3c2410_cpufreq_setrefresh(struct s3c_cpufreq_config *cfg) */ void s3c2410_set_fvco(struct s3c_cpufreq_config *cfg) { +#ifdef CONFIG_SAMSUNG_CLOCK __raw_writel(cfg->pll.driver_data, S3C2410_MPLLCON); +#endif + +#ifdef CONFIG_COMMON_CLK + static struct clk *mpll; + + if (!mpll) + mpll = clk_get(NULL, "mpll") + if (IS_ERR(mpll)) + return; + + clk_set_rate(mpll, cfg->pll.frequency); +#endif }
The s3c24xx cpufreq driver needs to change the mpll speed and was doing this by writing raw values from a translation table into the MPLLCON register. Change this to use a regular clk_set_rate call when using the common clock framework and only write the raw value in the samsung_clock case. To not needing to create additional infrastructure for this, the mpll clock is requested at the first call to s3c2410_set_fvco(). Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- arch/arm/mach-s3c24xx/cpufreq-utils.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)