Hi Jon,
On 7/15/2011 4:34 PM, Hunter, Jon wrote:
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.
Since both dividers are linked, I exposed only one to SW on purpose to
avoid conflict and confusion.
As you said, we should and can only take care of the intermediate clock
node.
The only drawback of not linking both nodes is that the clock rate of
the ocp_abe_iclk will be wrong if the parent clksel is changed.
So if the recalc is working well your patch should fix that.
My only concern is to find a way to generate that kind of hacky clock
node:-(
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.
This is again a consequence of the fake modulemode clock we introduced
initially and I tried to fix in the recent hwmod series.
Since the slimbus1 module does not have any main_clk, but instead a
bunch of optional clocks, I cannot affect any of them as the parent of
the modulemode.
That's why the iclk clock was used as the parent. That kind of issue
will not be there anymore after the module mode series.
Since the modulemode is not really a clock, the _ick / _fck extension
was not necessarily accurate previously.
Regards,
Benoit
--
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
