* [PATCH] ARM: SAMSUNG: s3c_set_clksrc returns for single parent clock. @ 2010-01-18 0:38 Thomas Abraham 2010-01-18 1:10 ` Ben Dooks 0 siblings, 1 reply; 5+ messages in thread From: Thomas Abraham @ 2010-01-18 0:38 UTC (permalink / raw) To: linux-samsung-soc; +Cc: ben-linux, Thomas Abraham The function s3c_set_clksrc should return if the clksrc_clk clock has only one possible parent clock. Signed-of-by: Thomas Abraham <thomas.ab@samsung.com> --- arch/arm/plat-samsung/clock-clksrc.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/plat-samsung/clock-clksrc.c b/arch/arm/plat-samsung/clock-clksrc.c index 33c633a..80da4c4 100644 --- a/arch/arm/plat-samsung/clock-clksrc.c +++ b/arch/arm/plat-samsung/clock-clksrc.c @@ -131,9 +131,10 @@ void __init_or_cpufreq s3c_set_clksrc(struct clksrc_clk *clk, bool announce) u32 mask = bit_mask(clk->reg_src.shift, clk->reg_src.size); u32 clksrc = 0; - if (clk->reg_src.reg) - clksrc = __raw_readl(clk->reg_src.reg); + if (!clk->reg_src.reg) + return; + clksrc = __raw_readl(clk->reg_src.reg); clksrc &= mask; clksrc >>= clk->reg_src.shift; -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: SAMSUNG: s3c_set_clksrc returns for single parent clock. 2010-01-18 0:38 [PATCH] ARM: SAMSUNG: s3c_set_clksrc returns for single parent clock Thomas Abraham @ 2010-01-18 1:10 ` Ben Dooks 2010-01-18 1:34 ` Thomas Abraham 0 siblings, 1 reply; 5+ messages in thread From: Ben Dooks @ 2010-01-18 1:10 UTC (permalink / raw) To: Thomas Abraham; +Cc: linux-samsung-soc, ben-linux On Mon, Jan 18, 2010 at 09:38:17AM +0900, Thomas Abraham wrote: > The function s3c_set_clksrc should return if the clksrc_clk clock has > only one possible parent clock. Does this actually relate to a real bug or is this something that you've come across whilst reading the code? >From arch/arm/plat-samnsung/clock-clksrc.c: 181 if (!clksrc->reg_div.reg) 182 clksrc->clk.ops = &clksrc_ops_nodiv; 183 else if (!clksrc->reg_src.reg) 184 clksrc->clk.ops = &clksrc_ops_nosrc; 185 else 186 clksrc->clk.ops = &clksrc_ops; So we change the ops dependant on which register(s) we have availalbe. > Signed-of-by: Thomas Abraham <thomas.ab@samsung.com> > --- > arch/arm/plat-samsung/clock-clksrc.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/plat-samsung/clock-clksrc.c b/arch/arm/plat-samsung/clock-clksrc.c > index 33c633a..80da4c4 100644 > --- a/arch/arm/plat-samsung/clock-clksrc.c > +++ b/arch/arm/plat-samsung/clock-clksrc.c > @@ -131,9 +131,10 @@ void __init_or_cpufreq s3c_set_clksrc(struct clksrc_clk *clk, bool announce) > u32 mask = bit_mask(clk->reg_src.shift, clk->reg_src.size); > u32 clksrc = 0; > > - if (clk->reg_src.reg) > - clksrc = __raw_readl(clk->reg_src.reg); > + if (!clk->reg_src.reg) > + return; > > + clksrc = __raw_readl(clk->reg_src.reg); > clksrc &= mask; > clksrc >>= clk->reg_src.shift; > > -- > 1.6.3.3 > -- -- Ben Q: What's a light-year? A: One-third less calories than a regular year. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: SAMSUNG: s3c_set_clksrc returns for single parent clock. 2010-01-18 1:10 ` Ben Dooks @ 2010-01-18 1:34 ` Thomas Abraham 2010-01-18 1:46 ` Ben Dooks 0 siblings, 1 reply; 5+ messages in thread From: Thomas Abraham @ 2010-01-18 1:34 UTC (permalink / raw) To: Ben Dooks; +Cc: linux-samsung-soc Hi Ben, On Mon, Jan 18, 2010 at 10:10 AM, Ben Dooks <ben-linux@fluff.org> wrote: > On Mon, Jan 18, 2010 at 09:38:17AM +0900, Thomas Abraham wrote: >> The function s3c_set_clksrc should return if the clksrc_clk clock has >> only one possible parent clock. > > Does this actually relate to a real bug or is this something that you've > come across whilst reading the code? This is required in cases where a clock has a divider but no mux. In s3c_register_clksrc function, after the default set of clock operations are assigned, the parent clock set function s3c_set_clksrc is called. For clocks that do not have mux, the call to s3c_set_clksrc is incorrect. > > From arch/arm/plat-samnsung/clock-clksrc.c: > > 181 if (!clksrc->reg_div.reg) > 182 clksrc->clk.ops = &clksrc_ops_nodiv; > 183 else if (!clksrc->reg_src.reg) > 184 clksrc->clk.ops = &clksrc_ops_nosrc; > 185 else > 186 clksrc->clk.ops = &clksrc_ops; > > So we change the ops dependant on which register(s) we have availalbe. > >> Signed-of-by: Thomas Abraham <thomas.ab@samsung.com> >> --- >> arch/arm/plat-samsung/clock-clksrc.c | 5 +++-- >> 1 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/plat-samsung/clock-clksrc.c b/arch/arm/plat-samsung/clock-clksrc.c >> index 33c633a..80da4c4 100644 >> --- a/arch/arm/plat-samsung/clock-clksrc.c >> +++ b/arch/arm/plat-samsung/clock-clksrc.c >> @@ -131,9 +131,10 @@ void __init_or_cpufreq s3c_set_clksrc(struct clksrc_clk *clk, bool announce) >> u32 mask = bit_mask(clk->reg_src.shift, clk->reg_src.size); >> u32 clksrc = 0; >> >> - if (clk->reg_src.reg) >> - clksrc = __raw_readl(clk->reg_src.reg); >> + if (!clk->reg_src.reg) >> + return; >> >> + clksrc = __raw_readl(clk->reg_src.reg); >> clksrc &= mask; >> clksrc >>= clk->reg_src.shift; >> >> -- >> 1.6.3.3 >> > > -- > -- > Ben > > Q: What's a light-year? > A: One-third less calories than a regular year. > > -- > 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 > Thanks, Thomas. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: SAMSUNG: s3c_set_clksrc returns for single parent clock. 2010-01-18 1:34 ` Thomas Abraham @ 2010-01-18 1:46 ` Ben Dooks 2010-01-18 1:59 ` Ben Dooks 0 siblings, 1 reply; 5+ messages in thread From: Ben Dooks @ 2010-01-18 1:46 UTC (permalink / raw) To: Thomas Abraham; +Cc: Ben Dooks, linux-samsung-soc On Mon, Jan 18, 2010 at 10:34:29AM +0900, Thomas Abraham wrote: > Hi Ben, > > On Mon, Jan 18, 2010 at 10:10 AM, Ben Dooks <ben-linux@fluff.org> wrote: > > On Mon, Jan 18, 2010 at 09:38:17AM +0900, Thomas Abraham wrote: > >> The function s3c_set_clksrc should return if the clksrc_clk clock has > >> only one possible parent clock. > > > > Does this actually relate to a real bug or is this something that you've > > come across whilst reading the code? > > This is required in cases where a clock has a divider but no mux. In > s3c_register_clksrc function, after the default set of clock > operations are assigned, the parent clock set function s3c_set_clksrc > is called. For clocks that do not have mux, the call to s3c_set_clksrc > is incorrect. If you have a look at s3c_set_clksrc() , you will find it already checks to see if clk->reg_src.reg is set. It also doesn't call the setparent directly either, as it only needs to set clk->clk.parent field before exiting. 132 u32 clksrc = 0; 133 134 if (clk->reg_src.reg) 135 clksrc = __raw_readl(clk->reg_src.reg); 136 137 clksrc &= mask; 138 clksrc >>= clk->reg_src.shift; 139 140 if (clksrc > srcs->nr_sources || !srcs->sources[clksrc]) { 141 printk(KERN_ERR "%s: bad source %d\n", 142 clk->clk.name, clksrc); 143 return; 144 } 145 146 clk->clk.parent = srcs->sources[clksrc]; This allows us to do some basic verification that the clksrc's sources data is valud (such that srcs->nr_sources at least contains one source and that it is not NULL). > > > > From arch/arm/plat-samnsung/clock-clksrc.c: > > > > 181 if (!clksrc->reg_div.reg) > > 182 clksrc->clk.ops = &clksrc_ops_nodiv; > > 183 else if (!clksrc->reg_src.reg) > > 184 clksrc->clk.ops = &clksrc_ops_nosrc; > > 185 else > > 186 clksrc->clk.ops = &clksrc_ops; > > > > So we change the ops dependant on which register(s) we have availalbe. > > > >> Signed-of-by: Thomas Abraham <thomas.ab@samsung.com> > >> --- > >> arch/arm/plat-samsung/clock-clksrc.c | 5 +++-- > >> 1 files changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/arm/plat-samsung/clock-clksrc.c b/arch/arm/plat-samsung/clock-clksrc.c > >> index 33c633a..80da4c4 100644 > >> --- a/arch/arm/plat-samsung/clock-clksrc.c > >> +++ b/arch/arm/plat-samsung/clock-clksrc.c > >> @@ -131,9 +131,10 @@ void __init_or_cpufreq s3c_set_clksrc(struct clksrc_clk *clk, bool announce) > >> u32 mask = bit_mask(clk->reg_src.shift, clk->reg_src.size); > >> u32 clksrc = 0; > >> > >> - if (clk->reg_src.reg) > >> - clksrc = __raw_readl(clk->reg_src.reg); > >> + if (!clk->reg_src.reg) > >> + return; > >> > >> + clksrc = __raw_readl(clk->reg_src.reg); > >> clksrc &= mask; > >> clksrc >>= clk->reg_src.shift; > >> > >> -- > >> 1.6.3.3 > >> > > > > -- > > -- > > Ben > > > > Q: What's a light-year? > > A: One-third less calories than a regular year. > > > > -- > > 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 > > > > > Thanks, > Thomas. > -- > 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 -- -- Ben Q: What's a light-year? A: One-third less calories than a regular year. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: SAMSUNG: s3c_set_clksrc returns for single parent clock. 2010-01-18 1:46 ` Ben Dooks @ 2010-01-18 1:59 ` Ben Dooks 0 siblings, 0 replies; 5+ messages in thread From: Ben Dooks @ 2010-01-18 1:59 UTC (permalink / raw) To: Ben Dooks; +Cc: Thomas Abraham, linux-samsung-soc On Mon, Jan 18, 2010 at 01:46:30AM +0000, Ben Dooks wrote: > On Mon, Jan 18, 2010 at 10:34:29AM +0900, Thomas Abraham wrote: > > Hi Ben, > > > > On Mon, Jan 18, 2010 at 10:10 AM, Ben Dooks <ben-linux@fluff.org> wrote: > > > On Mon, Jan 18, 2010 at 09:38:17AM +0900, Thomas Abraham wrote: > > >> The function s3c_set_clksrc should return if the clksrc_clk clock has > > >> only one possible parent clock. > > > > > > Does this actually relate to a real bug or is this something that you've > > > come across whilst reading the code? > > > > This is required in cases where a clock has a divider but no mux. In > > s3c_register_clksrc function, after the default set of clock > > operations are assigned, the parent clock set function s3c_set_clksrc > > is called. For clocks that do not have mux, the call to s3c_set_clksrc > > is incorrect. > > If you have a look at s3c_set_clksrc() , you will find it already > checks to see if clk->reg_src.reg is set. It also doesn't call the > setparent directly either, as it only needs to set clk->clk.parent > field before exiting. > > 132 u32 clksrc = 0; > 133 > 134 if (clk->reg_src.reg) > 135 clksrc = __raw_readl(clk->reg_src.reg); > 136 > 137 clksrc &= mask; > 138 clksrc >>= clk->reg_src.shift; > 139 > 140 if (clksrc > srcs->nr_sources || !srcs->sources[clksrc]) { > 141 printk(KERN_ERR "%s: bad source %d\n", > 142 clk->clk.name, clksrc); > 143 return; > 144 } > 145 > 146 clk->clk.parent = srcs->sources[clksrc]; > > This allows us to do some basic verification that the clksrc's sources > data is valud (such that srcs->nr_sources at least contains one source > and that it is not NULL). From IRL conversation, it seems the problem is that if we do not have reg_src.reg set at line 134 then we assume that the clksrc has a srcs array to check. We should change the regs_src.reg code to check whether the clk->clk.parent is already set and either return or print a warnign if it is not set. > > > > > > From arch/arm/plat-samnsung/clock-clksrc.c: > > > > > > 181 if (!clksrc->reg_div.reg) > > > 182 clksrc->clk.ops = &clksrc_ops_nodiv; > > > 183 else if (!clksrc->reg_src.reg) > > > 184 clksrc->clk.ops = &clksrc_ops_nosrc; > > > 185 else > > > 186 clksrc->clk.ops = &clksrc_ops; > > > > > > So we change the ops dependant on which register(s) we have availalbe. > > > > > >> Signed-of-by: Thomas Abraham <thomas.ab@samsung.com> > > >> --- > > >> arch/arm/plat-samsung/clock-clksrc.c | 5 +++-- > > >> 1 files changed, 3 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/arch/arm/plat-samsung/clock-clksrc.c b/arch/arm/plat-samsung/clock-clksrc.c > > >> index 33c633a..80da4c4 100644 > > >> --- a/arch/arm/plat-samsung/clock-clksrc.c > > >> +++ b/arch/arm/plat-samsung/clock-clksrc.c > > >> @@ -131,9 +131,10 @@ void __init_or_cpufreq s3c_set_clksrc(struct clksrc_clk *clk, bool announce) > > >> u32 mask = bit_mask(clk->reg_src.shift, clk->reg_src.size); > > >> u32 clksrc = 0; > > >> > > >> - if (clk->reg_src.reg) > > >> - clksrc = __raw_readl(clk->reg_src.reg); > > >> + if (!clk->reg_src.reg) > > >> + return; > > >> > > >> + clksrc = __raw_readl(clk->reg_src.reg); > > >> clksrc &= mask; > > >> clksrc >>= clk->reg_src.shift; > > >> > > >> -- > > >> 1.6.3.3 > > >> > > > > > > -- > > > -- > > > Ben > > > > > > Q: What's a light-year? > > > A: One-third less calories than a regular year. > > > > > > -- > > > 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 > > > > > > > > > Thanks, > > Thomas. > > -- > > 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 > > -- > -- > Ben > > Q: What's a light-year? > A: One-third less calories than a regular year. > -- -- Ben Q: What's a light-year? A: One-third less calories than a regular year. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-01-18 1:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-18 0:38 [PATCH] ARM: SAMSUNG: s3c_set_clksrc returns for single parent clock Thomas Abraham 2010-01-18 1:10 ` Ben Dooks 2010-01-18 1:34 ` Thomas Abraham 2010-01-18 1:46 ` Ben Dooks 2010-01-18 1:59 ` Ben Dooks
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.