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 BDD5B210D6CE7 for ; Mon, 18 Jun 2018 06:03:51 -0700 (PDT) Received: by mail-it0-x244.google.com with SMTP id a195-v6so11830524itd.3 for ; Mon, 18 Jun 2018 06:03:51 -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=DCBkyuz6oHr9FxpcO/t70JKKLyS/cM1IIP+9Hzty4pE=; b=TZVdVq3W+hovxC31bFiaR5M1eRIQhi3TvSCbTkz4R2zHzah8UuZXG4ysmE4yoRbq/C NieaKhHtZgib5GZ94F0MxjBL2YSR26crz0PG8yFVRJJwOWt9FFGv7TEc5YDZVebjstyj qjzR9Fzpmpfm2tZY+C5m5LTUgo3vN5zEE/m34= 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=DCBkyuz6oHr9FxpcO/t70JKKLyS/cM1IIP+9Hzty4pE=; b=C03TWvQ+W/6H5u5yNqEHlojzx0wvRPNlJJm+a2mH0LUv/+VD+bN28LYjV1CfGxFO45 sZ2qDe4BXuFLHam87r8wCyab84IxqTZQ+DOMynCobFDljJASxPV+wFN76l8o42os4p4T itMOPwLxq+2UVQd+WthEF/y65MfU1LyYi72jidT2aLco3TnYjEBSaj4D3j3+M4ViOZcZ cYq7MDB/W8ORr44lPzlgWdfItC37Sa1LaYUxC/h1D6L/0Y8C4AsZtgYKIM6FUsyOSRih B7Tk4a3MQTsEgUlpUl0di/Vx6LthLN1ZISNYZNpr1Ur31lKUhHXM1b0/B4czCbgXL3O/ JwgA== X-Gm-Message-State: APt69E1NZ8qqRQbConnQhRZ57ZaSZy31aVIJchAjKSYIXuqdpRkn89jq VVzTBgl7qs7xKKA1bt7GesC567tAP2vqnTkkA0VbMnVM X-Google-Smtp-Source: ADUXVKKxuPaC7/sQItWq1JWD/KHo7g0o2GlZIXdVODQZQxcnfM7W03SnLl9QGMU5hKvlSWkV43mhd5CWy8MJSoqp1Bw= X-Received: by 2002:a02:2422:: with SMTP id f34-v6mr9918054jaa.2.1529327030750; Mon, 18 Jun 2018 06:03:50 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Mon, 18 Jun 2018 06:03:50 -0700 (PDT) In-Reply-To: <20180618123155.2141-1-masahisa.kojima@linaro.org> References: <20180618123155.2141-1-masahisa.kojima@linaro.org> From: Ard Biesheuvel Date: Mon, 18 Jun 2018 15:03:50 +0200 Message-ID: To: Masahisa Kojima Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Yoshitoyo Osaki Subject: Re: [PATCH] Silicon/NXP/Pcf8563RealTimeClockLib: add rtc device initialization 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: Mon, 18 Jun 2018 13:03:52 -0000 Content-Type: text/plain; charset="UTF-8" On 18 June 2018 at 14:31, 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 > 2) DEBUG build > > 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. > > 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 set dummy date/time(01/01/2000 00:00:00) to > proceed succeeding SetTime 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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Masahisa Kojima > Signed-off-by: Yoshitoyo Osaki > --- > .../Pcf8563RealTimeClockLib.c | 32 ++++++++++++++++------ > 1 file changed, 23 insertions(+), 9 deletions(-) > > diff --git a/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c b/Silicon/NXP/Library/Pcf8563RealTimeClockLib/Pcf8563RealTimeClockLib.c > index fb58e1feb4..7be0d23eea 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,26 @@ 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) { Wouldn't it be better to return EFI_DEVICE_ERROR in this case? > + InitTime.Second = 0; > + InitTime.Minute = 0; > + InitTime.Hour = 0; > + InitTime.Day = 1; > + InitTime.Month = 1; > + InitTime.Year = EPOCH_BASE; > + (VOID)LibSetTime (&InitTime); > + (VOID)CopyMem (Time, &InitTime, sizeof(EFI_TIME)); > + } 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 >