From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::244; helo=mail-it0-x244.google.com; envelope-from=ard.biesheuvel@linaro.org; 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 EFF48210D7F38 for ; Thu, 2 Aug 2018 11:32:19 -0700 (PDT) Received: by mail-it0-x244.google.com with SMTP id 72-v6so4933261itw.3 for ; Thu, 02 Aug 2018 11:32:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=EMfIPzXhEp5XUt+J2KMc1b3KPoPdQpb5/QZXWu2FOqw=; b=aMwSfBvBT66WJgDG6BQHtHLxIWhyee3h7r/C7mG+cgq4rbUCCe6d/X+s8+9kqqJu2/ IiWd3kBXSD+wy6mWd7EZ5JBJpIAI7J7Gbj0S/xq5mmcoCd/IFVRoICn1FGi0QbohlKwc LZr+is/6mr5d/POYnYecufOuFyKDGj1FNvPOk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=EMfIPzXhEp5XUt+J2KMc1b3KPoPdQpb5/QZXWu2FOqw=; b=eS9ywObBnQi7jcyNle9F0ZeBDpMPSkd7NHJGRsf8uM0eFrfQWSaF4aKtSIftSenw9g gbAwilaj3tM/BMb8RNV5vLgUuYJSCxwf8E9ESb03B9l1V/hulMbAk7KylKuYBJP9+0D5 CUN5L4CMvP7phLQfjJFveJbJ/i0kSIkSdBKuw+etnd7v1F87VnwzkWCbv1R3RA7ReWMp QApq8m5XJ96upe/DoIgbssfMO8pkkEdzydeL9BGQOKc70QK2yMRPzvxr+AsSGDQUBQkn tzTD7UkfhlYwW27Gf/goEV8NC7MkEvVz8myIbgZ2o0QNsb3f2HChESZCyDpLsLpEItj0 bgTg== X-Gm-Message-State: AOUpUlHABJaoFnQlA8HhkTU+O+CUWRMwDTn84CuMO7BWACcFnuTqdMxD vq7+TsaDCzselrXkbMgqb0wNZyC4i0L2CmKBkKZWSQ== X-Google-Smtp-Source: AAOMgpeJ5lTyQpQUEWO5COfGBlY7aRouQaRePuCmmXKf8dpdNIQ0u9axIX4Gi1Z8AX1MbRi5oedJyMJpNK1QsKs827E= X-Received: by 2002:a24:610d:: with SMTP id s13-v6mr3775717itc.68.1533234738866; Thu, 02 Aug 2018 11:32:18 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:ac05:0:0:0:0:0 with HTTP; Thu, 2 Aug 2018 11:32:18 -0700 (PDT) In-Reply-To: References: <1533221396-11620-1-git-send-email-mw@semihalf.com> From: Ard Biesheuvel Date: Thu, 2 Aug 2018 20:32:18 +0200 Message-ID: To: Marcin Wojtas Cc: edk2-devel-01 , "Tian, Feng" , "Kinney, Michael D" , "Gao, Liming" , Leif Lindholm , Nadav Haklai , "jsd@semihalf.com" Subject: Re: [PATCH] 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: Thu, 02 Aug 2018 18:32:20 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On 2 August 2018 at 17:12, Marcin Wojtas wrote: > czw., 2 sie 2018 o 16:59 Ard Biesheuvel napis= a=C5=82(a): >> >> On 2 August 2018 at 16:49, Marcin Wojtas wrote: >> > EDK2 code uses a single 64bit write to update SBSA watchdog >> > compare registers, however an access to mmio registers should >> > be 32bit for some SoCs. Current usage of MmioWrite64 resulted >> > in an unpredicted behavior. Fix this by modifying >> > WatchdogWriteCompareRegister routine to use two consecutive >> > 32bit writes to Watchdog Compare Register Low and High. >> > >> >> You describe it as if this is generally the case, but this is just a >> silicon bug, right? > > Not sure if it's a bug, or simply SoC characterisctics to place SoC > registers to allow only mmio32 access to 32-bit registers. In any way, > even updated routine should be fine also for the ones capable of > mmio64 registers access. Do you have strong objections to the change? > To be fair, the SBSA v5.0 does describe this register as 0x010-0x013 WCV[31:0] Watchdog compare value. Read/write registers 0x014-0x017 WCV[63:32] containing the current value in the watchdog compare register. so I guess it is implied that any implementation should tolerate this value being written as 2 32-bit quantities. Leif? > >> >> > Contributed-under: TianoCore Contribution Agreement 1.1 >> > Signed-off-by: Marcin Wojtas >> > --- >> > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 6 +++++- >> > 1 file changed, 5 insertions(+), 1 deletion(-) >> > >> > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/= ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c >> > index 3180f01..b25d210 100644 >> > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c >> > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c >> > @@ -56,7 +56,11 @@ WatchdogWriteCompareRegister ( >> > UINT64 Value >> > ) >> > { >> > - MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value); >> > + MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG, Value & MAX_UINT32); >> > + MmioWrite32 ( >> > + GENERIC_WDOG_COMPARE_VALUE_REG + sizeof (UINT32), >> > + (Value >> 32) & MAX_UINT32 >> > + ); >> > } >> > >> > VOID >> > -- >> > 2.7.4 >> >