linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
@ 2013-09-18 16:02 Sricharan R
  2013-10-03 10:19 ` Sricharan R
  0 siblings, 1 reply; 10+ messages in thread
From: Sricharan R @ 2013-09-18 16:02 UTC (permalink / raw)
  To: linux-arm-kernel

The realtime counter called master counter, produces the count
used by the private timer peripherals in the MPU_CLUSTER. The
CNTFRQ per cpu register is used to denote the frequency of the counter.
Currently the frequency value is passed from the
DT file, but this is not scalable when we have other non-DT guest
OS. This register must be set to the right value by the
secure rom code. Setting this register helps in propagating the right
frequency value across OSes.

More discussions and the reason for adding this in a non-DT
way can be seen from below.
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg93832.html

So configuring this secure register for all the cpus here.

While here, removing the clock-frequency DT entry for omap5 as
it is no more needed after this patch.

Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Tested-by: Nishanth Menon <nm@ti.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Sricharan R <r.sricharan@ti.com>
---
[V4] Updated commit log, removed the redundant entry in OMAP5.dtsi and
     a unnecessary newline in timer.c

 arch/arm/boot/dts/omap5.dtsi      |    1 -
 arch/arm/mach-omap2/omap-secure.h |    2 ++
 arch/arm/mach-omap2/omap-smp.c    |    9 +++++++++
 arch/arm/mach-omap2/timer.c       |    5 +++++
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 07be2cd..8a45512 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -52,7 +52,6 @@
 			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
 			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
 			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
-		clock-frequency = <6144000>;
 	};
 
 	gic: interrupt-controller at 48211000 {
diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
index 0e72917..5f88824 100644
--- a/arch/arm/mach-omap2/omap-secure.h
+++ b/arch/arm/mach-omap2/omap-secure.h
@@ -42,6 +42,8 @@
 #define OMAP4_MON_L2X0_AUXCTRL_INDEX	0x109
 #define OMAP4_MON_L2X0_PREFETCH_INDEX	0x113
 
+#define OMAP5_DRA7_MON_SET_CNTFRQ_INDEX	0x109
+
 /* Secure PPA(Primary Protected Application) APIs */
 #define OMAP4_PPA_L2_POR_INDEX		0x23
 #define OMAP4_PPA_CPU_ACTRL_SMP_INDEX	0x25
diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
index 8708b2a..5a3c8d3 100644
--- a/arch/arm/mach-omap2/omap-smp.c
+++ b/arch/arm/mach-omap2/omap-smp.c
@@ -41,6 +41,8 @@
 
 u16 pm44xx_errata;
 
+extern unsigned long arch_timer_freq;
+
 /* SCU base address */
 static void __iomem *scu_base;
 
@@ -66,6 +68,13 @@ static void omap4_secondary_init(unsigned int cpu)
 							4, 0, 0, 0, 0, 0);
 
 	/*
+	 * Configure the CNTFRQ register for the secondary cpu's which
+	 * indicates the frequency of the cpu local timers.
+	 */
+	if (soc_is_omap54xx() || soc_is_dra7xx())
+		omap_smc1(OMAP5_DRA7_MON_SET_CNTFRQ_INDEX, arch_timer_freq);
+
+	/*
 	 * Synchronise with the boot thread.
 	 */
 	spin_lock(&boot_lock);
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index fa74a06..d0af9b2 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -55,6 +55,7 @@
 #include "soc.h"
 #include "common.h"
 #include "powerdomain.h"
+#include "omap-secure.h"
 
 #define REALTIME_COUNTER_BASE				0x48243200
 #define INCREMENTER_NUMERATOR_OFFSET			0x10
@@ -65,6 +66,7 @@
 
 static struct omap_dm_timer clkev;
 static struct clock_event_device clockevent_gpt;
+unsigned long arch_timer_freq;
 
 static irqreturn_t omap2_gp_timer_interrupt(int irq, void *dev_id)
 {
@@ -542,6 +544,9 @@ static void __init realtime_counter_init(void)
 	reg |= den;
 	__raw_writel(reg, base + INCREMENTER_DENUMERATOR_RELOAD_OFFSET);
 
+	arch_timer_freq = (rate / den) * num;
+	omap_smc1(OMAP5_DRA7_MON_SET_CNTFRQ_INDEX, arch_timer_freq);
+
 	iounmap(base);
 }
 #else
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
  2013-09-18 16:02 [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register Sricharan R
@ 2013-10-03 10:19 ` Sricharan R
  2013-10-08 21:45   ` Tony Lindgren
  0 siblings, 1 reply; 10+ messages in thread
From: Sricharan R @ 2013-10-03 10:19 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Tony,

On Wednesday 18 September 2013 09:32 PM, Sricharan R wrote:
> The realtime counter called master counter, produces the count
> used by the private timer peripherals in the MPU_CLUSTER. The
> CNTFRQ per cpu register is used to denote the frequency of the counter.
> Currently the frequency value is passed from the
> DT file, but this is not scalable when we have other non-DT guest
> OS. This register must be set to the right value by the
> secure rom code. Setting this register helps in propagating the right
> frequency value across OSes.
>
> More discussions and the reason for adding this in a non-DT
> way can be seen from below.
> http://www.mail-archive.com/linux-omap at vger.kernel.org/msg93832.html
>
> So configuring this secure register for all the cpus here.
>
> While here, removing the clock-frequency DT entry for omap5 as
> it is no more needed after this patch.
>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Rajendra Nayak <rnayak@ti.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Tested-by: Nishanth Menon <nm@ti.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Sricharan R <r.sricharan@ti.com>
> ---
> [V4] Updated commit log, removed the redundant entry in OMAP5.dtsi and
>      a unnecessary newline in timer.c
>
>  arch/arm/boot/dts/omap5.dtsi      |    1 -
>  arch/arm/mach-omap2/omap-secure.h |    2 ++
>  arch/arm/mach-omap2/omap-smp.c    |    9 +++++++++
>  arch/arm/mach-omap2/timer.c       |    5 +++++
>  4 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index 07be2cd..8a45512 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -52,7 +52,6 @@
>  			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>  			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>  			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
> -		clock-frequency = <6144000>;
>  	};
>  
>  	gic: interrupt-controller at 48211000 {
> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
> index 0e72917..5f88824 100644
> --- a/arch/arm/mach-omap2/omap-secure.h
> +++ b/arch/arm/mach-omap2/omap-secure.h
> @@ -42,6 +42,8 @@
>  #define OMAP4_MON_L2X0_AUXCTRL_INDEX	0x109
>  #define OMAP4_MON_L2X0_PREFETCH_INDEX	0x113
>  
> +#define OMAP5_DRA7_MON_SET_CNTFRQ_INDEX	0x109
> +
>  /* Secure PPA(Primary Protected Application) APIs */
>  #define OMAP4_PPA_L2_POR_INDEX		0x23
>  #define OMAP4_PPA_CPU_ACTRL_SMP_INDEX	0x25
> diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
> index 8708b2a..5a3c8d3 100644
> --- a/arch/arm/mach-omap2/omap-smp.c
> +++ b/arch/arm/mach-omap2/omap-smp.c
> @@ -41,6 +41,8 @@
>  
>  u16 pm44xx_errata;
>  
> +extern unsigned long arch_timer_freq;
> +
>  /* SCU base address */
>  static void __iomem *scu_base;
>  
> @@ -66,6 +68,13 @@ static void omap4_secondary_init(unsigned int cpu)
>  							4, 0, 0, 0, 0, 0);
>  
>  	/*
> +	 * Configure the CNTFRQ register for the secondary cpu's which
> +	 * indicates the frequency of the cpu local timers.
> +	 */
> +	if (soc_is_omap54xx() || soc_is_dra7xx())
> +		omap_smc1(OMAP5_DRA7_MON_SET_CNTFRQ_INDEX, arch_timer_freq);
> +
> +	/*
>  	 * Synchronise with the boot thread.
>  	 */
>  	spin_lock(&boot_lock);
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index fa74a06..d0af9b2 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -55,6 +55,7 @@
>  #include "soc.h"
>  #include "common.h"
>  #include "powerdomain.h"
> +#include "omap-secure.h"
>  
>  #define REALTIME_COUNTER_BASE				0x48243200
>  #define INCREMENTER_NUMERATOR_OFFSET			0x10
> @@ -65,6 +66,7 @@
>  
>  static struct omap_dm_timer clkev;
>  static struct clock_event_device clockevent_gpt;
> +unsigned long arch_timer_freq;
>  
>  static irqreturn_t omap2_gp_timer_interrupt(int irq, void *dev_id)
>  {
> @@ -542,6 +544,9 @@ static void __init realtime_counter_init(void)
>  	reg |= den;
>  	__raw_writel(reg, base + INCREMENTER_DENUMERATOR_RELOAD_OFFSET);
>  
> +	arch_timer_freq = (rate / den) * num;
> +	omap_smc1(OMAP5_DRA7_MON_SET_CNTFRQ_INDEX, arch_timer_freq);
> +
>  	iounmap(base);
>  }
>  #else
 Are you planning to pull this patch and the below $subject patch as well? They are
 acked and tested.

 ARM: DRA7: realtime_counter: Add ratio registers for 20MHZ sys-clk frequency

  http://www.spinics.net/lists/linux-omap/msg97281.html

 
Regards,
 Sricharan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
  2013-10-03 10:19 ` Sricharan R
@ 2013-10-08 21:45   ` Tony Lindgren
  2013-10-08 22:05     ` Santosh Shilimkar
  0 siblings, 1 reply; 10+ messages in thread
From: Tony Lindgren @ 2013-10-08 21:45 UTC (permalink / raw)
  To: linux-arm-kernel

* Sricharan R <r.sricharan@ti.com> [131003 03:27]:
> On Wednesday 18 September 2013 09:32 PM, Sricharan R wrote:
> > --- a/arch/arm/boot/dts/omap5.dtsi
> > +++ b/arch/arm/boot/dts/omap5.dtsi
> > @@ -52,7 +52,6 @@
> >  			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
> >  			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
> >  			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
> > -		clock-frequency = <6144000>;
> >  	};
> >  
> >  	gic: interrupt-controller at 48211000 {

Can the above be done later on in a separate clean-up patch?
If so I can drop that part as that removes a dependency to the
.dts patches queued by Benoit.

> > --- a/arch/arm/mach-omap2/omap-smp.c
> > +++ b/arch/arm/mach-omap2/omap-smp.c
> > @@ -41,6 +41,8 @@
> >  
> >  u16 pm44xx_errata;
> >  
> > +extern unsigned long arch_timer_freq;
> > +
> >  /* SCU base address */
> >  static void __iomem *scu_base;
> >  

No externs in *.c files please, checkpatch.pl and sparse should warn
about this.

>  Are you planning to pull this patch and the below $subject patch as well? They are
>  acked and tested.
> 
>  ARM: DRA7: realtime_counter: Add ratio registers for 20MHZ sys-clk frequency
> 
>   http://www.spinics.net/lists/linux-omap/msg97281.html

The 20MHz patch I've applied, just noticed the above things
when was about to apply this.

Regards,

Tony

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
  2013-10-08 21:45   ` Tony Lindgren
@ 2013-10-08 22:05     ` Santosh Shilimkar
  2013-10-09 13:15       ` Sricharan R
  0 siblings, 1 reply; 10+ messages in thread
From: Santosh Shilimkar @ 2013-10-08 22:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 08 October 2013 05:45 PM, Tony Lindgren wrote:
> * Sricharan R <r.sricharan@ti.com> [131003 03:27]:
>> On Wednesday 18 September 2013 09:32 PM, Sricharan R wrote:
>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>> @@ -52,7 +52,6 @@
>>>  			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>  			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>  			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
>>> -		clock-frequency = <6144000>;
>>>  	};
>>>  
>>>  	gic: interrupt-controller at 48211000 {
> 
> Can the above be done later on in a separate clean-up patch?
> If so I can drop that part as that removes a dependency to the
> .dts patches queued by Benoit.
>
This can be applied separately. 

 
>>> --- a/arch/arm/mach-omap2/omap-smp.c
>>> +++ b/arch/arm/mach-omap2/omap-smp.c
>>> @@ -41,6 +41,8 @@
>>>  
>>>  u16 pm44xx_errata;
>>>  
>>> +extern unsigned long arch_timer_freq;
>>> +
>>>  /* SCU base address */
>>>  static void __iomem *scu_base;
>>>  
> 
> No externs in *.c files please, checkpatch.pl and sparse should warn
> about this.
> 
>>  Are you planning to pull this patch and the below $subject patch as well? They are
>>  acked and tested.
>>
>>  ARM: DRA7: realtime_counter: Add ratio registers for 20MHZ sys-clk frequency
>>
>>   http://www.spinics.net/lists/linux-omap/msg97281.html
> 
> The 20MHz patch I've applied, just noticed the above things
> when was about to apply this.
> 
Now re-looking at the patch, I think this extern stuff can be and
should be avoided. It needs order change though like below. Not
tested but should work.

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index fa74a06..c8d8308 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -631,10 +631,9 @@ void __init omap4_local_timer_init(void)
 #ifdef CONFIG_SOC_OMAP5
 void __init omap5_realtime_timer_init(void)
 {
-	omap4_sync32k_timer_init();
 	realtime_counter_init();
-
 	clocksource_of_init();
+	omap4_sync32k_timer_init();
 }
 #endif /* CONFIG_SOC_OMAP5 */
 
Then, the CNTFREQ programming needs to be moved to
realtime_counter_init(). It should be actually part of that
first place instead of timer_init().

On secondary CPU then a simple asm accessor can
read the CNTFREQ and pass that to SMC.

Sricharan,
Can you try above and see if everything works as expected.
If it does, please post an updated patch based on above.

Regards,
santosh

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
  2013-10-08 22:05     ` Santosh Shilimkar
@ 2013-10-09 13:15       ` Sricharan R
  2013-10-09 13:16         ` Sricharan R
  2013-10-09 13:17         ` Santosh Shilimkar
  0 siblings, 2 replies; 10+ messages in thread
From: Sricharan R @ 2013-10-09 13:15 UTC (permalink / raw)
  To: linux-arm-kernel

Santosh,

On Wednesday 09 October 2013 03:35 AM, Santosh Shilimkar wrote:
> On Tuesday 08 October 2013 05:45 PM, Tony Lindgren wrote:
>> * Sricharan R <r.sricharan@ti.com> [131003 03:27]:
>>> On Wednesday 18 September 2013 09:32 PM, Sricharan R wrote:
>>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>>> @@ -52,7 +52,6 @@
>>>>  			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>  			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>  			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
>>>> -		clock-frequency = <6144000>;
>>>>  	};
>>>>  
>>>>  	gic: interrupt-controller at 48211000 {
>> Can the above be done later on in a separate clean-up patch?
>> If so I can drop that part as that removes a dependency to the
>> .dts patches queued by Benoit.
>>
> This can be applied separately. 
>
>  
>>>> --- a/arch/arm/mach-omap2/omap-smp.c
>>>> +++ b/arch/arm/mach-omap2/omap-smp.c
>>>> @@ -41,6 +41,8 @@
>>>>  
>>>>  u16 pm44xx_errata;
>>>>  
>>>> +extern unsigned long arch_timer_freq;
>>>> +
>>>>  /* SCU base address */
>>>>  static void __iomem *scu_base;
>>>>  
>> No externs in *.c files please, checkpatch.pl and sparse should warn
>> about this.
>>
>>>  Are you planning to pull this patch and the below $subject patch as well? They are
>>>  acked and tested.
>>>
>>>  ARM: DRA7: realtime_counter: Add ratio registers for 20MHZ sys-clk frequency
>>>
>>>   http://www.spinics.net/lists/linux-omap/msg97281.html
>> The 20MHz patch I've applied, just noticed the above things
>> when was about to apply this.
>>
> Now re-looking at the patch, I think this extern stuff can be and
> should be avoided. It needs order change though like below. Not
> tested but should work.
>
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index fa74a06..c8d8308 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -631,10 +631,9 @@ void __init omap4_local_timer_init(void)
>  #ifdef CONFIG_SOC_OMAP5
>  void __init omap5_realtime_timer_init(void)
>  {
> -	omap4_sync32k_timer_init();
>  	realtime_counter_init();
> -
>  	clocksource_of_init();
> +	omap4_sync32k_timer_init();
>  }
>  #endif /* CONFIG_SOC_OMAP5 */
>  
> Then, the CNTFREQ programming needs to be moved to
> realtime_counter_init(). It should be actually part of that
> first place instead of timer_init().
>
> On secondary CPU then a simple asm accessor can
> read the CNTFREQ and pass that to SMC.
Sorry,  I did not quite get you here. You mean an asm accessor to
 the read the variable that is set in timer.c ?

Regards,
 Sricharan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
  2013-10-09 13:15       ` Sricharan R
@ 2013-10-09 13:16         ` Sricharan R
  2013-10-09 13:18           ` Santosh Shilimkar
  2013-10-09 13:17         ` Santosh Shilimkar
  1 sibling, 1 reply; 10+ messages in thread
From: Sricharan R @ 2013-10-09 13:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 October 2013 06:45 PM, Sricharan R wrote:
> Santosh,
>
> On Wednesday 09 October 2013 03:35 AM, Santosh Shilimkar wrote:
>> On Tuesday 08 October 2013 05:45 PM, Tony Lindgren wrote:
>>> * Sricharan R <r.sricharan@ti.com> [131003 03:27]:
>>>> On Wednesday 18 September 2013 09:32 PM, Sricharan R wrote:
>>>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>>>> @@ -52,7 +52,6 @@
>>>>>  			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>  			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>  			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
>>>>> -		clock-frequency = <6144000>;
>>>>>  	};
>>>>>  
>>>>>  	gic: interrupt-controller at 48211000 {
>>> Can the above be done later on in a separate clean-up patch?
>>> If so I can drop that part as that removes a dependency to the
>>> .dts patches queued by Benoit.
>>>
>> This can be applied separately. 
>>
>>  
>>>>> --- a/arch/arm/mach-omap2/omap-smp.c
>>>>> +++ b/arch/arm/mach-omap2/omap-smp.c
>>>>> @@ -41,6 +41,8 @@
>>>>>  
>>>>>  u16 pm44xx_errata;
>>>>>  
>>>>> +extern unsigned long arch_timer_freq;
>>>>> +
>>>>>  /* SCU base address */
>>>>>  static void __iomem *scu_base;
>>>>>  
>>> No externs in *.c files please, checkpatch.pl and sparse should warn
>>> about this.
>>>
>>>>  Are you planning to pull this patch and the below $subject patch as well? They are
>>>>  acked and tested.
>>>>
>>>>  ARM: DRA7: realtime_counter: Add ratio registers for 20MHZ sys-clk frequency
>>>>
>>>>   http://www.spinics.net/lists/linux-omap/msg97281.html
>>> The 20MHz patch I've applied, just noticed the above things
>>> when was about to apply this.
>>>
>> Now re-looking at the patch, I think this extern stuff can be and
>> should be avoided. It needs order change though like below. Not
>> tested but should work.
>>
>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>> index fa74a06..c8d8308 100644
>> --- a/arch/arm/mach-omap2/timer.c
>> +++ b/arch/arm/mach-omap2/timer.c
>> @@ -631,10 +631,9 @@ void __init omap4_local_timer_init(void)
>>  #ifdef CONFIG_SOC_OMAP5
>>  void __init omap5_realtime_timer_init(void)
>>  {
>> -	omap4_sync32k_timer_init();
>>  	realtime_counter_init();
>> -
>>  	clocksource_of_init();
>> +	omap4_sync32k_timer_init();
>>  }
>>  #endif /* CONFIG_SOC_OMAP5 */
>>  
>> Then, the CNTFREQ programming needs to be moved to
>> realtime_counter_init(). It should be actually part of that
>> first place instead of timer_init().
>>
>> On secondary CPU then a simple asm accessor can
>> read the CNTFREQ and pass that to SMC.
> Sorry,  I did not quite get you here. You mean an asm accessor to
>  the read the variable that is set in timer.c ?
 Also, is it not ok to move the extern to a .h file instead ?

Regards,
 Sricharan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
  2013-10-09 13:15       ` Sricharan R
  2013-10-09 13:16         ` Sricharan R
@ 2013-10-09 13:17         ` Santosh Shilimkar
  2013-10-09 13:20           ` Sricharan R
  1 sibling, 1 reply; 10+ messages in thread
From: Santosh Shilimkar @ 2013-10-09 13:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 October 2013 09:15 AM, Sricharan R wrote:
> Santosh,
> 
> On Wednesday 09 October 2013 03:35 AM, Santosh Shilimkar wrote:
>> On Tuesday 08 October 2013 05:45 PM, Tony Lindgren wrote:
>>> * Sricharan R <r.sricharan@ti.com> [131003 03:27]:
>>>> On Wednesday 18 September 2013 09:32 PM, Sricharan R wrote:
>>>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>>>> @@ -52,7 +52,6 @@
>>>>>  			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>  			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>  			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
>>>>> -		clock-frequency = <6144000>;
>>>>>  	};
>>>>>  
>>>>>  	gic: interrupt-controller at 48211000 {
>>> Can the above be done later on in a separate clean-up patch?
>>> If so I can drop that part as that removes a dependency to the
>>> .dts patches queued by Benoit.
>>>
>> This can be applied separately. 
>>
>>  
>>>>> --- a/arch/arm/mach-omap2/omap-smp.c
>>>>> +++ b/arch/arm/mach-omap2/omap-smp.c
>>>>> @@ -41,6 +41,8 @@
>>>>>  
>>>>>  u16 pm44xx_errata;
>>>>>  
>>>>> +extern unsigned long arch_timer_freq;
>>>>> +
>>>>>  /* SCU base address */
>>>>>  static void __iomem *scu_base;
>>>>>  
>>> No externs in *.c files please, checkpatch.pl and sparse should warn
>>> about this.
>>>
>>>>  Are you planning to pull this patch and the below $subject patch as well? They are
>>>>  acked and tested.
>>>>
>>>>  ARM: DRA7: realtime_counter: Add ratio registers for 20MHZ sys-clk frequency
>>>>
>>>>   http://www.spinics.net/lists/linux-omap/msg97281.html
>>> The 20MHz patch I've applied, just noticed the above things
>>> when was about to apply this.
>>>
>> Now re-looking at the patch, I think this extern stuff can be and
>> should be avoided. It needs order change though like below. Not
>> tested but should work.
>>
>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>> index fa74a06..c8d8308 100644
>> --- a/arch/arm/mach-omap2/timer.c
>> +++ b/arch/arm/mach-omap2/timer.c
>> @@ -631,10 +631,9 @@ void __init omap4_local_timer_init(void)
>>  #ifdef CONFIG_SOC_OMAP5
>>  void __init omap5_realtime_timer_init(void)
>>  {
>> -	omap4_sync32k_timer_init();
>>  	realtime_counter_init();
>> -
>>  	clocksource_of_init();
>> +	omap4_sync32k_timer_init();
>>  }
>>  #endif /* CONFIG_SOC_OMAP5 */
>>  
>> Then, the CNTFREQ programming needs to be moved to
>> realtime_counter_init(). It should be actually part of that
>> first place instead of timer_init().
>>
>> On secondary CPU then a simple asm accessor can
>> read the CNTFREQ and pass that to SMC.
> Sorry,  I did not quite get you here. You mean an asm accessor to
>  the read the variable that is set in timer.c ?
> 
Reading CNTFREQ CP15 directly....

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
  2013-10-09 13:16         ` Sricharan R
@ 2013-10-09 13:18           ` Santosh Shilimkar
  0 siblings, 0 replies; 10+ messages in thread
From: Santosh Shilimkar @ 2013-10-09 13:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 October 2013 09:16 AM, Sricharan R wrote:
> On Wednesday 09 October 2013 06:45 PM, Sricharan R wrote:
>> Santosh,
>>
>> On Wednesday 09 October 2013 03:35 AM, Santosh Shilimkar wrote:
>>> On Tuesday 08 October 2013 05:45 PM, Tony Lindgren wrote:
>>>> * Sricharan R <r.sricharan@ti.com> [131003 03:27]:
>>>>> On Wednesday 18 September 2013 09:32 PM, Sricharan R wrote:
>>>>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>>>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>>>>> @@ -52,7 +52,6 @@
>>>>>>  			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>>  			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>>  			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
>>>>>> -		clock-frequency = <6144000>;
>>>>>>  	};
>>>>>>  
>>>>>>  	gic: interrupt-controller at 48211000 {
>>>> Can the above be done later on in a separate clean-up patch?
>>>> If so I can drop that part as that removes a dependency to the
>>>> .dts patches queued by Benoit.
>>>>
>>> This can be applied separately. 
>>>
>>>  
>>>>>> --- a/arch/arm/mach-omap2/omap-smp.c
>>>>>> +++ b/arch/arm/mach-omap2/omap-smp.c
>>>>>> @@ -41,6 +41,8 @@
>>>>>>  
>>>>>>  u16 pm44xx_errata;
>>>>>>  
>>>>>> +extern unsigned long arch_timer_freq;
>>>>>> +
>>>>>>  /* SCU base address */
>>>>>>  static void __iomem *scu_base;
>>>>>>  
>>>> No externs in *.c files please, checkpatch.pl and sparse should warn
>>>> about this.
>>>>
>>>>>  Are you planning to pull this patch and the below $subject patch as well? They are
>>>>>  acked and tested.
>>>>>
>>>>>  ARM: DRA7: realtime_counter: Add ratio registers for 20MHZ sys-clk frequency
>>>>>
>>>>>   http://www.spinics.net/lists/linux-omap/msg97281.html
>>>> The 20MHz patch I've applied, just noticed the above things
>>>> when was about to apply this.
>>>>
>>> Now re-looking at the patch, I think this extern stuff can be and
>>> should be avoided. It needs order change though like below. Not
>>> tested but should work.
>>>
>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>> index fa74a06..c8d8308 100644
>>> --- a/arch/arm/mach-omap2/timer.c
>>> +++ b/arch/arm/mach-omap2/timer.c
>>> @@ -631,10 +631,9 @@ void __init omap4_local_timer_init(void)
>>>  #ifdef CONFIG_SOC_OMAP5
>>>  void __init omap5_realtime_timer_init(void)
>>>  {
>>> -	omap4_sync32k_timer_init();
>>>  	realtime_counter_init();
>>> -
>>>  	clocksource_of_init();
>>> +	omap4_sync32k_timer_init();
>>>  }
>>>  #endif /* CONFIG_SOC_OMAP5 */
>>>  
>>> Then, the CNTFREQ programming needs to be moved to
>>> realtime_counter_init(). It should be actually part of that
>>> first place instead of timer_init().
>>>
>>> On secondary CPU then a simple asm accessor can
>>> read the CNTFREQ and pass that to SMC.
>> Sorry,  I did not quite get you here. You mean an asm accessor to
>>  the read the variable that is set in timer.c ?
>  Also, is it not ok to move the extern to a .h file instead ?
> 
I think you can avoid that variable export with above suggestion

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
  2013-10-09 13:17         ` Santosh Shilimkar
@ 2013-10-09 13:20           ` Sricharan R
  2013-10-09 13:23             ` Santosh Shilimkar
  0 siblings, 1 reply; 10+ messages in thread
From: Sricharan R @ 2013-10-09 13:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 October 2013 06:47 PM, Santosh Shilimkar wrote:
> On Wednesday 09 October 2013 09:15 AM, Sricharan R wrote:
>> Santosh,
>>
>> On Wednesday 09 October 2013 03:35 AM, Santosh Shilimkar wrote:
>>> On Tuesday 08 October 2013 05:45 PM, Tony Lindgren wrote:
>>>> * Sricharan R <r.sricharan@ti.com> [131003 03:27]:
>>>>> On Wednesday 18 September 2013 09:32 PM, Sricharan R wrote:
>>>>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>>>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>>>>> @@ -52,7 +52,6 @@
>>>>>>  			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>>  			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>>  			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
>>>>>> -		clock-frequency = <6144000>;
>>>>>>  	};
>>>>>>  
>>>>>>  	gic: interrupt-controller at 48211000 {
>>>> Can the above be done later on in a separate clean-up patch?
>>>> If so I can drop that part as that removes a dependency to the
>>>> .dts patches queued by Benoit.
>>>>
>>> This can be applied separately. 
>>>
>>>  
>>>>>> --- a/arch/arm/mach-omap2/omap-smp.c
>>>>>> +++ b/arch/arm/mach-omap2/omap-smp.c
>>>>>> @@ -41,6 +41,8 @@
>>>>>>  
>>>>>>  u16 pm44xx_errata;
>>>>>>  
>>>>>> +extern unsigned long arch_timer_freq;
>>>>>> +
>>>>>>  /* SCU base address */
>>>>>>  static void __iomem *scu_base;
>>>>>>  
>>>> No externs in *.c files please, checkpatch.pl and sparse should warn
>>>> about this.
>>>>
>>>>>  Are you planning to pull this patch and the below $subject patch as well? They are
>>>>>  acked and tested.
>>>>>
>>>>>  ARM: DRA7: realtime_counter: Add ratio registers for 20MHZ sys-clk frequency
>>>>>
>>>>>   http://www.spinics.net/lists/linux-omap/msg97281.html
>>>> The 20MHz patch I've applied, just noticed the above things
>>>> when was about to apply this.
>>>>
>>> Now re-looking at the patch, I think this extern stuff can be and
>>> should be avoided. It needs order change though like below. Not
>>> tested but should work.
>>>
>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>> index fa74a06..c8d8308 100644
>>> --- a/arch/arm/mach-omap2/timer.c
>>> +++ b/arch/arm/mach-omap2/timer.c
>>> @@ -631,10 +631,9 @@ void __init omap4_local_timer_init(void)
>>>  #ifdef CONFIG_SOC_OMAP5
>>>  void __init omap5_realtime_timer_init(void)
>>>  {
>>> -	omap4_sync32k_timer_init();
>>>  	realtime_counter_init();
>>> -
>>>  	clocksource_of_init();
>>> +	omap4_sync32k_timer_init();
>>>  }
>>>  #endif /* CONFIG_SOC_OMAP5 */
>>>  
>>> Then, the CNTFREQ programming needs to be moved to
>>> realtime_counter_init(). It should be actually part of that
>>> first place instead of timer_init().
>>>
>>> On secondary CPU then a simple asm accessor can
>>> read the CNTFREQ and pass that to SMC.
>> Sorry,  I did not quite get you here. You mean an asm accessor to
>>  the read the variable that is set in timer.c ?
>>
> Reading CNTFREQ CP15 directly....
>
  Sorry, Still not clear. CNTFREQ is per-cpu register. So when
 the secondary cpu init happens on CPU1, then you mean we will have
 to read it from CPU0 register and pass it for secondary ?

Regards,
 Sricharan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register
  2013-10-09 13:20           ` Sricharan R
@ 2013-10-09 13:23             ` Santosh Shilimkar
  0 siblings, 0 replies; 10+ messages in thread
From: Santosh Shilimkar @ 2013-10-09 13:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 October 2013 09:20 AM, Sricharan R wrote:
> On Wednesday 09 October 2013 06:47 PM, Santosh Shilimkar wrote:
>> On Wednesday 09 October 2013 09:15 AM, Sricharan R wrote:
>>> Santosh,
>>>
>>> On Wednesday 09 October 2013 03:35 AM, Santosh Shilimkar wrote:
>>>> On Tuesday 08 October 2013 05:45 PM, Tony Lindgren wrote:
>>>>> * Sricharan R <r.sricharan@ti.com> [131003 03:27]:
>>>>>> On Wednesday 18 September 2013 09:32 PM, Sricharan R wrote:
>>>>>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>>>>>> @@ -52,7 +52,6 @@
>>>>>>>  			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>>>  			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
>>>>>>>  			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
>>>>>>> -		clock-frequency = <6144000>;
>>>>>>>  	};
>>>>>>>  
>>>>>>>  	gic: interrupt-controller at 48211000 {
>>>>> Can the above be done later on in a separate clean-up patch?
>>>>> If so I can drop that part as that removes a dependency to the
>>>>> .dts patches queued by Benoit.
>>>>>
>>>> This can be applied separately. 
>>>>
>>>>  
>>>>>>> --- a/arch/arm/mach-omap2/omap-smp.c
>>>>>>> +++ b/arch/arm/mach-omap2/omap-smp.c
>>>>>>> @@ -41,6 +41,8 @@
>>>>>>>  
>>>>>>>  u16 pm44xx_errata;
>>>>>>>  
>>>>>>> +extern unsigned long arch_timer_freq;
>>>>>>> +
>>>>>>>  /* SCU base address */
>>>>>>>  static void __iomem *scu_base;
>>>>>>>  
>>>>> No externs in *.c files please, checkpatch.pl and sparse should warn
>>>>> about this.
>>>>>
>>>>>>  Are you planning to pull this patch and the below $subject patch as well? They are
>>>>>>  acked and tested.
>>>>>>
>>>>>>  ARM: DRA7: realtime_counter: Add ratio registers for 20MHZ sys-clk frequency
>>>>>>
>>>>>>   http://www.spinics.net/lists/linux-omap/msg97281.html
>>>>> The 20MHz patch I've applied, just noticed the above things
>>>>> when was about to apply this.
>>>>>
>>>> Now re-looking at the patch, I think this extern stuff can be and
>>>> should be avoided. It needs order change though like below. Not
>>>> tested but should work.
>>>>
>>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>>> index fa74a06..c8d8308 100644
>>>> --- a/arch/arm/mach-omap2/timer.c
>>>> +++ b/arch/arm/mach-omap2/timer.c
>>>> @@ -631,10 +631,9 @@ void __init omap4_local_timer_init(void)
>>>>  #ifdef CONFIG_SOC_OMAP5
>>>>  void __init omap5_realtime_timer_init(void)
>>>>  {
>>>> -	omap4_sync32k_timer_init();
>>>>  	realtime_counter_init();
>>>> -
>>>>  	clocksource_of_init();
>>>> +	omap4_sync32k_timer_init();
>>>>  }
>>>>  #endif /* CONFIG_SOC_OMAP5 */
>>>>  
>>>> Then, the CNTFREQ programming needs to be moved to
>>>> realtime_counter_init(). It should be actually part of that
>>>> first place instead of timer_init().
>>>>
>>>> On secondary CPU then a simple asm accessor can
>>>> read the CNTFREQ and pass that to SMC.
>>> Sorry,  I did not quite get you here. You mean an asm accessor to
>>>  the read the variable that is set in timer.c ?
>>>
>> Reading CNTFREQ CP15 directly....
>>
>   Sorry, Still not clear. CNTFREQ is per-cpu register. So when
>  the secondary cpu init happens on CPU1, then you mean we will have
>  to read it from CPU0 register and pass it for secondary ?
> 
Create a function within timer.c which sets the freq for a given CPU.
On Boot CPU you figure out frequency and then just use that static
variable.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-10-09 13:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-18 16:02 [PATCH V4] ARM: OMAP5/DRA7: realtime_counter: Configure CNTFRQ register Sricharan R
2013-10-03 10:19 ` Sricharan R
2013-10-08 21:45   ` Tony Lindgren
2013-10-08 22:05     ` Santosh Shilimkar
2013-10-09 13:15       ` Sricharan R
2013-10-09 13:16         ` Sricharan R
2013-10-09 13:18           ` Santosh Shilimkar
2013-10-09 13:17         ` Santosh Shilimkar
2013-10-09 13:20           ` Sricharan R
2013-10-09 13:23             ` Santosh Shilimkar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).