Patchwork [0/5] ARM: OMAP5: Misc fixes

login
register
mail settings
Submitter Santosh Shilimkar
Date Jan. 18, 2013, 3:32 p.m.
Message ID <1358523141-12295-1-git-send-email-santosh.shilimkar@ti.com>
Download mbox
Permalink /patch/213644/
State New
Headers show

Pull-request

git://github.com/SantoshShilimkar/linux.git for_3.9/omap5_misc_fixes

Comments

Santosh Shilimkar - Jan. 18, 2013, 3:32 p.m.
Series contains few random fixes needed for OMAP5 es2.0 boot going.
There is no depedency with data files for this series and can be
applied indepedently.

Tested on OMAP5 uEVM and sEVM boards.


The following changes since commit 7d1f9aeff1ee4a20b1aeb377dd0f579fe9647619:

  Linux 3.8-rc4 (2013-01-17 19:25:45 -0800)

are available in the git repository at:

  git://github.com/SantoshShilimkar/linux.git for_3.9/omap5_misc_fixes

for you to fetch changes up to 4bedcaba7da88d06cde60aa595dfe57ddd8495cc:

  ARM: dts: OMAP5: Specify nonsecure PPI IRQ for arch timer (2013-01-18 20:47:42 +0530)

----------------------------------------------------------------

Rajendra Nayak (1):
  ARM: dts: OMAP5: Specify nonsecure PPI IRQ for arch timer

Santosh Shilimkar (4):
  ARM: OMAP5: Update SOC id detection code for ES2
  ARM: OMAP2+: timer: Update the OMAP5 clocksource name as per clock
    data
  ARM: OMAP: prm: Allow prm init to success on OMAP5 SOCs
  ARM: dts: omap5-evm: Update available memory to 2032 MB

 arch/arm/boot/dts/omap5-evm.dts |    2 +-
 arch/arm/boot/dts/omap5.dtsi    |   16 ++++++++++++----
 arch/arm/mach-omap2/id.c        |   16 +++++++++++++---
 arch/arm/mach-omap2/prm44xx.c   |    2 +-
 arch/arm/mach-omap2/soc.h       |    2 ++
 arch/arm/mach-omap2/timer.c     |    5 +++--
 6 files changed, 32 insertions(+), 11 deletions(-)
Marc Zyngier - Jan. 18, 2013, 4:02 p.m.
On 18/01/13 15:32, Santosh Shilimkar wrote:
> From: Rajendra Nayak <rnayak@ti.com>
> 
> Specify both secure as well as nonsecure PPI IRQ for arch
> timer. This fixes the following errors seen on DT OMAP5 boot..
> 
> [    0.000000] arch_timer: No interrupt available, giving up
> 
> Cc: Benoit Cousson <b-cousson@ti.com>
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/boot/dts/omap5.dtsi |   16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index 790bb2a..7a78d1b 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -35,8 +35,12 @@
>  			compatible = "arm,cortex-a15";
>  			timer {
>  				compatible = "arm,armv7-timer";
> -				/* 14th PPI IRQ, active low level-sensitive */
> -				interrupts = <1 14 0x308>;
> +				/*
> +				 * PPI secure/nonsecure IRQ,
> +				 * active low level-sensitive
> +				 */
> +				interrupts = <1 13 0x308>,
> +					     <1 14 0x308>;

Care to add the virtual and HYP timer interrupts? So KVM can get a
chance to run on this HW...

>  				clock-frequency = <6144000>;
>  			};
>  		};
> @@ -44,8 +48,12 @@
>  			compatible = "arm,cortex-a15";
>  			timer {
>  				compatible = "arm,armv7-timer";
> -				/* 14th PPI IRQ, active low level-sensitive */
> -				interrupts = <1 14 0x308>;
> +				/*
> +				 * PPI secure/nonsecure IRQ,
> +				 * active low level-sensitive
> +				 */
> +				interrupts = <1 13 0x308>,
> +					     <1 14 0x308>;

Same here.

>  				clock-frequency = <6144000>;
>  			};
>  		};
> 

Thanks,

	M.
Santosh Shilimkar - Jan. 18, 2013, 5 p.m.
On Friday 18 January 2013 09:32 PM, Marc Zyngier wrote:
> On 18/01/13 15:32, Santosh Shilimkar wrote:
>> From: Rajendra Nayak <rnayak@ti.com>
>>
>> Specify both secure as well as nonsecure PPI IRQ for arch
>> timer. This fixes the following errors seen on DT OMAP5 boot..
>>
>> [    0.000000] arch_timer: No interrupt available, giving up
>>
>> Cc: Benoit Cousson <b-cousson@ti.com>
>>
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> ---
>>   arch/arm/boot/dts/omap5.dtsi |   16 ++++++++++++----
>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
>> index 790bb2a..7a78d1b 100644
>> --- a/arch/arm/boot/dts/omap5.dtsi
>> +++ b/arch/arm/boot/dts/omap5.dtsi
>> @@ -35,8 +35,12 @@
>>   			compatible = "arm,cortex-a15";
>>   			timer {
>>   				compatible = "arm,armv7-timer";
>> -				/* 14th PPI IRQ, active low level-sensitive */
>> -				interrupts = <1 14 0x308>;
>> +				/*
>> +				 * PPI secure/nonsecure IRQ,
>> +				 * active low level-sensitive
>> +				 */
>> +				interrupts = <1 13 0x308>,
>> +					     <1 14 0x308>;
>
> Care to add the virtual and HYP timer interrupts? So KVM can get a
> chance to run on this HW...
>
Thanks Marc for spotting it. Will take care of it.

Regards
Santosh
Marc Zyngier - Jan. 18, 2013, 5:08 p.m.
On 18/01/13 17:00, Santosh Shilimkar wrote:
> On Friday 18 January 2013 09:32 PM, Marc Zyngier wrote:
>> On 18/01/13 15:32, Santosh Shilimkar wrote:
>>> From: Rajendra Nayak <rnayak@ti.com>
>>>
>>> Specify both secure as well as nonsecure PPI IRQ for arch
>>> timer. This fixes the following errors seen on DT OMAP5 boot..
>>>
>>> [    0.000000] arch_timer: No interrupt available, giving up
>>>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>>
>>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>> ---
>>>   arch/arm/boot/dts/omap5.dtsi |   16 ++++++++++++----
>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
>>> index 790bb2a..7a78d1b 100644
>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>> @@ -35,8 +35,12 @@
>>>   			compatible = "arm,cortex-a15";
>>>   			timer {
>>>   				compatible = "arm,armv7-timer";
>>> -				/* 14th PPI IRQ, active low level-sensitive */
>>> -				interrupts = <1 14 0x308>;
>>> +				/*
>>> +				 * PPI secure/nonsecure IRQ,
>>> +				 * active low level-sensitive
>>> +				 */
>>> +				interrupts = <1 13 0x308>,
>>> +					     <1 14 0x308>;
>>
>> Care to add the virtual and HYP timer interrupts? So KVM can get a
>> chance to run on this HW...
>>
> Thanks Marc for spotting it. Will take care of it.

I just realised something silly... You have one timer node *per cpu*,
and this is not really expected.

The driver really wants one single node. See
arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts for an example.

Oh, and your GIC node could do with some updating too (no VGIC regs or
interrupt).

	M.
Santosh Shilimkar - Jan. 18, 2013, 6:51 p.m.
On Friday 18 January 2013 10:38 PM, Marc Zyngier wrote:
> On 18/01/13 17:00, Santosh Shilimkar wrote:
>> On Friday 18 January 2013 09:32 PM, Marc Zyngier wrote:
>>> On 18/01/13 15:32, Santosh Shilimkar wrote:
>>>> From: Rajendra Nayak <rnayak@ti.com>
>>>>
>>>> Specify both secure as well as nonsecure PPI IRQ for arch
>>>> timer. This fixes the following errors seen on DT OMAP5 boot..
>>>>
>>>> [    0.000000] arch_timer: No interrupt available, giving up
>>>>
>>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>>>
>>>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>>> ---
>>>>    arch/arm/boot/dts/omap5.dtsi |   16 ++++++++++++----
>>>>    1 file changed, 12 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
>>>> index 790bb2a..7a78d1b 100644
>>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>>> @@ -35,8 +35,12 @@
>>>>    			compatible = "arm,cortex-a15";
>>>>    			timer {
>>>>    				compatible = "arm,armv7-timer";
>>>> -				/* 14th PPI IRQ, active low level-sensitive */
>>>> -				interrupts = <1 14 0x308>;
>>>> +				/*
>>>> +				 * PPI secure/nonsecure IRQ,
>>>> +				 * active low level-sensitive
>>>> +				 */
>>>> +				interrupts = <1 13 0x308>,
>>>> +					     <1 14 0x308>;
>>>
>>> Care to add the virtual and HYP timer interrupts? So KVM can get a
>>> chance to run on this HW...
>>>
>> Thanks Marc for spotting it. Will take care of it.
>
> I just realised something silly... You have one timer node *per cpu*,
> and this is not really expected.
>
This was discussed on the list here [1]
Benoit suggested to add per CPU node since arch timer is per
CPU and DT should describe the hw the way it is. Did we miss
something ?

> The driver really wants one single node. See
> arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts for an example.
>
I remember adding only one node based on above file and then
updating the patch based on the comment.

> Oh, and your GIC node could do with some updating too (no VGIC regs or
> interrupt).
>
Will have a look at that as well.

Regards
Santosh,

[1] https://patchwork.kernel.org/patch/1312061/
Marc Zyngier - Jan. 19, 2013, 2:46 p.m.
On Sat, 19 Jan 2013 00:21:22 +0530, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> On Friday 18 January 2013 10:38 PM, Marc Zyngier wrote:
>> On 18/01/13 17:00, Santosh Shilimkar wrote:
>>> On Friday 18 January 2013 09:32 PM, Marc Zyngier wrote:
>>>> On 18/01/13 15:32, Santosh Shilimkar wrote:
>>>>> From: Rajendra Nayak <rnayak@ti.com>
>>>>>
>>>>> Specify both secure as well as nonsecure PPI IRQ for arch
>>>>> timer. This fixes the following errors seen on DT OMAP5 boot..
>>>>>
>>>>> [    0.000000] arch_timer: No interrupt available, giving up
>>>>>
>>>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>>>>
>>>>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>>>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>>>> ---
>>>>>    arch/arm/boot/dts/omap5.dtsi |   16 ++++++++++++----
>>>>>    1 file changed, 12 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/omap5.dtsi
>>>>> b/arch/arm/boot/dts/omap5.dtsi
>>>>> index 790bb2a..7a78d1b 100644
>>>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>>>> @@ -35,8 +35,12 @@
>>>>>    			compatible = "arm,cortex-a15";
>>>>>    			timer {
>>>>>    				compatible = "arm,armv7-timer";
>>>>> -				/* 14th PPI IRQ, active low level-sensitive */
>>>>> -				interrupts = <1 14 0x308>;
>>>>> +				/*
>>>>> +				 * PPI secure/nonsecure IRQ,
>>>>> +				 * active low level-sensitive
>>>>> +				 */
>>>>> +				interrupts = <1 13 0x308>,
>>>>> +					     <1 14 0x308>;
>>>>
>>>> Care to add the virtual and HYP timer interrupts? So KVM can get a
>>>> chance to run on this HW...
>>>>
>>> Thanks Marc for spotting it. Will take care of it.
>>
>> I just realised something silly... You have one timer node *per cpu*,
>> and this is not really expected.
>>
> This was discussed on the list here [1]
> Benoit suggested to add per CPU node since arch timer is per
> CPU and DT should describe the hw the way it is. Did we miss
> something ?

The current approach is not to duplicate banked resources. We do not have
duplicated twd-timer nodes on Cortex A9, we do not expose multiple CPU
interfaces for the GIC...

And speaking of the GIC: how do you interpret the CPU mask in the
interrupt field of the timer? The value 0x3xx in the third field is there
to indicate that CPU0 and CPU1 are getting interrupted by the timer. What
does it mean when you duplicate it?

        M.
Santosh Shilimkar - Jan. 19, 2013, 5:26 p.m.
On Saturday 19 January 2013 08:16 PM, Marc Zyngier wrote:
> On Sat, 19 Jan 2013 00:21:22 +0530, Santosh Shilimkar
> <santosh.shilimkar@ti.com> wrote:
>> On Friday 18 January 2013 10:38 PM, Marc Zyngier wrote:
>>> On 18/01/13 17:00, Santosh Shilimkar wrote:
>>>> On Friday 18 January 2013 09:32 PM, Marc Zyngier wrote:
>>>>> On 18/01/13 15:32, Santosh Shilimkar wrote:
>>>>>> From: Rajendra Nayak <rnayak@ti.com>
>>>>>>
>>>>>> Specify both secure as well as nonsecure PPI IRQ for arch
>>>>>> timer. This fixes the following errors seen on DT OMAP5 boot..
>>>>>>
>>>>>> [    0.000000] arch_timer: No interrupt available, giving up
>>>>>>
>>>>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>>>>>
>>>>>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>>>>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>>>>> ---
>>>>>>     arch/arm/boot/dts/omap5.dtsi |   16 ++++++++++++----
>>>>>>     1 file changed, 12 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/omap5.dtsi
>>>>>> b/arch/arm/boot/dts/omap5.dtsi
>>>>>> index 790bb2a..7a78d1b 100644
>>>>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>>>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>>>>> @@ -35,8 +35,12 @@
>>>>>>     			compatible = "arm,cortex-a15";
>>>>>>     			timer {
>>>>>>     				compatible = "arm,armv7-timer";
>>>>>> -				/* 14th PPI IRQ, active low level-sensitive */
>>>>>> -				interrupts = <1 14 0x308>;
>>>>>> +				/*
>>>>>> +				 * PPI secure/nonsecure IRQ,
>>>>>> +				 * active low level-sensitive
>>>>>> +				 */
>>>>>> +				interrupts = <1 13 0x308>,
>>>>>> +					     <1 14 0x308>;
>>>>>
>>>>> Care to add the virtual and HYP timer interrupts? So KVM can get a
>>>>> chance to run on this HW...
>>>>>
>>>> Thanks Marc for spotting it. Will take care of it.
>>>
>>> I just realised something silly... You have one timer node *per cpu*,
>>> and this is not really expected.
>>>
>> This was discussed on the list here [1]
>> Benoit suggested to add per CPU node since arch timer is per
>> CPU and DT should describe the hw the way it is. Did we miss
>> something ?
>
> The current approach is not to duplicate banked resources. We do not have
> duplicated twd-timer nodes on Cortex A9, we do not expose multiple CPU
> interfaces for the GIC...
>
Yes, I have observed that. The arch/twd timer DT extraction code doesn't
expect per CPU DT node and on that basis only, initial patch was
adding single timer node outside cpu node. Benoit comment was that
the DT should describe the way hardware is and probably the DT
extraction code should be updated accordingly.

> And speaking of the GIC: how do you interpret the CPU mask in the
> interrupt field of the timer? The value 0x3xx in the third field is there
> to indicate that CPU0 and CPU1 are getting interrupted by the timer. What
> does it mean when you duplicate it?>          M.
>

>
Here too I have to keep mask for both CPUs because of the current DT
extraction code. I some how forgot to bring that discussion to your
notice. I can update the DT files to go back to the single timer
node if the plan is not to duplicate the per CPU resources.

Benoit,
Whats your take on it ?

Regards
Santosh
Hunter, Jon - Jan. 30, 2013, 5:42 p.m.
On 01/18/2013 09:32 AM, Santosh Shilimkar wrote:
> OMAP5 clockdata has different sys clock clock node name. Fix the timer code
> to take care of it.
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/mach-omap2/timer.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 691aa67..a0a1b14 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -62,6 +62,7 @@
>  #define OMAP2_MPU_SOURCE	"sys_ck"
>  #define OMAP3_MPU_SOURCE	OMAP2_MPU_SOURCE
>  #define OMAP4_MPU_SOURCE	"sys_clkin_ck"
> +#define OMAP5_MPU_SOURCE	"sys_clkin"

I would like to remove this definitions and in fact posted a patch today
to do that [1] ;-)

>  #define OMAP2_32K_SOURCE	"func_32k_ck"
>  #define OMAP3_32K_SOURCE	"omap_32k_fck"
>  #define OMAP4_32K_SOURCE	"sys_32k_ck"
> @@ -498,7 +499,7 @@ static void __init realtime_counter_init(void)
>  		pr_err("%s: ioremap failed\n", __func__);
>  		return;
>  	}
> -	sys_clk = clk_get(NULL, "sys_clkin_ck");
> +	sys_clk = clk_get(NULL, OMAP5_MPU_SOURCE);
>  	if (IS_ERR(sys_clk)) {
>  		pr_err("%s: failed to get system clock handle\n", __func__);
>  		iounmap(base);
> @@ -638,7 +639,7 @@ OMAP_SYS_TIMER(4, local);
>  
>  #ifdef CONFIG_SOC_OMAP5
>  OMAP_SYS_32K_TIMER_INIT(5, 1, OMAP4_32K_SOURCE, "ti,timer-alwon",
> -			2, OMAP4_MPU_SOURCE);
> +			2, OMAP5_MPU_SOURCE);

I am hoping we can remove this completely for omap5 and use the same
function defined for omap2 [2].

Care to try your series on top of mine [3] on omap5?

Cheers
Jon

[1] http://www.spinics.net/lists/arm-kernel/msg221263.html
[2] http://www.spinics.net/lists/arm-kernel/msg221264.html
[3] http://www.spinics.net/lists/arm-kernel/msg221260.html