From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 11 Jul 2019 10:06:14 -0700 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 51DE33083394; Thu, 11 Jul 2019 17:06:13 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-231.ams2.redhat.com [10.36.117.231]) by smtp.corp.redhat.com (Postfix) with ESMTP id 48A355D9CA; Thu, 11 Jul 2019 17:06:12 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] PL031: Actually disable interrupts To: devel@edk2.groups.io, leif.lindholm@linaro.org, graf@amazon.com References: <20190710145311.12184-1-graf@amazon.com> <20190710171331.aj2smigjmtok4buz@bivouac.eciton.net> From: "Laszlo Ersek" Message-ID: <7df479df-21b9-b90d-3830-4035756289b6@redhat.com> Date: Thu, 11 Jul 2019 19:06:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190710171331.aj2smigjmtok4buz@bivouac.eciton.net> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.44]); Thu, 11 Jul 2019 17:06:13 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 07/10/19 19:13, Leif Lindholm wrote: > On Wed, Jul 10, 2019 at 04:53:11PM +0200, Alexander Graf via Groups.Io wrote: >> The PL031 interrupt mask register (IMSC) is not very clearly documented >> in the PL031 specification. However, bit 0 (RTCIMSC) indicates whether >> interrupts are enabled, not disabled. > > 3.3.5. Interrupt Mask Set or Clear register, RTCIMSC > ... Writing 1 sets the mask. ... > > 3.6. Interrupts > ... This interrupt is enabled or disabled by changing the mask bit in > RTCIMSC. To enable the interrupt, set bit[0] HIGH. ... > > *boggle* Heh :) Alex, out of interest, what were the symptoms of this issue on your end? Spurious interrupt confusing the firmware's exception handler, perhaps? Thanks! Laszlo > >> So before this commit, we were actually *enabling* interrupts for the RTC. >> >> This patch changes the logic to instead disable interrupts when they >> are not disabled already. > > Thanks for finding/fixing this. > >> Signed-off-by: Alexander Graf > > Reviewed-by: Leif Lindholm > > I took the liberty to change the subject line to > "ArmPlatformPkg: Actually disable PL031 interrupts" > (We tend to start with the top-level directory.) > > Pushed as 8df52631e53c. > > / > Leif > >> --- >> .../Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c >> index b630a5cfbf..75c95985d4 100644 >> --- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c >> +++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c >> @@ -80,8 +80,8 @@ InitializePL031 ( >> } >> >> // Ensure interrupts are masked. We do not want RTC interrupts in UEFI >> - if ((MmioRead32 (mPL031RtcBase + PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER) & PL031_SET_IRQ_MASK) != PL031_SET_IRQ_MASK) { >> - MmioOr32 (mPL031RtcBase + PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER, PL031_SET_IRQ_MASK); >> + if ((MmioRead32 (mPL031RtcBase + PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER) & PL031_SET_IRQ_MASK) != 0) { >> + MmioWrite32 (mPL031RtcBase + PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER, 0); >> } >> >> // Clear any existing interrupts >> -- >> 2.17.1 >> >> >> >> > > >