Re: [edk2-devel] [PATCH] PL031: Actually disable interrupts

["Leif Lindholm" <leif.lindholm@linaro.org>]

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*

> 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 <graf@amazon.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

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
> 
> 
> 
>