From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2010-08-02 02:51:16
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@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
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(-)
From: Kukjin Kim <hidden> Date: 2010-08-02 05:30:19
MyungJoo Ham wrote:
quoted hunk
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@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
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(-)
If you want to use this, just 'value' is better.
But...I think...we don't need this field now.
quoted hunk
+};++/** * 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
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2010-08-02 12:51:11
On Mon, Aug 2, 2010 at 2:30 PM, Kukjin Kim [off-list ref] wrote:
MyungJoo Ham wrote:
quoted
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@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
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(-)
Because .reg_*_stable is included in struct clksrc_clk and
s3c_wait_for_stable is called by clk_set_parent and clk_set_rate,
which have struct clksrc_clk, stat is unlikely to be NULL and if it's
NULL, s3c_wait_for_stable is called from a wrong function and wrong
context; thus, I'd use WARN for it rather than silently doing return.
If don't need to check status, have not become define relevant status
register.
quoted
+ ? ? if (stat->reg) {
No need?...or need to check stat->reg and stat->shift?
If stat->reg is NULL (it can be intentionally NULL), it means that the
corresponding struct clksrc_clk does not have stable-stat bit; thus,
pass this checking routine. And, stat->shift may be both 0 and
positive effectively.
stat->reg = 0 : NO STAT REGISTER
stat->reg != 0 : STAT Register Exists. Use stat->shift to specify the
bit (stat->shift can be 0 or larger)
I'm still wondering we _really_ need 'stable'...just comments is enough,
right now.
S5PV210 is fine without stable because stat->stable is always 0 for S5PV210.
However, this is for "Samsung-SoC" not for "S5PV210" although only
S5PV210 uses this feature for now.
I've added stat->stable for the generality. Would it be better discard
such generality and assume that other Samsung-SoC systems will also
use the same stat bit scheme for every register (0 for stable, 1 for
changing for every STAT register) with S5PV210? If we can guarantee
such homogeneousity, getting rid of stable property would be perfectly
fine. It is added just because I couldn't sure about Samsung-SoC chips
other than S5PV210. (and I thought some chips may use different scheme
for each register; e.g., some registers use 0 for stable, some others
use 1 for stable)
?};
?/**
+ * 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.
quoted
+};
+
+/**
? * 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
From: Kukjin Kim <hidden> Date: 2010-08-03 00:19:56
MyungJoo Ham wrote:
On Mon, Aug 2, 2010 at 2:30 PM, Kukjin Kim [off-list ref] wrote:
quoted
MyungJoo Ham wrote:
quoted
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@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
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(-)
Because .reg_*_stable is included in struct clksrc_clk and
s3c_wait_for_stable is called by clk_set_parent and clk_set_rate,
which have struct clksrc_clk, stat is unlikely to be NULL and if it's
NULL, s3c_wait_for_stable is called from a wrong function and wrong
context; thus, I'd use WARN for it rather than silently doing return.
I don't think that need warning to it, because it is not wrong being NULL.
quoted
If don't need to check status, have not become define relevant status
register.
quoted
+ if (stat->reg) {
No need?...or need to check stat->reg and stat->shift?
If stat->reg is NULL (it can be intentionally NULL), it means that the
corresponding struct clksrc_clk does not have stable-stat bit; thus,
pass this checking routine. And, stat->shift may be both 0 and
positive effectively.
stat->reg = 0 : NO STAT REGISTER
stat->reg != 0 : STAT Register Exists. Use stat->shift to specify the
bit (stat->shift can be 0 or larger)
If stat != NULL, I think that it does not need...because should being reg.
quoted
quoted
+ 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.
S5PV210 is fine without stable because stat->stable is always 0 for S5PV210.
Generally, the meaning of stable value may not change....frankly I'm not sure...
but if change, then we can add that later...so right now, unnecessary stuff like that does not need.
Then...and no need to add 'struct clkstat_reg'
However, this is for "Samsung-SoC" not for "S5PV210" although only
S5PV210 uses this feature for now.
I've added stat->stable for the generality. Would it be better discard
such generality and assume that other Samsung-SoC systems will also
use the same stat bit scheme for every register (0 for stable, 1 for
changing for every STAT register) with S5PV210? If we can guarantee
such homogeneousity, getting rid of stable property would be perfectly
fine. It is added just because I couldn't sure about Samsung-SoC chips
other than S5PV210. (and I thought some chips may use different scheme
for each register; e.g., some registers use 0 for stable, some others
use 1 for stable)
quoted
quoted
+ do { } while (((__raw_readl(stat->reg) >> stat->shift) & 1)
+ != stat->stable);
quoted
+ }+}+ 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
If you want to use this, just 'value' is better.
But...I think...we don't need this field now.
quoted
+};++/** * 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
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2010-08-03 09:29:34
On Tue, Aug 3, 2010 at 9:19 AM, Kukjin Kim [off-list ref] wrote:
MyungJoo Ham wrote:
quoted
On Mon, Aug 2, 2010 at 2:30 PM, Kukjin Kim [off-list ref] wrote:
quoted
MyungJoo Ham wrote:
quoted
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@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
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(-)
Because .reg_*_stable is included in struct clksrc_clk and
s3c_wait_for_stable is called by clk_set_parent and clk_set_rate,
which have struct clksrc_clk, stat is unlikely to be NULL and if it's
NULL, s3c_wait_for_stable is called from a wrong function and wrong
context; thus, I'd use WARN for it rather than silently doing return.
I don't think that need warning to it, because it is not wrong being NULL.
In this context (caller: s3c_setrate_clksrc, s3c_setparent_clksrc /
callee: s3c_wait_for_stable), it cannot be NULL as long as the callers
are these two. If it's NULL, it means that the function,
s3c_wait_for_stable, is called by someone who's not supposed to call.
In these two callers, &sclk->reg_xxx_stable cannot be NULL as long as
clk is not NULL and if clk is NULL, the thread cannot successfully
reach there.
quoted
quoted
If don't need to check status, have not become define relevant status
register.
quoted
+ ? ? if (stat->reg) {
No need?...or need to check stat->reg and stat->shift?
If stat->reg is NULL (it can be intentionally NULL), it means that the
corresponding struct clksrc_clk does not have stable-stat bit; thus,
pass this checking routine. And, stat->shift may be both 0 and
positive effectively.
stat->reg = 0 : NO STAT REGISTER
stat->reg != 0 : STAT Register Exists. Use stat->shift to specify the
bit (stat->shift can be 0 or larger)
If stat != NULL, I think that it does not need...because should being reg.
When stat != NULL, stat->reg may still be NULL intentionally as
mentioned in this paragraph because stat is not going to be NULL if
the owning clksrc_clk is present. Note that stat is a non-pointer
member of clksrc_clk.
I'm still wondering we _really_ need 'stable'...just comments is enough,
right now.
S5PV210 is fine without stable because stat->stable is always 0 for S5PV210.
Generally, the meaning of stable value may not change....frankly I'm not sure...
but if change, then we can add that later...so right now, unnecessary stuff like that does not need.
Then...and no need to add 'struct clkstat_reg'
Yes, if the definition of stable value does not change, "stable" is
not needed and only shift and reg values are required.
quoted
However, this is for "Samsung-SoC" not for "S5PV210" although only
S5PV210 uses this feature for now.
I've added stat->stable for the generality. Would it be better discard
such generality and assume that other Samsung-SoC systems will also
use the same stat bit scheme for every register (0 for stable, 1 for
changing for every STAT register) with S5PV210? If we can guarantee
such homogeneousity, getting rid of stable property would be perfectly
fine. It is added just because I couldn't sure about Samsung-SoC chips
other than S5PV210. (and I thought some chips may use different scheme
for each register; e.g., some registers use 0 for stable, some others
use 1 for stable)
?};
?/**
+ * 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.
quoted
+};
+
+/**
? * 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