MyungJoo Ham wrote: > > Many MUX and clock dividers have a status bit so that users can wait > until the status is stable. When corresponding registers are accessed > while a clock is not stable, we may suffer from unexpected errors. > > Therefore, we introduce a mechanism to let the operations related with > updating SRC/DIV registers of clksrc-clk wait for the stabilization: > clk_set_parent, clk_set_rate. > > In order to use this feature, the definition of clksrc_clk should > include reg_src_stable or reg_div_stable. With effective rec_src_stable > values, clk_set_parent returns with a stabilized SRC register and > with effective rec_div_stable values, clk_set_rate returns with a > stabilized DIV register. If .reg field is null, its (either SRC or DIV) > register's status is not checked and returned without waiting; i.e., > some MUX/DIV may not need this feature. > > When setting reg_*_stable, .size is used to tell the value of "stable". > If .size = 0, the stable status is 0 and if .size = 1, the stable status > is 1. > > Signed-off-by: MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> > -- > v2: > - Wait-for-stable loop is described at an inline function. > v3: > - Stop using clksrc_reg for stable bits. Use "clkstat_reg" for > them. > > --- > arch/arm/plat-samsung/clock-clksrc.c | 14 +++++++++++++ > arch/arm/plat-samsung/include/plat/clock-clksrc.h | 22 > +++++++++++++++++++++ > 2 files changed, 36 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/plat-samsung/clock-clksrc.c b/arch/arm/plat-samsung/clock- > clksrc.c > index 46d204a..79ae92e 100644 > --- a/arch/arm/plat-samsung/clock-clksrc.c > +++ b/arch/arm/plat-samsung/clock-clksrc.c > @@ -50,6 +50,15 @@ static unsigned long s3c_getrate_clksrc(struct clk *clk) > return rate; > } > > +static inline void s3c_wait_for_stable(struct clkstat_reg *stat) > +{ > + WARN_ON(stat == NULL); if (stat == NULL) return; If don't need to check status, have not become define relevant status register. > + if (stat->reg) { No need?...or need to check stat->reg and stat->shift? > + do { } while (((__raw_readl(stat->reg) >> stat->shift) & 1) > + != stat->stable); I'm still wondering we _really_ need 'stable'...just comments is enough, right now. > + do { } while (((__raw_readl(stat->reg) >> stat->shift) & 1) > + != stat->stable); > + } > +} > + > static int s3c_setrate_clksrc(struct clk *clk, unsigned long rate) > { > struct clksrc_clk *sclk = to_clksrc(clk); > @@ -68,6 +77,8 @@ static int s3c_setrate_clksrc(struct clk *clk, unsigned long rate) > val |= (div - 1) << sclk->reg_div.shift; > __raw_writel(val, reg); > > + s3c_wait_for_stable(&sclk->reg_div_stable); > + > return 0; > } > > @@ -93,6 +104,9 @@ static int s3c_setparent_clksrc(struct clk *clk, struct clk > *parent) > clksrc |= src_nr << sclk->reg_src.shift; > > __raw_writel(clksrc, sclk->reg_src.reg); > + > + s3c_wait_for_stable(&sclk->reg_src_stable); > + > return 0; > } > > diff --git a/arch/arm/plat-samsung/include/plat/clock-clksrc.h b/arch/arm/plat- > samsung/include/plat/clock-clksrc.h > index 50a8ca7..f1d54da 100644 > --- a/arch/arm/plat-samsung/include/plat/clock-clksrc.h > +++ b/arch/arm/plat-samsung/include/plat/clock-clksrc.h > @@ -40,11 +40,30 @@ struct clksrc_reg { > }; > > /** > + * struct clkstat_reg - register definition for clock stat bits > + * @reg: pointer to the register in virtual memory. > + * @shift: the shift in bits to where the bitfield is. > + * @stable: the bit value of stable status. > + */ > +struct clkstat_reg { > + void __iomem *reg; > + unsigned short shift; > + unsigned short stable; If you want to use this, just 'value' is better. But...I think...we don't need this field now. > +}; > + > +/** > * struct clksrc_clk - class of clock for newer style samsung devices. > * @clk: the standard clock representation > * @sources: the sources for this clock > * @reg_src: the register definition for selecting the clock's source > * @reg_div: the register definition for the clock's output divisor > + * @reg_src_stable: the register definition to probe if reg_src is > + * stabilized after the update of reg_src. It is "stabilized" if > + * reg[shift] == size. If reg == NULL, this stable reg is not looked > + * up. Thus, in S5PV210, size is usually 0. > + * @reg_div_stable: the register definition to probe if reg_div is > + * stabilized after the update of reg_div. Same mechanism with > + * reg_src_stable. > * > * This clock implements the features required by the newer SoCs where > * the standard clock block provides an input mux and a post-mux divisor > @@ -61,6 +80,9 @@ struct clksrc_clk { > > struct clksrc_reg reg_src; > struct clksrc_reg reg_div; > + > + struct clkstat_reg reg_src_stable; > + struct clkstat_reg reg_div_stable; > }; > > /** > -- Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@xxxxxxxxxxx>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html