From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::233; helo=mail-wm0-x233.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x233.google.com (mail-wm0-x233.google.com [IPv6:2a00:1450:400c:c09::233]) (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 2B9B221B00DC1 for ; Thu, 16 Nov 2017 08:15:23 -0800 (PST) Received: by mail-wm0-x233.google.com with SMTP id 9so1322171wme.4 for ; Thu, 16 Nov 2017 08:19:33 -0800 (PST) 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:in-reply-to:user-agent; bh=gvYjkH9prla9ez3yUhDUPqTKggg4mg0OWbkMwFJFiYY=; b=NqM898XRTuRuRX7IWNTS6tmI8QoaGndOOXppdTckH/7lwgpbcCZNcfPGk3KwDFdpwp wc59xBsR/uJWsRVNYeVydl4OFrzf8A7RzTi8J+Sc/EYRk2ux49IdymetbOfcaQ8mbWNm oYqJNmudwq2AsGIh7q9FOpTV3AfoT0Gqa6sPI= 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:in-reply-to:user-agent; bh=gvYjkH9prla9ez3yUhDUPqTKggg4mg0OWbkMwFJFiYY=; b=oGmN5AFAvuFhbYkbqQC9Ht4bXS4A9WAXlvdYbRWzlbvGZSsaxwby5wsEAUMJ/iQUce W8S1qxjm2pZ6emWkwg4UW1kFKb65XEZsSpq3bWbAvlZjSKOn/sTUae8/A2rQbr+5Aia5 EDmszHV9hLq7Vza0srbOf10SK2sEhvPMFNfGOSIIPanmL3MPKhx/g97E1LqFYe2Iy+0I LHUivBh3zkGV5DvFVqEXWhob97HUrGKbQ1MSbRFenL+opoueMUbd/qnKOnxaKx9vnXSB SJTLZaJCVEVlbqnBv/Ef3ex+5dXhY3abvTPmyAnpviJOopPbsHTin8UNEhb/WAGwRpFy qeUQ== X-Gm-Message-State: AJaThX7kcHAJ75pGd1tbgrwYGTzORDwLdA6qAubYukjbXqxwVlJ7W5cm tEQouO+GGfXfMt8mDDpZ4hJF2w== X-Google-Smtp-Source: AGs4zMYrk3/FPZ9lqLzdHNTp95KczvugGSa2ZjzdXxsPof8xhj2wbRSBhAxlkTFHXWFsni20Qerf+w== X-Received: by 10.28.210.129 with SMTP id j123mr1999745wmg.52.1510849172222; Thu, 16 Nov 2017 08:19:32 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id k9sm1606290wrk.88.2017.11.16.08.19.30 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 16 Nov 2017 08:19:31 -0800 (PST) Date: Thu, 16 Nov 2017 16:19:29 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org Message-ID: <20171116161929.m3katezdyc25auky@bivouac.eciton.net> References: <20171115130645.31738-1-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20171115130645.31738-1-ard.biesheuvel@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH] ArmPlatformPkg/PL031RealTimeClockLib: drop ArmPlatformSysConfigLib reference 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, 16 Nov 2017 16:15:24 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Nov 15, 2017 at 01:06:45PM +0000, Ard Biesheuvel wrote: > The PL031 driver implements a VExpress/Juno specific hack to set the > battery backed clock in addition to the PL031. However, none of the > remaining VExpress based hardware we support in EDK2 actuall implements > this feature so we can just remove it, and get rid of the cumbersome > dependency on ArmPlatform.h. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel Yes, this is an improvement. Can you also delete the ArmPlatformSysConfigLib dependency in .inf as part of this patch? If you can, and fold that in: Reviewed-by: Leif Lindholm / Leif > --- > ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 43 +++----------------- > 1 file changed, 6 insertions(+), 37 deletions(-) > > diff --git a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c > index 459dcc0a0519..1334ad446cd9 100644 > --- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c > +++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c > @@ -23,7 +23,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -38,8 +37,6 @@ > > #include > > -#include > - > STATIC BOOLEAN mPL031Initialized = FALSE; > STATIC EFI_EVENT mRtcVirtualAddrChangeEvent; > STATIC UINTN mPL031RtcBase; > @@ -133,6 +130,11 @@ LibGetTime ( > EFI_STATUS Status = EFI_SUCCESS; > UINT32 EpochSeconds; > > + // Ensure Time is a valid pointer > + if (Time == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > // Initialize the hardware if not already done > if (!mPL031Initialized) { > Status = InitializePL031 (); > @@ -141,27 +143,7 @@ LibGetTime ( > } > } > > - // Snapshot the time as early in the function call as possible > - // On some platforms we may have access to a battery backed up hardware clock. > - // If such RTC exists try to use it first. > - Status = ArmPlatformSysConfigGet (SYS_CFG_RTC, &EpochSeconds); > - if (Status == EFI_UNSUPPORTED) { > - // Battery backed up hardware RTC does not exist, revert to PL031 > - EpochSeconds = MmioRead32 (mPL031RtcBase + PL031_RTC_DR_DATA_REGISTER); > - Status = EFI_SUCCESS; > - } else if (EFI_ERROR (Status)) { > - // Battery backed up hardware RTC exists but could not be read due to error. Abort. > - return Status; > - } else { > - // Battery backed up hardware RTC exists and we read the time correctly from it. > - // Now sync the PL031 to the new time. > - MmioWrite32 (mPL031RtcBase + PL031_RTC_LR_LOAD_REGISTER, EpochSeconds); > - } > - > - // Ensure Time is a valid pointer > - if (Time == NULL) { > - return EFI_INVALID_PARAMETER; > - } > + EpochSeconds = MmioRead32 (mPL031RtcBase + PL031_RTC_DR_DATA_REGISTER); > > // Adjust for the correct time zone > // The timezone setting also reflects the DST setting of the clock > @@ -235,19 +217,6 @@ LibSetTime ( > EpochSeconds -= SEC_PER_HOUR; > } > > - // On some platforms we may have access to a battery backed up hardware clock. > - // > - // If such RTC exists then it must be updated first, before the PL031, > - // to minimise any time drift. This is important because the battery backed-up > - // RTC maintains the master time for the platform across reboots. > - // > - // If such RTC does not exist then the following function returns UNSUPPORTED. > - Status = ArmPlatformSysConfigSet (SYS_CFG_RTC, EpochSeconds); > - if ((EFI_ERROR (Status)) && (Status != EFI_UNSUPPORTED)){ > - // Any status message except SUCCESS and UNSUPPORTED indicates a hardware failure. > - return Status; > - } > - > // Set the PL031 > MmioWrite32 (mPL031RtcBase + PL031_RTC_LR_LOAD_REGISTER, EpochSeconds); > > -- > 2.11.0 >