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::242; helo=mail-it0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x242.google.com (mail-it0-x242.google.com [IPv6:2607:f8b0:4001:c0b::242]) (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 BEF372119925D for ; Tue, 19 Jun 2018 23:21:13 -0700 (PDT) Received: by mail-it0-x242.google.com with SMTP id j135-v6so3958762itj.1 for ; Tue, 19 Jun 2018 23:21:13 -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; bh=UhZqV1Anxuvh/65qk6UyCbnks6JiZNwJFb+jKdq3yXE=; b=QL2Ginjk141KuxOWKgVZj+JNfmQifER9HSEV+tpTdmVxvssbUd9AfTHgexm8Ht1mmE LMHADqmX4qwRZyk04XWJGh4bxCZ2fe6mBkGCrotIcIcjeClQ+xut+7Rr3c2aE1gDsagB iYUGlO2YVMMzn12GnaMK8k5lh4sFOiFy8YZKY= 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; bh=UhZqV1Anxuvh/65qk6UyCbnks6JiZNwJFb+jKdq3yXE=; b=Az3Zl+E4sf4tv62eLE2v3d3/bZqBCwa+rFNQ0lBM7IAHEeWW7ydHndsCyd59y/wmXC Wm7ZGlcQ0nEoz8VX81y5kOx16W7IcUPW+3EZNfXnfU5YQkumMOa1KzU13Twl7qRQB/jR 3gzchFTEY0c8cNwe4NJxQBRjpwcla15Yy4v69+WIi/aPnK5bLGyzXZeap2owMFT0A2/H i8UJxmDnDdTMLYMiw6uvkguUrRRDTQeKLWjiv4sWBsEZp0S1tMF+6rQsphpFwYye8E4n wYvqU8ds7FLfTIY416gLsgs9QrKdu6PFKBflEx9wdwrNeM0zrHTfN4WHHfr0EcWLJnQy y9CA== X-Gm-Message-State: APt69E1Dwd3o5hp3ZNup4Nou26Iip2ay+/9V8bw2RSA8SEzwXKOqHfv2 +nPUj6dqgu1Umey7YehUBzSTNW/B9ZikRWMCY/mtbw== X-Google-Smtp-Source: ADUXVKI63g6Jac5Po54CdiknQm4bjOdYuZckktd561awpt0W8nEOPKTb2qWt87eC8mAqN6F5qSG8UY6ekNXJs8Jffbo= X-Received: by 2002:a24:e105:: with SMTP id n5-v6mr550900ith.68.1529475672745; Tue, 19 Jun 2018 23:21:12 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Tue, 19 Jun 2018 23:21:12 -0700 (PDT) In-Reply-To: <20180620030240.2409-1-masahisa.kojima@linaro.org> References: <20180620030240.2409-1-masahisa.kojima@linaro.org> From: Ard Biesheuvel Date: Wed, 20 Jun 2018 08:21:12 +0200 Message-ID: To: Masahisa Kojima Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Yoshitoyo Osaki Subject: Re: [PATCH v2] Silicon/NXP/Pcf8563RealTimeClockLib: add Voltage-low handling X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Jun 2018 06:21:14 -0000 Content-Type: text/plain; charset="UTF-8" On 20 June 2018 at 05:02, wrote: > From: Masahisa Kojima > > BcdToDecimal8() in LibGetTime() asserts with the > following condition. > 1) RTC device has not been initialized yet, RTC device > returns indeterminate value with VL bit(1) > 2) DEBUG build > > UEFI boot fails with assertion when it satisfies above conditions. > > Aside form boot failure, UEFI shell commands "date/time" expect > that getting time from RTC should success when user sets the time. > ShellCommandRunTime() performs GetTime()->update time->SetTime(), > if the first GetTime() fails, user can not set time. > With that, simply returning EFI_DEVICE_ERROR in LibGetTime() > is not applicable to VL bit handling. > > To avoid this situation, even if it only occurs in DEBUG build, > RTC driver should check the VL bit in the VL_seconds register. > This VL bit is voltage-low detector, it means integrity of the > clock information is not guaranteed if it sets to 1. In this > case, driver return dummy date/time(01/01/2000 00:00:00) to > proceed succeeding process. > > linux driver also checks this bit when driver gets the time > from RTC. If VL bit is 1, linux driver discard the retreived > time data. > > Note that if VL bit is 1, GetTime() returns always > 01/01/2000 00:00:00 until user sets date/time. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Masahisa Kojima > Signed-off-by: Yoshitoyo Osaki > --- > .../Pcf8563RealTimeClockLib.c | 31 +++++++++++++++------- > 1 file changed, 22 insertions(+), 9 deletions(-) > > diff --git a/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c b/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c > index fb58e1feb4..9f49ae91de 100644 > --- a/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c > +++ b/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c > @@ -19,11 +19,13 @@ > #include > #include > #include > +#include > #include > > #define SLAVE_ADDRESS (FixedPcdGet8 (PcdI2cSlaveAddress)) > #define PCF8563_DATA_REG_OFFSET 0x2 > > +#define PCF8563_CLOCK_INVALID 0x80 > #define PCF8563_SECONDS_MASK 0x7f > #define PCF8563_MINUTES_MASK 0x7f > #define PCF8563_HOURS_MASK 0x3f > @@ -95,6 +97,7 @@ LibGetTime ( > RTC_DATETIME DateTime; > EFI_STATUS Status; > UINT8 Reg; > + EFI_TIME InitTime; > > if (Time == NULL) { > return EFI_INVALID_PARAMETER; > @@ -122,15 +125,25 @@ LibGetTime ( > return EFI_DEVICE_ERROR; > } > > - Time->Second = BcdToDecimal8 (DateTime.VL_seconds & PCF8563_SECONDS_MASK); > - Time->Minute = BcdToDecimal8 (DateTime.Minutes & PCF8563_MINUTES_MASK); > - Time->Hour = BcdToDecimal8 (DateTime.Hours & PCF8563_HOURS_MASK); > - Time->Day = BcdToDecimal8 (DateTime.Days & PCF8563_DAYS_MASK); > - Time->Month = BcdToDecimal8 (DateTime.Century_months & PCF8563_MONTHS_MASK); > - Time->Year = BcdToDecimal8 (DateTime.Years) + EPOCH_BASE; > - > - if (DateTime.Century_months & PCF8563_CENTURY_MASK) { > - Time->Year += 100; > + if ((DateTime.VL_seconds & PCF8563_CLOCK_INVALID) != 0) { > + InitTime.Second = 0; > + InitTime.Minute = 0; > + InitTime.Hour = 0; > + InitTime.Day = 1; > + InitTime.Month = 1; > + InitTime.Year = EPOCH_BASE; > + (VOID)CopyMem (Time, &InitTime, sizeof(EFI_TIME)); I don't think we need InitTime or this CopyMem(). instead, we can just set the Time-> fields directly, no? > + } else { > + Time->Second = BcdToDecimal8 (DateTime.VL_seconds & PCF8563_SECONDS_MASK); > + Time->Minute = BcdToDecimal8 (DateTime.Minutes & PCF8563_MINUTES_MASK); > + Time->Hour = BcdToDecimal8 (DateTime.Hours & PCF8563_HOURS_MASK); > + Time->Day = BcdToDecimal8 (DateTime.Days & PCF8563_DAYS_MASK); > + Time->Month = BcdToDecimal8 (DateTime.Century_months & PCF8563_MONTHS_MASK); > + Time->Year = BcdToDecimal8 (DateTime.Years) + EPOCH_BASE; > + > + if (DateTime.Century_months & PCF8563_CENTURY_MASK) { > + Time->Year += 100; > + } > } > > if (Capabilities != NULL) { > -- > 2.14.2 >