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::22c; helo=mail-it0-x22c.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x22c.google.com (mail-it0-x22c.google.com [IPv6:2607:f8b0:4001:c0b::22c]) (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 AF04D20356886 for ; Mon, 13 Nov 2017 04:38:41 -0800 (PST) Received: by mail-it0-x22c.google.com with SMTP id m191so9241169itg.2 for ; Mon, 13 Nov 2017 04:42:47 -0800 (PST) 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=YEpN0loEya64xYq39pdzdARbjYkHuhUZ0Q09+kWwMs4=; b=kACbx7ZEP37Zyr5XYBuv0bCX6oQz3TefZt+19vbZPPdQ6ZuzVLIIT4r1c4b2hhVSOy 6/LuNJYcCfyCIAevAxTeoIITcEBqMOxrSo9jdBUOK3C5dUpMwNqUJD+7w6J5NN4VLyyq umRgVAY8PKoswV7H7U0+t5B/oJZo4HQKrkc10= 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=YEpN0loEya64xYq39pdzdARbjYkHuhUZ0Q09+kWwMs4=; b=GDcK0qeUxQKFQQhV+NPnJ8ViVbUfEwvYPSPSiNpevw699HUM5mZjt1CRVCWgtL9knN /DdqcptuO8LAU8udt6GPWVOm9wOlftA4E8Mv8/1UrwL2Pjjcmx6CVGtaZCfz3/DM+5ud bn8LPHKrdDJoejtc0+YHrEnLwiSWqGvtHfYaeTQ3MDQeiH6oYUoQC4lEEpZwebYhEMPm pAi7y+zYkp7mxpNTgfriE44ivXg/mE/ZMEK3AvfoKoMozq8tC5lAlZ0Qex2tCN80qeCe IJPaFIF4HBOlZ1dR+UTetAtRER1U9TTBalY84dwtFqZXV2B74ozAp74u0NvJ/wjzByG1 KUoA== X-Gm-Message-State: AJaThX4FJTssytrwpnPHERH1tMo+WU7a4w5rrI7W5qs4UAHbch7fx5W7 GAekji/fH8Bbh5bAiKiyYxwQP4Iz1Uu2kylXwdZulwYbgsU= X-Google-Smtp-Source: AGs4zMZ0WPNYlC27XSQjWRuaG2Eaa+4ek8sOq0r5CQywEvVgASQIZx99l8ImO33pgM5QtRa18thl122riEDxhrPYUSE= X-Received: by 10.36.141.70 with SMTP id w67mr10268099itd.58.1510576967141; Mon, 13 Nov 2017 04:42:47 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.20 with HTTP; Mon, 13 Nov 2017 04:42:46 -0800 (PST) In-Reply-To: <1510065736-9394-8-git-send-email-meenakshi.aggarwal@nxp.com> References: <1510065736-9394-1-git-send-email-meenakshi.aggarwal@nxp.com> <1510065736-9394-8-git-send-email-meenakshi.aggarwal@nxp.com> From: Ard Biesheuvel Date: Mon, 13 Nov 2017 12:42:46 +0000 Message-ID: To: Meenakshi Aggarwal Cc: Leif Lindholm , "Kinney, Michael D" , "edk2-devel@lists.01.org" , Udit Kumar , Varun Sethi Subject: Re: [PATCH 07/10] Platform/NXP : Add support for DS1307 RTC library 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: Mon, 13 Nov 2017 12:38:41 -0000 Content-Type: text/plain; charset="UTF-8" On 7 November 2017 at 14:42, Meenakshi Aggarwal wrote: > Real time clock Apis on top of I2C Apis > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Meenakshi Aggarwal > --- > Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h | 40 ++++ > Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c | 226 +++++++++++++++++++++ > Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.inf | 40 ++++ > 3 files changed, 306 insertions(+) > create mode 100644 Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h > create mode 100644 Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c > create mode 100644 Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.inf > > diff --git a/Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h b/Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h > new file mode 100644 > index 0000000..952933f > --- /dev/null > +++ b/Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h > @@ -0,0 +1,40 @@ > +/** Ds1307Rtc.h > +* > +* Copyright 2017 NXP > +* > +* 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. > +* > +**/ > + > +#ifndef __DS1307RTC_H__ > +#define __DS1307RTC_H__ > + > +#define Bin(Bcd) ((Bcd) & 0x0f) + ((Bcd) >> 4) * 10 > +#define Bcd(Bin) (((Bin / 10) << 4) | (Bin % 10)) Please use BcdToDecimal8 and DecimalToBcd8 from BaseLib instead > + > +/* > + * RTC register addresses > + */ > +#define DS1307_SEC_REG_ADDR 0x00 > +#define DS1307_MIN_REG_ADDR 0x01 > +#define DS1307_HR_REG_ADDR 0x02 > +#define DS1307_DAY_REG_ADDR 0x03 > +#define DS1307_DATE_REG_ADDR 0x04 > +#define DS1307_MON_REG_ADDR 0x05 > +#define DS1307_YR_REG_ADDR 0x06 > +#define DS1307_CTL_REG_ADDR 0x07 > + > +#define DS1307_SEC_BIT_CH 0x80 /* Clock Halt (in Register 0) */ > + > +#define DS1307_CTL_BIT_RS0 0x01 /* Rate select 0 */ > +#define DS1307_CTL_BIT_RS1 0x02 /* Rate select 1 */ > +#define DS1307_CTL_BIT_SQWE 0x10 /* Square Wave Enable */ > +#define DS1307_CTL_BIT_OUT 0x80 /* Output Control */ > + > +#endif // __DS1307RTC_H__ > diff --git a/Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c b/Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c > new file mode 100644 > index 0000000..5f620a3 > --- /dev/null > +++ b/Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c > @@ -0,0 +1,226 @@ > +/** Ds1307RtcLib.c > + Implement EFI RealTimeClock with runtime services via RTC Lib for DS1307 RTC. > + > + Based on RTC implementation available in > + EmbeddedPkg/Library/TemplateRealTimeClockLib/RealTimeClockLib.c > + > + Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
> + Copyright 2017 NXP > + > + 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. > + > +**/ > + > +#include > +#include > +#include > +#include In general, please try to use the PI protocols for I2C instead of rolling your own. This will allow this driver to be shared between platforms. > +#include > + > +#include "Ds1307Rtc.h" > + > +/** > + Read RTC register. > + > + @param RtcRegAddr Register offset of RTC to be read. > + > + @retval Register Value read > + > +**/ > + > +UINT8 > +RtcRead ( > + IN UINT8 RtcRegAddr > + ) > +{ > + INT32 Status; > + UINT8 Val; > + > + Val = 0; > + Status = I2cDataRead(PcdGet32(PcdRtcI2cBus), PcdGet32(PcdDs1307I2cAddress), > + RtcRegAddr, 0x1, &Val, sizeof(Val)); > + if (EFI_ERROR(Status)) > + DEBUG((DEBUG_ERROR, "RTC read error at Addr:0x%x\n", RtcRegAddr)); > + Always use { } after if, and add space before ( > + return Val; > +} > +/** > + Write RTC register. > + > + @param RtcRegAddr Register offset of RTC to write. > + @param Val Value to be written > + > +**/ > + > +VOID > +RtcWrite( > + IN UINT8 RtcRegAddr, > + IN UINT8 Val > + ) > +{ > + EFI_STATUS Status; > + > + Status = I2cDataWrite(PcdGet32(PcdRtcI2cBus), PcdGet32(PcdDs1307I2cAddress), > + RtcRegAddr, 0x1, &Val, sizeof(Val)); > + if (EFI_ERROR(Status)) > + DEBUG((DEBUG_ERROR, "RTC write error at Addr:0x%x\n", RtcRegAddr)); same here > +} > + > +/** > + Returns the current time and date information, and the time-keeping capabilities > + of the hardware platform. > + > + @param Time A pointer to storage to receive a snapshot of the current time. > + @param Capabilities An optional pointer to a buffer to receive the real time clock > + device's capabilities. > + > + @retval EFI_SUCCESS The operation completed successfully. > + @retval EFI_INVALID_PARAMETER Time is NULL. > + @retval EFI_DEVICE_ERROR The time could not be retrieved due to hardware error. > + > +**/ > + > +EFI_STATUS > +EFIAPI > +LibGetTime ( > + OUT EFI_TIME *Time, > + OUT EFI_TIME_CAPABILITIES *Capabilities > + ) > +{ > + EFI_STATUS Status; > + UINT8 Second; > + UINT8 Minute; > + UINT8 Hour; > + UINT8 Day; > + UINT8 Month; > + UINT8 Year; > + > + Status = EFI_SUCCESS; > + > + Second = RtcRead (DS1307_SEC_REG_ADDR); > + Minute = RtcRead (DS1307_MIN_REG_ADDR); > + Hour = RtcRead (DS1307_HR_REG_ADDR); > + Day = RtcRead (DS1307_DATE_REG_ADDR); > + Month = RtcRead (DS1307_MON_REG_ADDR); > + Year = RtcRead (DS1307_YR_REG_ADDR); > + Is it safe to read the RTC using separate I2C transactions? What happens if there is a carry between any of those registers? > + if (Second & DS1307_SEC_BIT_CH) { > + DEBUG ((DEBUG_ERROR, "### Warning: RTC oscillator has stopped\n")); > + /* clear the CH flag */ > + RtcWrite (DS1307_SEC_REG_ADDR, > + RtcRead (DS1307_SEC_REG_ADDR) & ~DS1307_SEC_BIT_CH); > + Status = EFI_DEVICE_ERROR; > + } > + > + Time->Second = Bin (Second & 0x7F); > + Time->Minute = Bin (Minute & 0x7F); > + Time->Hour = Bin (Hour & 0x3F); > + Time->Day = Bin (Day & 0x3F); > + Time->Month = Bin (Month & 0x1F); > + Time->Year = Bin (Year) + ( Bin (Year) >= 70 ? 1900 : 2000); > + Please use symbolic constants and some comments to clarify how the 2-digit year counter maps onto a real year. > + return Status; > +} > + > +/** > + Sets the current local time and date information. > + > + @param Time A pointer to the current time. > + > + @retval EFI_SUCCESS The operation completed successfully. > + @retval EFI_INVALID_PARAMETER A time field is out of range. > + @retval EFI_DEVICE_ERROR The time could not be set due due to hardware error. > + > +**/ > +EFI_STATUS > +EFIAPI > +LibSetTime ( > + IN EFI_TIME *Time > + ) > +{ > + if (Time->Year < 1970 || Time->Year > 2069) > + DEBUG((DEBUG_ERROR, "WARNING: Year should be between 1970 and 2069!\n")); > + Missing { } > + RtcWrite (DS1307_YR_REG_ADDR, Bcd (Time->Year % 100)); > + RtcWrite (DS1307_MON_REG_ADDR, Bcd (Time->Month)); > + RtcWrite (DS1307_DATE_REG_ADDR, Bcd (Time->Day)); > + RtcWrite (DS1307_HR_REG_ADDR, Bcd (Time->Hour)); > + RtcWrite (DS1307_MIN_REG_ADDR, Bcd (Time->Minute)); > + RtcWrite (DS1307_SEC_REG_ADDR, Bcd (Time->Second)); > + Can you use a single bus transaction here? > + return EFI_SUCCESS; > +} > + > +/** > + Returns the current wakeup alarm clock setting. > + > + @param Enabled Indicates if the alarm is currently enabled or disabled. > + @param Pending Indicates if the alarm signal is pending and requires acknowledgement. > + @param Time The current alarm setting. > + > + @retval EFI_SUCCESS The alarm settings were returned. > + @retval EFI_INVALID_PARAMETER Any parameter is NULL. > + @retval EFI_DEVICE_ERROR The wakeup time could not be retrieved due to a hardware error. > + > +**/ > +EFI_STATUS > +EFIAPI > +LibGetWakeupTime ( > + OUT BOOLEAN *Enabled, > + OUT BOOLEAN *Pending, > + OUT EFI_TIME *Time > + ) > +{ > + // Not a required feature ... but does the IP support it? > + return EFI_UNSUPPORTED; > +} > + > +/** > + Sets the system wakeup alarm clock time. > + > + @param Enabled Enable or disable the wakeup alarm. > + @param Time If Enable is TRUE, the time to set the wakeup alarm for. > + > + @retval EFI_SUCCESS If Enable is TRUE, then the wakeup alarm was enabled. If > + Enable is FALSE, then the wakeup alarm was disabled. > + @retval EFI_INVALID_PARAMETER A time field is out of range. > + @retval EFI_DEVICE_ERROR The wakeup time could not be set due to a hardware error. > + @retval EFI_UNSUPPORTED A wakeup timer is not supported on this platform. > + > +**/ > +EFI_STATUS > +EFIAPI > +LibSetWakeupTime ( > + IN BOOLEAN Enabled, > + OUT EFI_TIME *Time > + ) > +{ > + // Not a required feature > + return EFI_UNSUPPORTED; > +} > + > +/** > + This is the declaration of an EFI image entry point. This can be the entry point to an application > + written to this specification, an EFI boot service driver, or an EFI runtime driver. > + > + @param ImageHandle Handle that identifies the loaded image. > + @param SystemTable System Table for this image. > + > + @retval EFI_SUCCESS The operation completed successfully. > + > +**/ > +EFI_STATUS > +EFIAPI > +LibRtcInitialize ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + return I2cBusInit(PcdGet32(PcdRtcI2cBus), PcdGet32(PcdI2cSpeed)); > +} > diff --git a/Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.inf b/Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.inf > new file mode 100644 > index 0000000..b068b43 > --- /dev/null > +++ b/Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.inf > @@ -0,0 +1,40 @@ > +# @Ds1307RtcLib.inf > +# > +# Lib to provide support for DS1307 Real Time Clock > +# > +# Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved. > +# Copyright (c) 2017 NXP > +# > +# 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. > +# > +# > + > +[Defines] > + INF_VERSION = 0x00010005 0x0001001A > + BASE_NAME = Ds1307RtcLib > + FILE_GUID = B661E02D-A90B-42AB-A5F9-CF841AAA43D9 Please don't reuse the GUID of TemplateRealTimeClockLib.inf > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = RealTimeClockLib > + > +[Sources.common] > + Ds1307RtcLib.c > + > +[Packages] > + EmbeddedPkg/EmbeddedPkg.dec > + MdePkg/MdePkg.dec > + edk2-platforms/Platform/NXP/NxpQoriqLs.dec > + > +[LibraryClasses] > + DebugLib > + I2cLib > + > +[Pcd] > + gNxpQoriqLsTokenSpaceGuid.PcdRtcI2cBus > + gNxpQoriqLsTokenSpaceGuid.PcdI2cSpeed > + gNxpQoriqLsTokenSpaceGuid.PcdDs1307I2cAddress > -- > 1.9.1 >