[PATCH 1/6] OMAP4: Add missing clock divider for OCP_ABE_ICLK

[PATCH 1/6] OMAP4: Add missing clock divider for OCP_ABE_ICLK

[Jon Hunter]

From: Jon Hunter <jon-hunter@ti.com>

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.

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@ti.com>
---
 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 = {
-- 
1.7.4.1

Re: [PATCH 1/6] OMAP4: Add missing clock divider for OCP_ABE_ICLK

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2809 bytes --]

cc'ing Benoît

Hi Jon

On Thu, 14 Jul 2011, Jon Hunter wrote:

> From: Jon Hunter <jon-hunter@ti.com>
> 
> 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 :-(

> 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@ti.com>
> ---
>  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...


- Paul

Re: [PATCH 1/6] OMAP4: Add missing clock divider for OCP_ABE_ICLK

[Jon Hunter]

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@ti.com>
>>
>> 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@ti.com>
>> ---
>>   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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] OMAP4: Add missing clock divider for OCP_ABE_ICLK

[Jon Hunter]

On 7/15/2011 9:34, Jon Hunter wrote:
> 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.

On further inspection of the clock44xx_data.c, it appears that all 
interface clocks are called xxx_fck and not xxx_ick. I will ask Benoit 
about this. However, this particular clock we are dealing with here is 
an interface clock.

Cheers
Jon

Re: [PATCH 1/6] OMAP4: Add missing clock divider for OCP_ABE_ICLK

[Jon Hunter]

On 7/18/2011 15:57, Jon Hunter wrote:
> On further inspection of the clock44xx_data.c, it appears that all
> interface clocks are called xxx_fck and not xxx_ick. I will ask Benoit
> about this. However, this particular clock we are dealing with here is
> an interface clock.

Actually, the above it not true. I will ask Benoit about the naming of 
the slimbus interface clocks.

Jon

Re: [PATCH 1/6] OMAP4: Add missing clock divider for OCP_ABE_ICLK

[Cousson, Benoit]

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@ti.com>
>>>
>>> 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@ti.com>
>>> ---
>>>    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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] OMAP4: Add missing clock divider for OCP_ABE_ICLK

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5866 bytes --]

Hi Benoît, Jon,

On Wed, 27 Jul 2011, Cousson, Benoit wrote:
> On 7/15/2011 4:34 PM, Hunter, Jon wrote:
> > On 7/15/2011 3:21, Paul Walmsley wrote:
> > > On Thu, 14 Jul 2011, Jon Hunter wrote:
> > > 
> > > > From: Jon Hunter<jon-hunter@ti.com>
> > > > 
> > > > 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@ti.com>
> > > > ---
> > > >    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.

Just doublechecking on this patch.  Does anything need to be changed 
before we merge this?  I can take a look at the autogeneration script if 
that's the only issue left.



- Paul

Re: [PATCH 1/6] OMAP4: Add missing clock divider for OCP_ABE_ICLK

[Jon Hunter]

Hi Paul

On 8/28/2011 23:09, Paul Walmsley wrote:
> Just doublechecking on this patch.  Does anything need to be changed
> before we merge this?  I can take a look at the autogeneration script if
> that's the only issue left.

Sorry, I have been out on holiday. I believe that was the only problem 
with this.

Benoit what are your thoughts?

Cheers
Jon