From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-x22d.google.com (mail-wr0-x22d.google.com [IPv6:2a00:1450:400c:c0c::22d]) (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 C944A2095B9E1 for ; Thu, 17 Aug 2017 08:52:39 -0700 (PDT) Received: by mail-wr0-x22d.google.com with SMTP id z91so38779641wrc.4 for ; Thu, 17 Aug 2017 08:55:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=+sBqsxN09kj5NXHorbEE/B6eFBcTdJMwZvfNEzFe+XQ=; b=Rs15SkgizlJr/XAzwtG+5sjyuBQfTYvB07zJmKhedm9KR6cPu/WRafMmPd52Wvu/Lj SN+5HwDiskasRr9I7jv8rRUEpsIx0HW7Nqffzo41Uk7dIsG9Alzya5VDtRm2z/2h7fjP 1Suax9TxiUjPYUVKtlmfEyGavgayyHXwGYbAU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=+sBqsxN09kj5NXHorbEE/B6eFBcTdJMwZvfNEzFe+XQ=; b=e3frxSSS+V1wypAQv4hxAN5L63/jO7PqlKGZOFXAFJ5CglgcbhDKpXlXJyiFG2yFSG TonuDN23XcZPjsqUFnOjLFPJPS+Xq7Bg7OcwxFp4HK3N36gE21HWGd+lgwV4qCeva2Ry RDmnvvSB5CgMpqsbgfzDgWZWYbzCZQ+BIETEsHGhz0+Op5bbNucMV9Lc996GF4A1ae3i X04ktQPgRGWcC1l4Abwc3/yYyb1a3pICzHgDPQGHVzRHrWX6ejhr0Q7kWTKor6tJpI1A FzK8iTCFrBArTepmYL+KgOch4qwylBFFGF/wRlDazM5/C6orWLqVxjmXe7mKW/PlCV+1 zILA== X-Gm-Message-State: AHYfb5gk84glIUT7IKjuNpw5cBUb2DzjU/Ey1i6drd0G7p9tlt6a00mA tQoHhs8a1o/ufXNG X-Received: by 10.28.57.197 with SMTP id g188mr1758966wma.33.1502985305625; Thu, 17 Aug 2017 08:55:05 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id p17sm4118554wma.45.2017.08.17.08.55.04 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 17 Aug 2017 08:55:05 -0700 (PDT) Date: Thu, 17 Aug 2017 16:55:02 +0100 From: Leif Lindholm To: Jun Nie Cc: haojian.zhuang@linaro.org, ard.biesheuvel@linaro.org, linaro-uefi@lists.linaro.org, shawn.guo@linaro.org, jason.liu@linaro.org, edk2-devel@lists.01.org Message-ID: <20170817155502.5rjnteaye6sq6m4y@bivouac.eciton.net> References: <1502287959-16806-1-git-send-email-jun.nie@linaro.org> <1502287959-16806-2-git-send-email-jun.nie@linaro.org> <20170810140314.jqqtcspfizqhhntf@bivouac.eciton.net> <09ad9459-9cbe-ea49-53f0-e7ce1ba74302@linaro.org> MIME-Version: 1.0 In-Reply-To: <09ad9459-9cbe-ea49-53f0-e7ce1ba74302@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH 2/4] Platforms: Add ZX RTC driver for Sanchip SoC X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Aug 2017 15:52:40 -0000 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Thu, Aug 17, 2017 at 11:43:37PM +0800, Jun Nie wrote: > On 2017年08月10日 22:03, Leif Lindholm wrote: > > On Wed, Aug 09, 2017 at 10:12:37PM +0800, Jun Nie wrote: > > > Runtime service is not supported yet. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Jun Nie > > > --- > > > .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.c | 376 +++++++++++++++++++++ > > > .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.h | 102 ++++++ > > > .../Zx296718RealTimeClock.inf | 42 +++ > > > 3 files changed, 520 insertions(+) > > > create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c > > > create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h > > > create mode 100644 Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf > > > > > > diff --git a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c > > > new file mode 100644 > > > index 0000000..af6e5bd > > > --- /dev/null > > > +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c > > > @@ -0,0 +1,376 @@ > > > +/** @file > > > + Implement EFI RealTimeClock runtime services via RTC Lib. > > > + > > > + Currently this driver does not support runtime virtual calling. > > > + > > > + Copyright (C) 2017 Sanechips Technology Co., Ltd. > > > + Copyright (c) 2017, Linaro Limited. > > > + > > > + This program and the accompanying materials > > > + are licensed and made available under the terms and conditions of the BSD License > > > + which accompanies this distribution. The full text of the license may be found at > > > + http://opensource.org/licenses/bsd-license.php > > > + > > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > > + > > > + Based on the files under ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf > > > + > > > +**/ > > > + > > > +#include > > > +#include > > > > P before U. > > > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +// Use EfiAtRuntime to check stage > > > +#include > > > > L (UefiRuntimeLib) before S (UefiRuntimeServices...). > > No need for a comment explaining why we include headers. > > > > > +#include > > > +#include "Zx296718RealTimeClock.h" > > > + > > > +STATIC UINTN RtcBase; > > > > mRtcBase. > > > > > +STATIC BOOLEAN RTCInitialized = FALSE; > > > > mRTCInitialized. > > > > > + > > > +BOOLEAN > > > +EFIAPI > > > +IsTimeValid( > > > + IN EFI_TIME *Time > > > + ) > > > +{ > > > + // Check the input parameters are within the range specified by UEFI > > > + if ((Time->Year < 2000) || > > > + (Time->Year > 2099) || > > > + (Time->Month < 1 ) || > > > + (Time->Month > 12 ) || > > > + (Time->Day < 1 ) || > > > + (Time->Day > 31 ) || > > > + (Time->Hour > 23 ) || > > > + (Time->Minute > 59 ) || > > > + (Time->Second > 59 ) || > > > + (Time->Nanosecond > 999999999) || > > > + (!((Time->TimeZone == EFI_UNSPECIFIED_TIMEZONE) || ((Time->TimeZone >= -1440) && (Time->TimeZone <= 1440)))) || > > > + (Time->Daylight & (~(EFI_TIME_ADJUST_DAYLIGHT | EFI_TIME_IN_DAYLIGHT))) > > > + ) { > > > + return FALSE; > > > + } > > > + > > > + return TRUE; > > > +} > > > > This appears a direct copy of the version in > > EmbeddedPkg/Library/TimeBaseLib. Can you add that TimeBaseLib library > > dependency to decrease duplication? > > (This may not have existed as a standalone library at the time you > > started this port.) > > > > > + > > > +VOID > > > > A lot of the functions in this file could do with a STATIC prefix, > > since they are only used internally. > > > > > +Wait4Busy ( > > > > Semantically, this name is incorrect. > > The function is waiting _while_ the state is RTC_BUSY, so seems to me > > it should be called WaitBusy. > > > > > + VOID > > > + ) > > > +{ > > > + UINT32 Val, Retry = 1000; > > > + do { > > > + MicroSecondDelay (200); > > > > Why 200? > > It is just a suggested delay time according to RTC clock frequency. You want > a RTC_WAIT_DELAY macro here for better understanding, right? That would be more clear, yes. It is also good if next to that definition there is some description of how the value was detemined. (I.e., are we waiting for a particular number of cycles, or is this simply a determined to be "good enough".) / Leif