Hi Paul,
On 7/15/2011 3:21, Paul Walmsley wrote:
cc'ing Benoît
Hi Jon
On Thu, 14 Jul 2011, Jon Hunter wrote:
From: Jon Hunter<jon-hunter@xxxxxx>
The parent clock of the OCP_ABE_ICLK is the AESS_FCLK and the
parent clock of the AESS_FCLK is the ABE_FCLK...
ABE_FCLK --> AESS_FCLK --> OCP_ABE_ICLK
The AESS_FCLK and OCP_ABE_ICLK clocks both have dividers which
determine their operational frequency. However, the dividers for
the AESS_FCLK and OCP_ABE_ICLK are controlled via a single bit,
which is the CM1_ABE_AESS_CLKCTRL[24] bit.> When this bit is set to
0, the AESS_FCLK divider is 1 and the OCP_ABE_ICLK divider is 2.
Similarly, when this bit is set to 1, the AESS_FCLK divider is 2
and the OCP_ABE_ICLK is 1.
Sigh. This type of hardware design makes software design difficult :-(
Hopefully, because this is a interface clock the impact is really
minimal...more below...
The above relationship between the AESS_FCLK and OCP_ABE_ICLK
dividers ensure that the OCP_ABE_ICLK clock is always half the
frequency of the ABE_CLK...
OCP_ABE_ICLK = ABE_FCLK/2
The divider for the OCP_ABE_ICLK is currently missing so add a
divider that will ensure the OCP_ABE_ICLK frequency is always half
the ABE_FCLK frequency.
Signed-off-by: Jon Hunter<jon-hunter@xxxxxx>
---
arch/arm/mach-omap2/clock44xx_data.c | 16 +++++++++++++++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 8c96567..6e158ce 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -1301,11 +1301,25 @@ static struct clk mcasp3_fclk = {
.recalc =&followparent_recalc,
};
+static const struct clksel_rate div2_2to1_rates[] = {
+ { .div = 1, .val = 1, .flags = RATE_IN_4430 },
+ { .div = 2, .val = 0, .flags = RATE_IN_4430 },
+ { .div = 0 },
+};
+
+static const struct clksel ocp_abe_iclk_div[] = {
+ { .parent =&aess_fclk, .rates = div2_2to1_rates },
+ { .parent = NULL },
+};
+
static struct clk ocp_abe_iclk = {
.name = "ocp_abe_iclk",
.parent =&aess_fclk,
+ .clksel = ocp_abe_iclk_div,
+ .clksel_reg = OMAP4430_CM1_ABE_AESS_CLKCTRL,
+ .clksel_mask = OMAP4430_CLKSEL_AESS_FCLK_MASK,
.ops =&clkops_null,
- .recalc =&followparent_recalc,
+ .recalc =&omap2_clksel_recalc,
};
static struct clk per_abe_24m_fclk = {
I guess the reason that this patch can get away with this is because it
doesn't allow software to change the rate of ocp_abe_iclk, and the
ocp_abe_iclk is a child of aess_fclk, so when aess_fclk is changed, it
will recalc ocp_abe_iclk.
Benoît, what do you think? Is this a reasonable approach for the script?
Or do we need to deal with some kind of 'linked clock' implementation...
If you want my two cents on this matter, I would say that...
1). People should not need to configure the "ocp_abe_iclk" clock
directly, because regardless of the divider setting it is always 1/2 the
"abe_fclk". In other words, only the "aess_fclk" frequency is really
changing because of the divider and the above relationship ensures that
the "ocp_abe_iclk" is always the same frequency. So a user only cares
about the "aess_fclk" frequency and the "ocp_abe_iclk" frequency is
handled for them.
2). The "ocp_abe_iclk" is an interface clock and is not a parent to any
other functional clock and therefore, is not driving any internal logic
in a peripheral which would have a direct impact of the speed of that
peripheral. However, there does appear to be another bug here in the
clock44xx_data.c as it shows that the "ocp_abe_iclk" is parent to the
"slimbus1_fck" which I believe is incorrect. According to the TRM, the
the ocp_abe_iclk is parent to the slimbus1_iclk. I can send another
patch for this. However, I will let Benoit chime in first.
Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
