From patchwork Wed Feb 15 11:23:22 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Andreas_F=C3=A4rber?= X-Patchwork-Id: 141312 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id E16C41007D6 for ; Wed, 15 Feb 2012 22:23:45 +1100 (EST) Received: from localhost ([::1]:41249 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rxcxb-0006ub-Qs for incoming@patchwork.ozlabs.org; Wed, 15 Feb 2012 06:23:43 -0500 Received: from eggs.gnu.org ([140.186.70.92]:41532) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RxcxS-0006tu-C2 for qemu-devel@nongnu.org; Wed, 15 Feb 2012 06:23:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RxcxM-0002xq-CA for qemu-devel@nongnu.org; Wed, 15 Feb 2012 06:23:34 -0500 Received: from cantor2.suse.de ([195.135.220.15]:46085 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RxcxM-0002xL-32 for qemu-devel@nongnu.org; Wed, 15 Feb 2012 06:23:28 -0500 Received: from relay2.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 5EBCC8FD0F; Wed, 15 Feb 2012 12:23:23 +0100 (CET) Message-ID: <4F3B95AA.8090007@suse.de> Date: Wed, 15 Feb 2012 12:23:22 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= Organization: SUSE LINUX Products GmbH User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20120129 Thunderbird/10.0 MIME-Version: 1.0 To: Peter Maydell References: <1329241239-9327-1-git-send-email-peter.maydell@linaro.org> In-Reply-To: <1329241239-9327-1-git-send-email-peter.maydell@linaro.org> X-Enigmail-Version: 1.3.5 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4-2.6 X-Received-From: 195.135.220.15 Cc: daniel.forsgren@enea.com, qemu-devel@nongnu.org, patches@linaro.org Subject: Re: [Qemu-devel] [PATCH] hw/pl031: Actually raise interrupt on timer expiry X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Am 14.02.2012 18:40, schrieb Peter Maydell: > Fix a typo in pl031_interrupt() which meant we were setting a bit > in the interrupt mask rather than the interrupt status register > and thus not actually raising an interrupt. This fix allows the > rtctest program from the kernel's Documentation/rtc.txt to pass > rather than hanging. > Reported-by: Daniel Forsgren > Signed-off-by: Peter Maydell > --- > Looks like our PL031 has always had this bug since it was added > in 2007... Daniel Forsgren reported this, suggested the fix and > pointed me at the test program. Thanks! Down here the credit for the find gets lost. > https://bugs.launchpad.net/qemu-linaro/+bug/931940 > > hw/pl031.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/pl031.c b/hw/pl031.c > index 8416a60..f06b5ae 100644 > --- a/hw/pl031.c > +++ b/hw/pl031.c > @@ -76,7 +76,7 @@ static void pl031_interrupt(void * opaque) > { > pl031_state *s = (pl031_state *)opaque; > > - s->im = 1; > + s->is = 1; > DPRINTF("Alarm raised\n"); > pl031_update(s); > } So on RTC_ICR write s->is = 0; but it was never set elsewhere, so RTC_RIS would always return 0. Acked-by: Andreas Färber However, to facilitate future review of these non-telling fields I propose the following documentation patch as a follow-up: Andreas diff --git a/hw/pl031.c b/hw/pl031.c index 8416a60..a20c625 100644 --- a/hw/pl031.c +++ b/hw/pl031.c @@ -32,6 +32,11 @@ do { printf("pl031: " fmt , ## __VA_ARGS__); } while (0) #define RTC_MIS 0x18 /* Masked interrupt status register */ #define RTC_ICR 0x1c /* Interrupt clear register */ +/** + * pl031_state: + * @im: Interrupt mask. + * @is: Interrupt state. + */ typedef struct { SysBusDevice busdev; MemoryRegion iomem;