Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1757221AbcC2OVL (ORCPT <rfc822;w@1wt.eu>);
	Tue, 29 Mar 2016 10:21:11 -0400
Received: from mail-wm0-f47.google.com ([74.125.82.47]:35368 "EHLO
	mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1757166AbcC2OVH (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Tue, 29 Mar 2016 10:21:07 -0400
Subject: Re: [PATCH] clockevents/drivers/arm_global_timer: Fix erratum 740657
 workaround
To: Rabin Vincent <rabin.vincent@axis.com>, tglx@linutronix.de
References: <1459238917-2240-1-git-send-email-rabin.vincent@axis.com>
Cc: srinivas.kandagatla@gmail.com, maxime.coquelin@st.com,
        patrice.chotard@st.com, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, Rabin Vincent <rabinv@axis.com>
From: Daniel Lezcano <daniel.lezcano@linaro.org>
Message-ID: <56FA8F4F.1000900@linaro.org>
Date: Tue, 29 Mar 2016 16:21:03 +0200
User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101
 Thunderbird/38.5.1
MIME-Version: 1.0
In-Reply-To: <1459238917-2240-1-git-send-email-rabin.vincent@axis.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 3812
Lines: 94

On 03/29/2016 10:08 AM, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
>
> According to the errata document for the Cortex A9 MPCore, the correct
> code sequence in the interrupt handler to workaround erratum 740657
> "Global Timer can send two interrupts for the same event" is:
>
>   (1) Read the ICCIAR (Interrupt Acknowledge) register
>   (2) Modify the comparator value, to set it to a higher value
>   (3) Clear the Global Timer flag
>   (4) Clear the Pending Status information for Interrupt 27 (Global
>       Timer interrupt) in the Distributor of the Interrupt Controller.
>   (5) Write the ICCEOIR (End of Interrupt) register
>
> (1) and (5) are done by the GIC driver and (2) and (3) are done by the
> Global Timer driver.  However, nobody does (4) and thus the workaround
> is inactive and the timer triggers many spurious interrupts:
>
>   <idle>-0 [001] d.h2 99.850527: irq_handler_entry: irq=16 name=gt
>   <idle>-0 [001] d.h2 99.850538: irq_handler_exit: irq=16 ret=handled
>   <idle>-0 [001] d.H2 99.850540: irq_handler_entry: irq=16 name=gt
>   <idle>-0 [001] d.H2 99.850542: irq_handler_exit: irq=16 ret=unhandled
>   <idle>-0 [001] d.h2 99.987832: irq_handler_entry: irq=16 name=gt
>   <idle>-0 [001] dnh2 99.987845: irq_handler_exit: irq=16 ret=handled
>   <idle>-0 [001] dnh2 99.987848: irq_handler_entry: irq=16 name=gt
>   <idle>-0 [001] dnh2 99.987850: irq_handler_exit: irq=16 ret=unhandled

Hi Rabin,

what platform are you using to test that ?


> Make the Global Timer driver perform step (4) via the GIC driver with
> the help of the irq_set_irqchip_state() function, to prevent the
> spurious interrupts.
>
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>   drivers/clocksource/arm_global_timer.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
> index 9df0d16..b9d0f86 100644
> --- a/drivers/clocksource/arm_global_timer.c
> +++ b/drivers/clocksource/arm_global_timer.c
> @@ -140,26 +140,31 @@ static int gt_clockevent_set_next_event(unsigned long evt,
>   static irqreturn_t gt_clockevent_interrupt(int irq, void *dev_id)
>   {
>   	struct clock_event_device *evt = dev_id;
> +	bool workaround = clockevent_state_oneshot(evt);
>
>   	if (!(readl_relaxed(gt_base + GT_INT_STATUS) &
>   				GT_INT_STATUS_EVENT_FLAG))
>   		return IRQ_NONE;
>
>   	/**
> -	 * ERRATA 740657( Global Timer can send 2 interrupts for
> +	 * ERRATA 740657 (Global Timer can send 2 interrupts for
>   	 * the same event in single-shot mode)
>   	 * Workaround:
> -	 *	Either disable single-shot mode.
> -	 *	Or
> -	 *	Modify the Interrupt Handler to avoid the
> -	 *	offending sequence. This is achieved by clearing
> -	 *	the Global Timer flag _after_ having incremented
> -	 *	the Comparator register	value to a higher value.
> +	 * - Read the ICCIAR (Interrupt Acknowledge) register
> +	 * - Modify the comparator value, to set it to a higher value
> +	 * - Clear the Global Timer flag
> +	 * - Clear the Pending Status information for Interrupt 27 (Global
> +	 *   Timer interrupt) in the Distributor of the Interrupt Controller.
> +	 * - Write the ICCEOIR (End of Interrupt) register
>   	 */
> -	if (clockevent_state_oneshot(evt))
> +	if (workaround)
>   		gt_compare_set(ULONG_MAX, 0);
>
>   	writel_relaxed(GT_INT_STATUS_EVENT_FLAG, gt_base + GT_INT_STATUS);
> +
> +	if (workaround)
> +		irq_set_irqchip_state(irq, IRQCHIP_STATE_PENDING, false);
> +
>   	evt->event_handler(evt);
>
>   	return IRQ_HANDLED;
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog