[PATCH 0/2] PPI fixes for 3.2

[PATCH 0/2] PPI fixes for 3.2

[Marc Zyngier]

After the merge of the percpu interrupt patches, a couple of platforms
need to be updated:
- Samsung MCT timers have gained a PPI implementation using the old
  API,
- imx6q needs to be updated not to care about local timer interrupts
  anymore.

The patches are also available at the following location:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git ppi-fixes-for-3.2

Thanks,

	M.

Marc Zyngier (2):
  ARM: EXYNOS4: convert MCT to percpu interrupt API
  ARM: mcx: fix local timer interrupt handling

 arch/arm/mach-exynos4/mct.c |   40 +++++++++++++++++++++++++++-------------
 arch/arm/plat-mxc/gic.c     |   11 ++---------
 2 files changed, 29 insertions(+), 22 deletions(-)

[PATCH 1/2] ARM: EXYNOS4: convert MCT to percpu interrupt API

[Marc Zyngier]

MCT recently gained per cpu interrupts, and missed the fact that
ARM has moved to a genirq based implementation.

This patch converts the driver to the new API.

Boot tested on Origen.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-exynos4/mct.c |   40 +++++++++++++++++++++++++++-------------
 1 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-exynos4/mct.c b/arch/arm/mach-exynos4/mct.c
index f191608..97343df 100644
--- a/arch/arm/mach-exynos4/mct.c
+++ b/arch/arm/mach-exynos4/mct.c
@@ -44,7 +44,7 @@ struct mct_clock_event_device {
 	char name[10];
 };
 
-struct mct_clock_event_device mct_tick[NR_CPUS];
+static DEFINE_PER_CPU(struct mct_clock_event_device, percpu_mct_tick);
 
 static void exynos4_mct_write(unsigned int value, void *addr)
 {
@@ -302,7 +302,7 @@ static void exynos4_mct_tick_start(unsigned long cycles,
 static int exynos4_tick_set_next_event(unsigned long cycles,
 				       struct clock_event_device *evt)
 {
-	struct mct_clock_event_device *mevt = &mct_tick[smp_processor_id()];
+	struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick);
 
 	exynos4_mct_tick_start(cycles, mevt);
 
@@ -312,7 +312,7 @@ static int exynos4_tick_set_next_event(unsigned long cycles,
 static inline void exynos4_tick_set_mode(enum clock_event_mode mode,
 					 struct clock_event_device *evt)
 {
-	struct mct_clock_event_device *mevt = &mct_tick[smp_processor_id()];
+	struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick);
 
 	exynos4_mct_tick_stop(mevt);
 
@@ -376,14 +376,16 @@ static struct irqaction mct_tick1_event_irq = {
 
 static void exynos4_mct_tick_init(struct clock_event_device *evt)
 {
+	struct mct_clock_event_device *mevt;
 	unsigned int cpu = smp_processor_id();
 
-	mct_tick[cpu].evt = evt;
+	mevt = this_cpu_ptr(&percpu_mct_tick);
+	mevt->evt = evt;
 
-	mct_tick[cpu].base = EXYNOS4_MCT_L_BASE(cpu);
-	sprintf(mct_tick[cpu].name, "mct_tick%d", cpu);
+	mevt->base = EXYNOS4_MCT_L_BASE(cpu);
+	sprintf(mevt->name, "mct_tick%d", cpu);
 
-	evt->name = mct_tick[cpu].name;
+	evt->name = mevt->name;
 	evt->cpumask = cpumask_of(cpu);
 	evt->set_next_event = exynos4_tick_set_next_event;
 	evt->set_mode = exynos4_tick_set_mode;
@@ -398,21 +400,21 @@ static void exynos4_mct_tick_init(struct clock_event_device *evt)
 
 	clockevents_register_device(evt);
 
-	exynos4_mct_write(0x1, mct_tick[cpu].base + MCT_L_TCNTB_OFFSET);
+	exynos4_mct_write(0x1, mevt->base + MCT_L_TCNTB_OFFSET);
 
 	if (mct_int_type == MCT_INT_SPI) {
 		if (cpu == 0) {
-			mct_tick0_event_irq.dev_id = &mct_tick[cpu];
+			mct_tick0_event_irq.dev_id = mevt;
 			evt->irq = IRQ_MCT_L0;
 			setup_irq(IRQ_MCT_L0, &mct_tick0_event_irq);
 		} else {
-			mct_tick1_event_irq.dev_id = &mct_tick[cpu];
+			mct_tick1_event_irq.dev_id = mevt;
 			evt->irq = IRQ_MCT_L1;
 			setup_irq(IRQ_MCT_L1, &mct_tick1_event_irq);
 			irq_set_affinity(IRQ_MCT_L1, cpumask_of(1));
 		}
 	} else {
-		gic_enable_ppi(IRQ_MCT_LOCALTIMER);
+		enable_percpu_irq(IRQ_MCT_LOCALTIMER, 0);
 	}
 }
 
@@ -427,9 +429,11 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
 void local_timer_stop(struct clock_event_device *evt)
 {
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
-	disable_irq(evt->irq);
+	if (mct_int_type == MCT_INT_SPI)
+		disable_irq(evt->irq);
+	else
+		disable_percpu_irq(IRQ_MCT_LOCALTIMER);
 }
-
 #endif /* CONFIG_LOCAL_TIMERS */
 
 static void __init exynos4_timer_resources(void)
@@ -438,6 +442,16 @@ static void __init exynos4_timer_resources(void)
 	mct_clk = clk_get(NULL, "xtal");
 
 	clk_rate = clk_get_rate(mct_clk);
+
+	if (mct_int_type == MCT_INT_PPI) {
+		int err;
+
+		err = request_percpu_irq(IRQ_MCT_LOCALTIMER,
+					 exynos4_mct_tick_isr, "MCT",
+					 &percpu_mct_tick);
+		WARN(err, "MCT: can't request IRQ %d (%d)\n",
+		     IRQ_MCT_LOCALTIMER, err);
+	}
 }
 
 static void __init exynos4_timer_init(void)
-- 
1.7.0.4

[PATCH 2/2] ARM: mcx: fix local timer interrupt handling

[Marc Zyngier]

As local timer interrupts are now handled as normal interrupts,
remove the special case in the GIC handler.

Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/plat-mxc/gic.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/arm/plat-mxc/gic.c b/arch/arm/plat-mxc/gic.c
index b3b8eed..12f8f81 100644
--- a/arch/arm/plat-mxc/gic.c
+++ b/arch/arm/plat-mxc/gic.c
@@ -28,21 +28,14 @@ asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
 		if (irqnr == 1023)
 			break;
 
-		if (irqnr > 29 && irqnr < 1021)
+		if (irqnr > 15 && irqnr < 1021)
 			handle_IRQ(irqnr, regs);
 #ifdef CONFIG_SMP
-		else if (irqnr < 16) {
+		else {
 			writel_relaxed(irqstat, gic_cpu_base_addr +
 						GIC_CPU_EOI);
 			handle_IPI(irqnr, regs);
 		}
 #endif
-#ifdef CONFIG_LOCAL_TIMERS
-		else if (irqnr == 29) {
-			writel_relaxed(irqstat, gic_cpu_base_addr +
-						GIC_CPU_EOI);
-			handle_local_timer(regs);
-		}
-#endif
 	} while (1);
 }
-- 
1.7.0.4

[PATCH 2/2] ARM: mcx: fix local timer interrupt handling

[Uwe Kleine-König]

$Subject ~= s/cx/xc/;

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 2/2] ARM: mcx: fix local timer interrupt handling

[Shawn Guo]

On 3 November 2011 01:30, Marc Zyngier <marc.zyngier@arm.com> wrote:
> As local timer interrupts are now handled as normal interrupts,
> remove the special case in the GIC handler.
>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

I also noticed this problem and had a patch fixing it.  But due to the
tight Linaro Connect agenda, I have not got the chance to send it out.
 Glad to see this patch fixing the problem.

Acked-and-tested-by: Shawn Guo <shawn.guo@linaro.org>

> ---
> ?arch/arm/plat-mxc/gic.c | ? 11 ++---------
> ?1 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/plat-mxc/gic.c b/arch/arm/plat-mxc/gic.c
> index b3b8eed..12f8f81 100644
> --- a/arch/arm/plat-mxc/gic.c
> +++ b/arch/arm/plat-mxc/gic.c
> @@ -28,21 +28,14 @@ asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
> ? ? ? ? ? ? ? ?if (irqnr == 1023)
> ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> - ? ? ? ? ? ? ? if (irqnr > 29 && irqnr < 1021)
> + ? ? ? ? ? ? ? if (irqnr > 15 && irqnr < 1021)
> ? ? ? ? ? ? ? ? ? ? ? ?handle_IRQ(irqnr, regs);
> ?#ifdef CONFIG_SMP
> - ? ? ? ? ? ? ? else if (irqnr < 16) {
> + ? ? ? ? ? ? ? else {
> ? ? ? ? ? ? ? ? ? ? ? ?writel_relaxed(irqstat, gic_cpu_base_addr +
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GIC_CPU_EOI);
> ? ? ? ? ? ? ? ? ? ? ? ?handle_IPI(irqnr, regs);
> ? ? ? ? ? ? ? ?}
> ?#endif
> -#ifdef CONFIG_LOCAL_TIMERS
> - ? ? ? ? ? ? ? else if (irqnr == 29) {
> - ? ? ? ? ? ? ? ? ? ? ? writel_relaxed(irqstat, gic_cpu_base_addr +
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GIC_CPU_EOI);
> - ? ? ? ? ? ? ? ? ? ? ? handle_local_timer(regs);
> - ? ? ? ? ? ? ? }
> -#endif
> ? ? ? ?} while (1);
> ?}
> --
> 1.7.0.4
>
>
>



-- 
Regards,
Shawn

[PATCH 1/2] ARM: EXYNOS4: convert MCT to percpu interrupt API

[Kukjin Kim]

Marc Zyngier wrote:
> 
> MCT recently gained per cpu interrupts, and missed the fact that
> ARM has moved to a genirq based implementation.
> 
> This patch converts the driver to the new API.
> 
> Boot tested on Origen.
> 
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/mach-exynos4/mct.c |   40 +++++++++++++++++++++++++++-----------
> --
>  1 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos4/mct.c b/arch/arm/mach-exynos4/mct.c
> index f191608..97343df 100644
> --- a/arch/arm/mach-exynos4/mct.c
> +++ b/arch/arm/mach-exynos4/mct.c
> @@ -44,7 +44,7 @@ struct mct_clock_event_device {
>  	char name[10];
>  };
> 
> -struct mct_clock_event_device mct_tick[NR_CPUS];
> +static DEFINE_PER_CPU(struct mct_clock_event_device, percpu_mct_tick);
> 
>  static void exynos4_mct_write(unsigned int value, void *addr)
>  {
> @@ -302,7 +302,7 @@ static void exynos4_mct_tick_start(unsigned long
cycles,
>  static int exynos4_tick_set_next_event(unsigned long cycles,
>  				       struct clock_event_device *evt)
>  {
> -	struct mct_clock_event_device *mevt = &mct_tick[smp_processor_id()];
> +	struct mct_clock_event_device *mevt =
this_cpu_ptr(&percpu_mct_tick);
> 
>  	exynos4_mct_tick_start(cycles, mevt);
> 
> @@ -312,7 +312,7 @@ static int exynos4_tick_set_next_event(unsigned long
> cycles,
>  static inline void exynos4_tick_set_mode(enum clock_event_mode mode,
>  					 struct clock_event_device *evt)
>  {
> -	struct mct_clock_event_device *mevt = &mct_tick[smp_processor_id()];
> +	struct mct_clock_event_device *mevt =
this_cpu_ptr(&percpu_mct_tick);
> 
>  	exynos4_mct_tick_stop(mevt);
> 
> @@ -376,14 +376,16 @@ static struct irqaction mct_tick1_event_irq = {
> 
>  static void exynos4_mct_tick_init(struct clock_event_device *evt)
>  {
> +	struct mct_clock_event_device *mevt;
>  	unsigned int cpu = smp_processor_id();
> 
> -	mct_tick[cpu].evt = evt;
> +	mevt = this_cpu_ptr(&percpu_mct_tick);
> +	mevt->evt = evt;
> 
> -	mct_tick[cpu].base = EXYNOS4_MCT_L_BASE(cpu);
> -	sprintf(mct_tick[cpu].name, "mct_tick%d", cpu);
> +	mevt->base = EXYNOS4_MCT_L_BASE(cpu);
> +	sprintf(mevt->name, "mct_tick%d", cpu);
> 
> -	evt->name = mct_tick[cpu].name;
> +	evt->name = mevt->name;
>  	evt->cpumask = cpumask_of(cpu);
>  	evt->set_next_event = exynos4_tick_set_next_event;
>  	evt->set_mode = exynos4_tick_set_mode;
> @@ -398,21 +400,21 @@ static void exynos4_mct_tick_init(struct
> clock_event_device *evt)
> 
>  	clockevents_register_device(evt);
> 
> -	exynos4_mct_write(0x1, mct_tick[cpu].base + MCT_L_TCNTB_OFFSET);
> +	exynos4_mct_write(0x1, mevt->base + MCT_L_TCNTB_OFFSET);
> 
>  	if (mct_int_type == MCT_INT_SPI) {
>  		if (cpu == 0) {
> -			mct_tick0_event_irq.dev_id = &mct_tick[cpu];
> +			mct_tick0_event_irq.dev_id = mevt;
>  			evt->irq = IRQ_MCT_L0;
>  			setup_irq(IRQ_MCT_L0, &mct_tick0_event_irq);
>  		} else {
> -			mct_tick1_event_irq.dev_id = &mct_tick[cpu];
> +			mct_tick1_event_irq.dev_id = mevt;
>  			evt->irq = IRQ_MCT_L1;
>  			setup_irq(IRQ_MCT_L1, &mct_tick1_event_irq);
>  			irq_set_affinity(IRQ_MCT_L1, cpumask_of(1));
>  		}
>  	} else {
> -		gic_enable_ppi(IRQ_MCT_LOCALTIMER);
> +		enable_percpu_irq(IRQ_MCT_LOCALTIMER, 0);
>  	}
>  }
> 
> @@ -427,9 +429,11 @@ int __cpuinit local_timer_setup(struct
clock_event_device
> *evt)
>  void local_timer_stop(struct clock_event_device *evt)
>  {
>  	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
> -	disable_irq(evt->irq);
> +	if (mct_int_type == MCT_INT_SPI)
> +		disable_irq(evt->irq);
> +	else
> +		disable_percpu_irq(IRQ_MCT_LOCALTIMER);
>  }
> -
>  #endif /* CONFIG_LOCAL_TIMERS */
> 
>  static void __init exynos4_timer_resources(void)
> @@ -438,6 +442,16 @@ static void __init exynos4_timer_resources(void)
>  	mct_clk = clk_get(NULL, "xtal");
> 
>  	clk_rate = clk_get_rate(mct_clk);
> +
> +	if (mct_int_type == MCT_INT_PPI) {
> +		int err;
> +
> +		err = request_percpu_irq(IRQ_MCT_LOCALTIMER,
> +					 exynos4_mct_tick_isr, "MCT",
> +					 &percpu_mct_tick);
> +		WARN(err, "MCT: can't request IRQ %d (%d)\n",
> +		     IRQ_MCT_LOCALTIMER, err);
> +	}
>  }
> 
>  static void __init exynos4_timer_init(void)
> --
> 1.7.0.4

Hi Marc,

It works fine on SMDKV310 and SMDK4X12.
I will pick this up in Samsung tree and it will be sent to Arnd for this
merge window soon.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 2/2] ARM: mcx: fix local timer interrupt handling

[Sascha Hauer]

On Wed, Nov 02, 2011 at 05:30:49PM +0000, Marc Zyngier wrote:
> As local timer interrupts are now handled as normal interrupts,
> remove the special case in the GIC handler.
> 
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Ok, I'll pick this one up.

Sascha

> ---
>  arch/arm/plat-mxc/gic.c |   11 ++---------
>  1 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/gic.c b/arch/arm/plat-mxc/gic.c
> index b3b8eed..12f8f81 100644
> --- a/arch/arm/plat-mxc/gic.c
> +++ b/arch/arm/plat-mxc/gic.c
> @@ -28,21 +28,14 @@ asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
>  		if (irqnr == 1023)
>  			break;
>  
> -		if (irqnr > 29 && irqnr < 1021)
> +		if (irqnr > 15 && irqnr < 1021)
>  			handle_IRQ(irqnr, regs);
>  #ifdef CONFIG_SMP
> -		else if (irqnr < 16) {
> +		else {
>  			writel_relaxed(irqstat, gic_cpu_base_addr +
>  						GIC_CPU_EOI);
>  			handle_IPI(irqnr, regs);
>  		}
>  #endif
> -#ifdef CONFIG_LOCAL_TIMERS
> -		else if (irqnr == 29) {
> -			writel_relaxed(irqstat, gic_cpu_base_addr +
> -						GIC_CPU_EOI);
> -			handle_local_timer(regs);
> -		}
> -#endif
>  	} while (1);
>  }
> -- 
> 1.7.0.4
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/2] ARM: EXYNOS4: convert MCT to percpu interrupt API

[MyungJoo Ham]

On Thu, Nov 3, 2011 at 2:30 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> MCT recently gained per cpu interrupts, and missed the fact that
> ARM has moved to a genirq based implementation.
>
> This patch converts the driver to the new API.
>
> Boot tested on Origen.
>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

There is one concern regarding the CONFIG_LOCAL_TIMER.

[]
> ?#endif /* CONFIG_LOCAL_TIMERS */
>
> ?static void __init exynos4_timer_resources(void)
> @@ -438,6 +442,16 @@ static void __init exynos4_timer_resources(void)
> ? ? ? ?mct_clk = clk_get(NULL, "xtal");
>
> ? ? ? ?clk_rate = clk_get_rate(mct_clk);
> +
> + ? ? ? if (mct_int_type == MCT_INT_PPI) {
> + ? ? ? ? ? ? ? int err;
> +
> + ? ? ? ? ? ? ? err = request_percpu_irq(IRQ_MCT_LOCALTIMER,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?exynos4_mct_tick_isr, "MCT",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&percpu_mct_tick);
> + ? ? ? ? ? ? ? WARN(err, "MCT: can't request IRQ %d (%d)\n",
> + ? ? ? ? ? ? ? ? ? ?IRQ_MCT_LOCALTIMER, err);
> + ? ? ? }
> ?}
>

You've added exynos4_mct_tick_isr, which is defined in
CONFIG_LOCAL_TIMER section, in the place that is compiled without
CONFIG_LOCAL_TIMER.
I guess we are going to stop supporting LOCAL_TIMER in Exynos later
and this could be a problem with it.


Cheers!
MyungJoo

-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

[PATCH 1/2] ARM: EXYNOS4: convert MCT to percpu interrupt API

[Marc Zyngier]

On 10/11/11 02:40, MyungJoo Ham wrote:
> On Thu, Nov 3, 2011 at 2:30 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> MCT recently gained per cpu interrupts, and missed the fact that
>> ARM has moved to a genirq based implementation.
>>
>> This patch converts the driver to the new API.
>>
>> Boot tested on Origen.
>>
>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> There is one concern regarding the CONFIG_LOCAL_TIMER.
> 
> []
>>  #endif /* CONFIG_LOCAL_TIMERS */
>>
>>  static void __init exynos4_timer_resources(void)
>> @@ -438,6 +442,16 @@ static void __init exynos4_timer_resources(void)
>>        mct_clk = clk_get(NULL, "xtal");
>>
>>        clk_rate = clk_get_rate(mct_clk);
>> +
>> +       if (mct_int_type == MCT_INT_PPI) {
>> +               int err;
>> +
>> +               err = request_percpu_irq(IRQ_MCT_LOCALTIMER,
>> +                                        exynos4_mct_tick_isr, "MCT",
>> +                                        &percpu_mct_tick);
>> +               WARN(err, "MCT: can't request IRQ %d (%d)\n",
>> +                    IRQ_MCT_LOCALTIMER, err);
>> +       }
>>  }
>>
> 
> You've added exynos4_mct_tick_isr, which is defined in
> CONFIG_LOCAL_TIMER section, in the place that is compiled without
> CONFIG_LOCAL_TIMER.
> I guess we are going to stop supporting LOCAL_TIMER in Exynos later
> and this could be a problem with it.

Yup, this is a problem. It probably means we need to #ifdef that chink
as well. I'm not sure I get your remark about not supporting LOCAL_TIMER
though. Are you planning to move away from the LOCAL_TIMER infrastructure?

If that's the case, you might as well know that my pet plan is to get
rid of the LOCAL_TIMER option altogether, and move to a model where each
timer driver is (sort of) autonomous, instead of being driven by the SMP
code.

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 1/2] ARM: EXYNOS4: convert MCT to percpu interrupt API

[MyungJoo Ham]

On Thu, Nov 10, 2011 at 6:43 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 10/11/11 02:40, MyungJoo Ham wrote:
>> On Thu, Nov 3, 2011 at 2:30 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> MCT recently gained per cpu interrupts, and missed the fact that
>>> ARM has moved to a genirq based implementation.
>>>
>>> This patch converts the driver to the new API.
>>>
>>> Boot tested on Origen.
>>>
>>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>
>> There is one concern regarding the CONFIG_LOCAL_TIMER.
>>
>> []
>>> ?#endif /* CONFIG_LOCAL_TIMERS */
>>>
>>> ?static void __init exynos4_timer_resources(void)
>>> @@ -438,6 +442,16 @@ static void __init exynos4_timer_resources(void)
>>> ? ? ? ?mct_clk = clk_get(NULL, "xtal");
>>>
>>> ? ? ? ?clk_rate = clk_get_rate(mct_clk);
>>> +
>>> + ? ? ? if (mct_int_type == MCT_INT_PPI) {
>>> + ? ? ? ? ? ? ? int err;
>>> +
>>> + ? ? ? ? ? ? ? err = request_percpu_irq(IRQ_MCT_LOCALTIMER,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?exynos4_mct_tick_isr, "MCT",
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&percpu_mct_tick);
>>> + ? ? ? ? ? ? ? WARN(err, "MCT: can't request IRQ %d (%d)\n",
>>> + ? ? ? ? ? ? ? ? ? ?IRQ_MCT_LOCALTIMER, err);
>>> + ? ? ? }
>>> ?}
>>>
>>
>> You've added exynos4_mct_tick_isr, which is defined in
>> CONFIG_LOCAL_TIMER section, in the place that is compiled without
>> CONFIG_LOCAL_TIMER.
>> I guess we are going to stop supporting LOCAL_TIMER in Exynos later
>> and this could be a problem with it.
>
> Yup, this is a problem. It probably means we need to #ifdef that chink
> as well. I'm not sure I get your remark about not supporting LOCAL_TIMER
> though. Are you planning to move away from the LOCAL_TIMER infrastructure?

Yes, we may need to #ifdef that block.
And yes, for the Exynos series, I'll need to double check; however, I
think we are planning to move away from the LOCAL_TIMER for MCT.

>
> If that's the case, you might as well know that my pet plan is to get
> rid of the LOCAL_TIMER option altogether, and move to a model where each
> timer driver is (sort of) autonomous, instead of being driven by the SMP
> code.
>
> ? ? ? ?M.
> --
> Jazz is not dead. It just smells funny...
>
>

Cheers!
MyungJoo

-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

[PATCH 1/2] ARM: EXYNOS4: convert MCT to percpu interrupt API

[Marc Zyngier]

On 10/11/11 23:33, MyungJoo Ham wrote:
> On Thu, Nov 10, 2011 at 6:43 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 10/11/11 02:40, MyungJoo Ham wrote:
>>> On Thu, Nov 3, 2011 at 2:30 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> MCT recently gained per cpu interrupts, and missed the fact that
>>>> ARM has moved to a genirq based implementation.
>>>>
>>>> This patch converts the driver to the new API.
>>>>
>>>> Boot tested on Origen.
>>>>
>>>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>
>>> There is one concern regarding the CONFIG_LOCAL_TIMER.
>>>
>>> []
>>>>  #endif /* CONFIG_LOCAL_TIMERS */
>>>>
>>>>  static void __init exynos4_timer_resources(void)
>>>> @@ -438,6 +442,16 @@ static void __init exynos4_timer_resources(void)
>>>>        mct_clk = clk_get(NULL, "xtal");
>>>>
>>>>        clk_rate = clk_get_rate(mct_clk);
>>>> +
>>>> +       if (mct_int_type == MCT_INT_PPI) {
>>>> +               int err;
>>>> +
>>>> +               err = request_percpu_irq(IRQ_MCT_LOCALTIMER,
>>>> +                                        exynos4_mct_tick_isr, "MCT",
>>>> +                                        &percpu_mct_tick);
>>>> +               WARN(err, "MCT: can't request IRQ %d (%d)\n",
>>>> +                    IRQ_MCT_LOCALTIMER, err);
>>>> +       }
>>>>  }
>>>>
>>>
>>> You've added exynos4_mct_tick_isr, which is defined in
>>> CONFIG_LOCAL_TIMER section, in the place that is compiled without
>>> CONFIG_LOCAL_TIMER.
>>> I guess we are going to stop supporting LOCAL_TIMER in Exynos later
>>> and this could be a problem with it.
>>
>> Yup, this is a problem. It probably means we need to #ifdef that chink
>> as well. I'm not sure I get your remark about not supporting LOCAL_TIMER
>> though. Are you planning to move away from the LOCAL_TIMER infrastructure?
> 
> Yes, we may need to #ifdef that block.
> And yes, for the Exynos series, I'll need to double check; however, I
> think we are planning to move away from the LOCAL_TIMER for MCT.

I have patches for this already.

	M.
-- 
Jazz is not dead. It just smells funny...