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: 6786 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id B4CD523F8D for ; Wed, 15 Feb 2012 11:23:30 +0000 (UTC) Received: from mail-yx0-f180.google.com (mail-yx0-f180.google.com [209.85.213.180]) by fiordland.canonical.com (Postfix) with ESMTP id 7A4B6A18ACC for ; Wed, 15 Feb 2012 11:23:30 +0000 (UTC) Received: by yenr11 with SMTP id r11so666089yen.11 for ; Wed, 15 Feb 2012 03:23:30 -0800 (PST) Received: by 10.50.87.136 with SMTP id ay8mr23546129igb.25.1329305008008; Wed, 15 Feb 2012 03:23:28 -0800 (PST) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.66.135 with SMTP id n7cs8454ibi; Wed, 15 Feb 2012 03:23:25 -0800 (PST) Received: by 10.213.15.81 with SMTP id j17mr183490eba.48.1329305004133; Wed, 15 Feb 2012 03:23:24 -0800 (PST) Received: from mx2.suse.de (cantor2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id s41si1878624ees.110.2012.02.15.03.23.23 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 15 Feb 2012 03:23:24 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of afaerber@suse.de designates 195.135.220.15 as permitted sender) client-ip=195.135.220.15; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of afaerber@suse.de designates 195.135.220.15 as permitted sender) smtp.mail=afaerber@suse.de 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 Cc: qemu-devel@nongnu.org, patches@linaro.org, daniel.forsgren@enea.com Subject: Re: [Qemu-devel] [PATCH] hw/pl031: Actually raise interrupt on timer expiry 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-Gm-Message-State: ALoCoQlhHE7tcOMukYAIPcs19kRDJ4x1OhhsNF2vX11ff1RF7kxgYK33ef2OObUmpt8vySvw+Tfz 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;