From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::244; helo=mail-it0-x244.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-it0-x244.google.com (mail-it0-x244.google.com [IPv6:2607:f8b0:4001:c0b::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id AC8E0210C5138 for ; Fri, 3 Aug 2018 02:41:30 -0700 (PDT) Received: by mail-it0-x244.google.com with SMTP id s7-v6so7565884itb.4 for ; Fri, 03 Aug 2018 02:41:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=UhN4yD8MWeV3DYc2eT9skOCVHiJxqqx2NtWzVBCUG1Y=; b=bvyuci55bsv37hKExIiU3Mx5MHKWDCFfKpqEvaWHCl86O1JwiB2LYtHdx6U5XfAmi6 MnCnw3qsB4j6aCkz1SLBm3+Tj9pdtfbUITgoNMgT0cH2Qy5JJ/5JWtfcxZuYA/gjpu4s r4wKz8cSMGsUplfoAcWzzOvz+qgygnuN9NA+mSVxCmskf/9dXPn+0wS13VuyVg3PuvnZ D7+gCk/VR5uhjlkD4PUUbcihh6MLEauzob6vixxr34+hVawO8ySCAht75iza3SJL+nT8 6c4AJvti2WmS8kHn+viI6SFmx9g4WMJFkgWfWEForbnQc5xBtVrzrhdFVlHUoZXkbYu7 EYJA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=UhN4yD8MWeV3DYc2eT9skOCVHiJxqqx2NtWzVBCUG1Y=; b=mBYl3ho3GHH5eL9ukh9X1B56YvVMw3+8NryiGJYoW9McAkoiwVsGrfZgqdN4n66AeH dwwl2t5HYEtktPkhLZO/0b0KOu7U6e6Pb6QPGXSXyshv8Oqu5PJITZcbP5JBlLvMuM9D uqz6dZgPy+4/eM5aMn0HFxBbbrw8Iz53j/KsEdlui1Vr+iKdmpEBx1qiaiJSfVerYMaR 2JapugeELF7Zf6pwLhyZEnw7oIqmmdd0KzDMejBneW93eO1Fx4X1eNjlPEAIihtMEKgh OtnfoTqraIG+Glc8x2nh4A5OdoQkmBwBJKg4nftuTQAIfsQowCTdSwGPhukHbEe5+wkP fccw== X-Gm-Message-State: AOUpUlHUKgToQR5UAxFu8VLfwRyirFP7YDWqVoZK0UhGAi2Azeplo2Ry l59l2IaE7eiFjjIhCX6K5+eW91gGK7/l7I/lREzxWA== X-Google-Smtp-Source: AAOMgpf6kAw+r077k3i64lKIJnz6CCuDdDxCsC3sQVkScADaXFC5jbB3bHozqjyClimQwlnSjUK4lc+0T9VPF9kPzlQ= X-Received: by 2002:a24:8282:: with SMTP id t124-v6mr5269655itd.57.1533289289605; Fri, 03 Aug 2018 02:41:29 -0700 (PDT) MIME-Version: 1.0 References: <1533243054-14251-1-git-send-email-mw@semihalf.com> In-Reply-To: From: Marcin Wojtas Date: Fri, 3 Aug 2018 11:41:17 +0200 Message-ID: To: Ard Biesheuvel Cc: edk2-devel-01 , "Tian, Feng" , "Kinney, Michael D" , "Gao, Liming" , Leif Lindholm , nadavh@marvell.com, "jsd@semihalf.com" Subject: Re: [PATCH v2 1/1] ArmPkg/GenericWatchdogDxe: Split 64bit register write to 2x32bit X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Aug 2018 09:41:30 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable pt., 3 sie 2018 o 09:33 Ard Biesheuvel napisa= =C5=82(a): > > On 2 August 2018 at 22:50, Marcin Wojtas wrote: > > According to the SBSA specification the Watchdog Compare > > Register is split into two separate 32bit registers. > > EDK2 code uses a single 64bit transaction to update > > them, which can be problematic, depending on the SoC > > implementation and could result in an unpredicted behavior. > > > > Fix this by modifying WatchdogWriteCompareRegister routine to > > use two consecutive 32bit writes to the Watchdog Compare Register > > Low and High, using new dedicated macros. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > Reviewed-by: Leif Lindholm > > Pushed as dd4cae4d82c7 > Thanks a lot! Marcin > > > --- > > Changelog v1 -> v2: > > - use separate macros for WCV register low and high > > - improve commit message > > - add Leif's RB > > > > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h | 3 ++- > > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 3 ++- > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h b/ArmP= kg/Drivers/GenericWatchdogDxe/GenericWatchdog.h > > index 9e2aebc..4f42c56 100644 > > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h > > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h > > @@ -20,7 +20,8 @@ > > // Control Frame: > > #define GENERIC_WDOG_CONTROL_STATUS_REG ((UINTN)FixedPcdGet64 (P= cdGenericWatchdogControlBase) + 0x000) > > #define GENERIC_WDOG_OFFSET_REG ((UINTN)FixedPcdGet64 (P= cdGenericWatchdogControlBase) + 0x008) > > -#define GENERIC_WDOG_COMPARE_VALUE_REG ((UINTN)FixedPcdGet64 (P= cdGenericWatchdogControlBase) + 0x010) > > +#define GENERIC_WDOG_COMPARE_VALUE_REG_LOW ((UINTN)FixedPcdGet64 (P= cdGenericWatchdogControlBase) + 0x010) > > +#define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH ((UINTN)FixedPcdGet64 (P= cdGenericWatchdogControlBase) + 0x014) > > > > // Values of bit 0 of the Control/Status Register > > #define GENERIC_WDOG_ENABLED 1 > > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/A= rmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > > index 3180f01..8ccf153 100644 > > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > > @@ -56,7 +56,8 @@ WatchdogWriteCompareRegister ( > > UINT64 Value > > ) > > { > > - MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value); > > + MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_LOW, Value & MAX_UINT32)= ; > > + MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & MA= X_UINT32); > > } > > > > VOID > > -- > > 2.7.4 > >