mbox

[0/8] OMAP3/4: Misc fixes and clean-up

Message ID 1315459327-3285-1-git-send-email-santosh.shilimkar@ti.com
State New
Headers show

Pull-request

git://gitorious.org/omap-sw-develoment/linux-omap-dev.git v3.1-rc4-omap-misc

Message

Santosh Shilimkar Sept. 8, 2011, 5:21 a.m. UTC
The series contains few fixes and clean-up for OMAP.
Briefly,
- HWMOD fix for the address space count
- Improving the L3 register accesses
- Bug fix in the L3 error handler
- Sparce warning and indentation fixes in L3 error handler
- Print master id in case of L3 custom errors for better debug.
- Adding local time clock node for the CPUfreq and time re-calibration
- Fix in the address overlap for emif and emulation domain.

The series is tested on OMAP4430 SDP and OMAP4430 beagle and SDP.

The following changes since commit c6a389f123b9f68d605bb7e0f9b32ec1e3e14132:

  Linux 3.1-rc4 (2011-08-28 21:16:01 -0700)

are available in the git repository at:
  git://gitorious.org/omap-sw-develoment/linux-omap-dev.git v3.1-rc4-omap-misc

Santosh Shilimkar (2):
      OMAP4: clock: Add CPU local timer clock node.
      OMAP4: Fix the emif and dmm virtual mapping

Todd Poynor (2):
      OMAP: Improve register access in L3 Error handler.
      OMAP: Fix a BUG in l3 error handler.

sricharan (4):
      OMAP: hwmod: Fix the addr spaces count API.
      OMAP: Fix indentation issues in l3 error handler.
      OMAP: Fix sparse warnings in l3 error handler.
      OMAP: Print Initiator name for l3 custom error.

 arch/arm/mach-omap2/clock44xx_data.c |    9 ++
 arch/arm/mach-omap2/omap_hwmod.c     |    8 +-
 arch/arm/mach-omap2/omap_l3_noc.c    |  130 ++++++++++----------
 arch/arm/mach-omap2/omap_l3_noc.h    |  224 +++++++++++++++++++---------------
 arch/arm/mach-omap2/omap_l3_smx.c    |   91 +++++++-------
 arch/arm/mach-omap2/omap_l3_smx.h    |  164 ++++++++++++------------
 arch/arm/plat-omap/include/plat/io.h |    4 +-
 7 files changed, 336 insertions(+), 294 deletions(-)

Comments

Cousson, Benoit Sept. 8, 2011, 7:47 a.m. UTC | #1
Hi Sricharan,

On 9/8/2011 7:22 AM, Shilimkar, Santosh wrote:
> From: sricharan<r.sricharan@ti.com>
>
> The address space count API returns the number of address space
> entries for a hwmod including a additional null value present in the
> address space structure introduced recently.

That's a minor nit, but you might give the commit you are referencing here.

> The devices which
> have multiple hwmods and use device_build_ss are broken with this,
> as their address resources are populated with a extra null value,
> subsequently the probe fails. So fix the API not to add the null value.

It seems that in every cases, we are adding an extra null resource for 
nothing. But it is true that will not crash if the driver is just 
expecting an unique entry.
What is happening with multiple hwmods is probably the introduction of 
that extra null resource in the middle of the real ones, hence shifting 
the resource index?
You might give more details here.

> Signed-off-by: sricharan<r.sricharan@ti.com>
> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
> Cc: Benoit Cousson<b-cousson@ti.com>
> Cc: Paul Walmsley<paul@pwsan.com>
> Cc: Kevin Hilman<khilman@ti.com>
> ---
>   arch/arm/mach-omap2/omap_hwmod.c |    8 +++++---
>   1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 84cc0bd..32a0f48a 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -791,9 +791,11 @@ static int _count_ocp_if_addr_spaces(struct omap_hwmod_ocp_if *os)
>   	if (!os || !os->addr)
>   		return 0;
>
> -	do {
> -		mem =&os->addr[i++];
> -	} while (mem->pa_start != mem->pa_end);
> +	mem =&os->addr[i];
> +
> +	while (mem->pa_start != mem->pa_end) {
> +		mem =&os->addr[++i];
> +	};
>
>   	return i;

Cannot you just do "return i - 1"?

Regards,
Benoit
Kevin Hilman Sept. 16, 2011, 5:56 p.m. UTC | #2
Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

> Fix the address overlap with Emulation domain (EMU).
>
> The previous mapping was entering into EMU mapping
> and was not as per comments. Fix the mapping accordingly.
>
> [girishsg@ti.com: Helped fixing comments.]
> Signed-off-by: Girish S G <girishsg@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/plat-omap/include/plat/io.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/io.h b/arch/arm/plat-omap/include/plat/io.h
> index d72ec85..a2f7d31 100644
> --- a/arch/arm/plat-omap/include/plat/io.h
> +++ b/arch/arm/plat-omap/include/plat/io.h
> @@ -228,12 +228,12 @@
>  
>  #define OMAP44XX_EMIF2_PHYS	OMAP44XX_EMIF2_BASE
>  						/* 0x4d000000 --> 0xfd200000 */
> -#define OMAP44XX_EMIF2_VIRT	(OMAP44XX_EMIF2_PHYS + OMAP4_L3_PER_IO_OFFSET)
> +#define OMAP44XX_EMIF2_VIRT	(OMAP44XX_EMIF1_VIRT + SZ_1M)

IMO, this would be much clearer (and future proof) if you used
'+ OMAP44XX_EMIF1_SIZE' instead of SZ_1M.

>  #define OMAP44XX_EMIF2_SIZE	SZ_1M
>  
>  #define OMAP44XX_DMM_PHYS	OMAP44XX_DMM_BASE
>  						/* 0x4e000000 --> 0xfd300000 */
> -#define OMAP44XX_DMM_VIRT	(OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
> +#define OMAP44XX_DMM_VIRT	(OMAP44XX_EMIF2_VIRT + SZ_1M)

and '+ OMAP44XX_EMIF2_SIZE' here.

>  #define OMAP44XX_DMM_SIZE	SZ_1M
>  /*
>   * ----------------------------------------------------------------------------

Kevin
Santosh Shilimkar Sept. 20, 2011, 3:01 p.m. UTC | #3
On Friday 16 September 2011 11:26 PM, Kevin Hilman wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
> 
>> Fix the address overlap with Emulation domain (EMU).
>>
>> The previous mapping was entering into EMU mapping
>> and was not as per comments. Fix the mapping accordingly.
>>
>> [girishsg@ti.com: Helped fixing comments.]
>> Signed-off-by: Girish S G <girishsg@ti.com>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> ---
>>  arch/arm/plat-omap/include/plat/io.h |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/plat/io.h b/arch/arm/plat-omap/include/plat/io.h
>> index d72ec85..a2f7d31 100644
>> --- a/arch/arm/plat-omap/include/plat/io.h
>> +++ b/arch/arm/plat-omap/include/plat/io.h
>> @@ -228,12 +228,12 @@
>>  
>>  #define OMAP44XX_EMIF2_PHYS	OMAP44XX_EMIF2_BASE
>>  						/* 0x4d000000 --> 0xfd200000 */
>> -#define OMAP44XX_EMIF2_VIRT	(OMAP44XX_EMIF2_PHYS + OMAP4_L3_PER_IO_OFFSET)
>> +#define OMAP44XX_EMIF2_VIRT	(OMAP44XX_EMIF1_VIRT + SZ_1M)
> 
> IMO, this would be much clearer (and future proof) if you used
> '+ OMAP44XX_EMIF1_SIZE' instead of SZ_1M.
> 
ok.

>>  #define OMAP44XX_EMIF2_SIZE	SZ_1M
>>  
>>  #define OMAP44XX_DMM_PHYS	OMAP44XX_DMM_BASE
>>  						/* 0x4e000000 --> 0xfd300000 */
>> -#define OMAP44XX_DMM_VIRT	(OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
>> +#define OMAP44XX_DMM_VIRT	(OMAP44XX_EMIF2_VIRT + SZ_1M)
> 
> and '+ OMAP44XX_EMIF2_SIZE' here.
> 
Will add OMAP44XX_EMIF_SIZE since 2 EMIFs instaces are and
suppose to be identical.Almost missed this email in other traffic.

Regards
Santosh
Kevin Hilman Sept. 21, 2011, 5:31 p.m. UTC | #4
Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

> On Tuesday 20 September 2011 08:31 PM, Santosh Shilimkar wrote:
>> On Friday 16 September 2011 11:26 PM, Kevin Hilman wrote:
>>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>>>
>
> [...]
>
>> 
>>>>  #define OMAP44XX_EMIF2_SIZE	SZ_1M
>>>>  
>>>>  #define OMAP44XX_DMM_PHYS	OMAP44XX_DMM_BASE
>>>>  						/* 0x4e000000 --> 0xfd300000 */
>>>> -#define OMAP44XX_DMM_VIRT	(OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
>>>> +#define OMAP44XX_DMM_VIRT	(OMAP44XX_EMIF2_VIRT + SZ_1M)
>>>
>>> and '+ OMAP44XX_EMIF2_SIZE' here.
>>>
>> Will add OMAP44XX_EMIF_SIZE since 2 EMIFs instaces are and
>> suppose to be identical.Almost missed this email in other traffic.
>
> OPPs. It was already there. Dumb of me not using it. Below
> is the update what I will do.
>
> diff --git a/arch/arm/plat-omap/include/plat/io.h
> b/arch/arm/plat-omap/include/plat/io.h
> index d72ec85..db36292 100644
> --- a/arch/arm/plat-omap/include/plat/io.h
> +++ b/arch/arm/plat-omap/include/plat/io.h
> @@ -228,13 +228,13 @@
>
>  #define OMAP44XX_EMIF2_PHYS	OMAP44XX_EMIF2_BASE
>  						/* 0x4d000000 --> 0xfd200000 */
> -#define OMAP44XX_EMIF2_VIRT	(OMAP44XX_EMIF2_PHYS + OMAP4_L3_PER_IO_OFFSET)
>  #define OMAP44XX_EMIF2_SIZE	SZ_1M
> +#define OMAP44XX_EMIF2_VIRT	(OMAP44XX_EMIF1_VIRT + OMAP44XX_EMIF2_SIZE)

This doesn't look right either.

This #define is for the *start* of EMIF2 virtual address space, so 
assuming the start address of EMIF2 is immediatly after EMIF1, it should
be EMIF1_VIRT + EMIF1_SIZE (not EMIF2_SIZE.)

>  #define OMAP44XX_DMM_PHYS	OMAP44XX_DMM_BASE
>  						/* 0x4e000000 --> 0xfd300000 */
> -#define OMAP44XX_DMM_VIRT	(OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
>  #define OMAP44XX_DMM_SIZE	SZ_1M
> +#define OMAP44XX_DMM_VIRT	(OMAP44XX_EMIF2_VIRT + OMAP44XX_DMM_SIZE)

And here, assuming DMM range is immediately after EMIF2, this should be
EMIF2_VIRT + EMFI2_SIZE)

Kevin
Santosh Shilimkar Sept. 22, 2011, 5:53 a.m. UTC | #5
On Wed, Sep 21, 2011 at 11:01 PM, Kevin Hilman <khilman@ti.com> wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>
>> On Tuesday 20 September 2011 08:31 PM, Santosh Shilimkar wrote:
>>> On Friday 16 September 2011 11:26 PM, Kevin Hilman wrote:
>>>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>>>>
>>
>> [...]
>>
>>>
>>>>>  #define OMAP44XX_EMIF2_SIZE       SZ_1M
>>>>>
>>>>>  #define OMAP44XX_DMM_PHYS OMAP44XX_DMM_BASE
>>>>>                                            /* 0x4e000000 --> 0xfd300000 */
>>>>> -#define OMAP44XX_DMM_VIRT (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
>>>>> +#define OMAP44XX_DMM_VIRT (OMAP44XX_EMIF2_VIRT + SZ_1M)
>>>>
>>>> and '+ OMAP44XX_EMIF2_SIZE' here.
>>>>
>>> Will add OMAP44XX_EMIF_SIZE since 2 EMIFs instaces are and
>>> suppose to be identical.Almost missed this email in other traffic.
>>
>> OPPs. It was already there. Dumb of me not using it. Below
>> is the update what I will do.
>>
>> diff --git a/arch/arm/plat-omap/include/plat/io.h
>> b/arch/arm/plat-omap/include/plat/io.h
>> index d72ec85..db36292 100644
>> --- a/arch/arm/plat-omap/include/plat/io.h
>> +++ b/arch/arm/plat-omap/include/plat/io.h
>> @@ -228,13 +228,13 @@
>>
>>  #define OMAP44XX_EMIF2_PHYS  OMAP44XX_EMIF2_BASE
>>                                               /* 0x4d000000 --> 0xfd200000 */
>> -#define OMAP44XX_EMIF2_VIRT  (OMAP44XX_EMIF2_PHYS + OMAP4_L3_PER_IO_OFFSET)
>>  #define OMAP44XX_EMIF2_SIZE  SZ_1M
>> +#define OMAP44XX_EMIF2_VIRT  (OMAP44XX_EMIF1_VIRT + OMAP44XX_EMIF2_SIZE)
>
> This doesn't look right either.
>
> This #define is for the *start* of EMIF2 virtual address space, so
> assuming the start address of EMIF2 is immediatly after EMIF1, it should
> be EMIF1_VIRT + EMIF1_SIZE (not EMIF2_SIZE.)
>
>>  #define OMAP44XX_DMM_PHYS    OMAP44XX_DMM_BASE
>>                                               /* 0x4e000000 --> 0xfd300000 */
>> -#define OMAP44XX_DMM_VIRT    (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
>>  #define OMAP44XX_DMM_SIZE    SZ_1M
>> +#define OMAP44XX_DMM_VIRT    (OMAP44XX_EMIF2_VIRT + OMAP44XX_DMM_SIZE)
>
> And here, assuming DMM range is immediately after EMIF2, this should be
> EMIF2_VIRT + EMFI2_SIZE)
>
I agree.

Regards
Santosh
Santosh Shilimkar Sept. 24, 2011, 6:03 a.m. UTC | #6
Paul,

On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote:
> The series contains few fixes and clean-up for OMAP.
> Briefly,
> - HWMOD fix for the address space count
> - Improving the L3 register accesses
> - Bug fix in the L3 error handler
> - Sparce warning and indentation fixes in L3 error handler
> - Print master id in case of L3 custom errors for better debug.
> - Adding local time clock node for the CPUfreq and time re-calibration
> - Fix in the address overlap for emif and emulation domain.
> 
> The series is tested on OMAP4430 SDP and OMAP4430 beagle and SDP.
> 
> The following changes since commit c6a389f123b9f68d605bb7e0f9b32ec1e3e14132:
> 
>   Linux 3.1-rc4 (2011-08-28 21:16:01 -0700)
> 
> are available in the git repository at:
>   git://gitorious.org/omap-sw-develoment/linux-omap-dev.git v3.1-rc4-omap-misc
>
I need your ack on below two patches so that I can re-base them and
send a pull request to Tony.

OMAP4: clock: Add CPU local timer clock node.
OMAP: Fix sparse warnings in l3 error handler.

Regards
Santosh
Paul Walmsley Sept. 24, 2011, 6:31 a.m. UTC | #7
Hi Santosh

On Sat, 24 Sep 2011, Santosh Shilimkar wrote:

> On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote:
> > The series contains few fixes and clean-up for OMAP.
> > Briefly,
> > - HWMOD fix for the address space count
> > - Improving the L3 register accesses
> > - Bug fix in the L3 error handler
> > - Sparce warning and indentation fixes in L3 error handler
> > - Print master id in case of L3 custom errors for better debug.
> > - Adding local time clock node for the CPUfreq and time re-calibration
> > - Fix in the address overlap for emif and emulation domain.
> > 
> > The series is tested on OMAP4430 SDP and OMAP4430 beagle and SDP.
> > 
> > The following changes since commit c6a389f123b9f68d605bb7e0f9b32ec1e3e14132:
> > 
> >   Linux 3.1-rc4 (2011-08-28 21:16:01 -0700)
> > 
> > are available in the git repository at:
> >   git://gitorious.org/omap-sw-develoment/linux-omap-dev.git v3.1-rc4-omap-misc
> >
> I need your ack on below two patches so that I can re-base them and
> send a pull request to Tony.
> 
> OMAP4: clock: Add CPU local timer clock node.
> OMAP: Fix sparse warnings in l3 error handler.

Are you targeting these for 3.1-rc ?


- Paul
Santosh Shilimkar Sept. 24, 2011, 6:35 a.m. UTC | #8
On Saturday 24 September 2011 12:01 PM, Paul Walmsley wrote:
> Hi Santosh
> 
> On Sat, 24 Sep 2011, Santosh Shilimkar wrote:
> 
>> On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote:
>>> The series contains few fixes and clean-up for OMAP.
>>> Briefly,
>>> - HWMOD fix for the address space count
>>> - Improving the L3 register accesses
>>> - Bug fix in the L3 error handler
>>> - Sparce warning and indentation fixes in L3 error handler
>>> - Print master id in case of L3 custom errors for better debug.
>>> - Adding local time clock node for the CPUfreq and time re-calibration
>>> - Fix in the address overlap for emif and emulation domain.
>>>
>>> The series is tested on OMAP4430 SDP and OMAP4430 beagle and SDP.
>>>
>>> The following changes since commit c6a389f123b9f68d605bb7e0f9b32ec1e3e14132:
>>>
>>>   Linux 3.1-rc4 (2011-08-28 21:16:01 -0700)
>>>
>>> are available in the git repository at:
>>>   git://gitorious.org/omap-sw-develoment/linux-omap-dev.git v3.1-rc4-omap-misc
>>>
>> I need your ack on below two patches so that I can re-base them and
>> send a pull request to Tony.
>>
>> OMAP4: clock: Add CPU local timer clock node.
>> OMAP: Fix sparse warnings in l3 error handler.
> 
> Are you targeting these for 3.1-rc ?
> 
Nope. It's for 3.2 merge window.
As such, the series doesn't have any major regression
fixes which should make it before 3.2

Regards
Santosh
Paul Walmsley Sept. 24, 2011, 7:36 a.m. UTC | #9
Hi

On Sat, 24 Sep 2011, Santosh Shilimkar wrote:

> On Saturday 24 September 2011 12:01 PM, Paul Walmsley wrote:
> > Hi Santosh
> > 
> > On Sat, 24 Sep 2011, Santosh Shilimkar wrote:
> > 
> >> On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote:
> >>>
> >> I need your ack on below two patches so that I can re-base them and
> >> send a pull request to Tony.
> >>
> >> OMAP4: clock: Add CPU local timer clock node.
> >> OMAP: Fix sparse warnings in l3 error handler.
> > 
> > Are you targeting these for 3.1-rc ?
> > 
> Nope. It's for 3.2 merge window.
> As such, the series doesn't have any major regression
> fixes which should make it before 3.2

In that case, patches 1 and 7 should go in through me.  The rest should go 
via Tony.  So you can just drop 1 and 7 from your branch, I'll pull those 
off the list. 

Some comments though:  Looks like you need to repost patch 1 due to 
comments from Benoît?  Also, have the autogeneration scripts been updated 
for patch 7?

...

As far as patch 5 goes, you don't need my ack on that one, since I'm not 
the maintainer or original author of that code.  But feel free to add a 
Reviewed-by:.  I also tested the two L3 files with sparse and they are 
clean now.  Thanks very much for taking the initiative on this; I 
personally appreciate it quite a bit, and I think having clean code 
increases our credibility with upstream maintainers. 


- Paul
Santosh Shilimkar Sept. 24, 2011, 7:46 a.m. UTC | #10
On Saturday 24 September 2011 01:06 PM, Paul Walmsley wrote:
> Hi
> 
> On Sat, 24 Sep 2011, Santosh Shilimkar wrote:
> 
>> On Saturday 24 September 2011 12:01 PM, Paul Walmsley wrote:
>>> Hi Santosh
>>>
>>> On Sat, 24 Sep 2011, Santosh Shilimkar wrote:
>>>
>>>> On Thursday 08 September 2011 10:51 AM, Santosh Shilimkar wrote:
>>>>>
>>>> I need your ack on below two patches so that I can re-base them and
>>>> send a pull request to Tony.
>>>>
>>>> OMAP4: clock: Add CPU local timer clock node.
>>>> OMAP: Fix sparse warnings in l3 error handler.
>>>
>>> Are you targeting these for 3.1-rc ?
>>>
>> Nope. It's for 3.2 merge window.
>> As such, the series doesn't have any major regression
>> fixes which should make it before 3.2
> 
> In that case, patches 1 and 7 should go in through me.  The rest should go 
> via Tony.  So you can just drop 1 and 7 from your branch, I'll pull those 
> off the list. 
> 
Will do.

> Some comments though:  Looks like you need to repost patch 1 due to 
> comments from Benoît?  Also, have the autogeneration scripts been updated 
> for patch 7?
> 
Ok. Will post updated patch1.
For the patch 7, I will align with Benoit on the autogen script. It has
some differences with mainline OMAP4 clock data file.

> ...
> 
> As far as patch 5 goes, you don't need my ack on that one, since I'm not 
> the maintainer or original author of that code.  But feel free to add a 
> Reviewed-by:.  I also tested the two L3 files with sparse and they are 
> clean now.  Thanks very much for taking the initiative on this; I 
> personally appreciate it quite a bit, and I think having clean code 
> increases our credibility with upstream maintainers. 
> 
Actually I wanted you to have a look to ensure that we addressed the
all sparce warnings. Will add your reviewed by on that patch.

Couldn't agree more on the clean code remark.

Regards
Santosh
Linus Walleij Sept. 29, 2011, 9:30 p.m. UTC | #11
2011/9/8 Santosh Shilimkar <santosh.shilimkar@ti.com>:

> Local timer clock is sourced from the CPU clock and hence changes
> along with CPU clock. These per CPU local timers are used as
> clock-events, so they need to be reconfigured on CPU frequency
> change as part of CPUfreq governor.

This requires patch 6956/2 right?
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6956/2

Can I add your Tested-by: to the patch as well?

Yours,
Linus Walleij
Santosh Shilimkar Sept. 30, 2011, 9:14 a.m. UTC | #12
On Fri, Sep 30, 2011 at 12:42 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Hi Santosh,
>
> On Thu, 8 Sep 2011, Santosh Shilimkar wrote:
>
>> Local timer clock is sourced from the CPU clock and hence changes
>> along with CPU clock. These per CPU local timers are used as
>> clock-events, so they need to be reconfigured on CPU frequency
>> change as part of CPUfreq governor.
>>
>> Newly introduced clockevents_reconfigure() needs to know the
>> twd clock-rate. Provide a clock-node to make clk_get_rate() work
>> for TWD.
>>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Kevin Hilman <khilman@ti.com>
>
> This clock looks like the ARM Cortex-A9 MPCore PERIPHCLK, correct?  From
> ARM Cortex-A9 MPCore Technical Reference Manual (DDI 0407G) Section 5.1
> "Clocks"?
>
Yes. Peripheral clock/2

> Assuming that is so, I've modified this patch appropriately, and added
> support to the autogeneration script to generate this clock node.
> Care to take a quick look at it to make sure it's okay?
>
Thanks a lot Paul. It looks good.

Regards
Santosh
Santosh Shilimkar Sept. 30, 2011, 9:15 a.m. UTC | #13
On Fri, Sep 30, 2011 at 3:00 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> 2011/9/8 Santosh Shilimkar <santosh.shilimkar@ti.com>:
>
>> Local timer clock is sourced from the CPU clock and hence changes
>> along with CPU clock. These per CPU local timers are used as
>> clock-events, so they need to be reconfigured on CPU frequency
>> change as part of CPUfreq governor.
>
> This requires patch 6956/2 right?
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6956/2
>
> Can I add your Tested-by: to the patch as well?
>
Sure.
Santosh Shilimkar Dec. 12, 2011, 8:15 a.m. UTC | #14
On Fri, Dec 9, 2011 at 4:32 AM, Turquette, Mike <mturquette@ti.com> wrote:
> On Fri, Sep 30, 2011 at 2:15 AM, Shilimkar, Santosh
> <santosh.shilimkar@ti.com> wrote:
>> On Fri, Sep 30, 2011 at 3:00 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> 2011/9/8 Santosh Shilimkar <santosh.shilimkar@ti.com>:
>>>
>>>> Local timer clock is sourced from the CPU clock and hence changes
>>>> along with CPU clock. These per CPU local timers are used as
>>>> clock-events, so they need to be reconfigured on CPU frequency
>>>> change as part of CPUfreq governor.
>>>
>>> This requires patch 6956/2 right?
>>> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6956/2
>>>
>>> Can I add your Tested-by: to the patch as well?
>>>
>> Sure.
>
> Santosh,
>
> I've taken in Linus' patch as well as this patch while testing clk
> notifiers (part of common clk framework code).
>
> Linus' patch matches the clk based on dev_id, but the clkdev entry in
> your patch populated dev_id with NULL and con_id with "smp_twd" so
> things never worked correctly.  Below changed fixed it for me:
>
You are right.
When I picked up the patch while changing the
OMAP44XX stuff to OMAP4430, looks like I miss-placed it.

> diff --git a/arch/arm/mach-omap2/clock44xx_data.c
> b/arch/arm/mach-omap2/clock44xx_data.c
> index ae8c292..0f8cf68 100644
> --- a/arch/arm/mach-omap2/clock44xx_data.c
> +++ b/arch/arm/mach-omap2/clock44xx_data.c
> @@ -4114,7 +4114,7 @@ static struct omap_clk omap44xx_clks[] = {
>        CLK(NULL,       "l4_div_ck",
> &l4_div_ck_hw.clk,      CK_443X),
>        CLK(NULL,       "lp_clk_div_ck",
> &lp_clk_div_ck_hw.clk,  CK_443X),
>        CLK(NULL,       "l4_wkup_clk_mux_ck",
> &l4_wkup_clk_mux_ck_hw.clk,     CK_443X),
> -       CLK(NULL,       "smp_twd",
> &mpu_periphclk_hw.clk,  CK_443X),
> +       CLK("smp_twd",  NULL,                   &mpu_periphclk_hw.clk,
>  CK_443X),
>        CLK(NULL,       "ocp_abe_iclk",
> &ocp_abe_iclk_hw.clk,   CK_443X),
>        CLK(NULL,       "per_abe_24m_fclk",
> &per_abe_24m_fclk_hw.clk,       CK_443X),
>        CLK(NULL,       "per_abe_nc_fclk",
> &per_abe_nc_fclk_hw.clk,        CK_443X),
>
> A better change might be for the code in smp_twd.c to match on con_id
> but I'll let you guys sort that out.

I guess we can stick to what Linus and others have followed.
Will post an updated patch. Thanks for catch.

Regards
Santosh