From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by mx.groups.io with SMTP id smtpd.web11.702.1589824986514794257 for ; Mon, 18 May 2020 11:03:06 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=Na1oHNTb; spf=pass (domain: nuviainc.com, ip: 209.85.221.68, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f68.google.com with SMTP id h17so12906095wrc.8 for ; Mon, 18 May 2020 11:03:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=9FKhX1zkRf91PgEjFSZ7NPzGGZ11WxLbczaeSiO0ZeU=; b=Na1oHNTb2P6ops2jLnbSijNgMK/Df7hoMldI3v5y/MgmwOicSibx1CoAKXYxWaVM1s 7WrVQM+mMrxjY/Odu5olRZxF7agz0o80RaTPPus1BZMvyxSwPWoq/fstrwXxkACerpbD kb939i6DNp6oKWzHOAyFFn9yVZQzoq+HBAYxeF+vI65L+mppsuU0U8rxKIieviNzl62g A39M9WWB39DeBAl3xSEp/83FnZq+B2qA+Rfyjf7CeK14iFPS5vMaxSTS64fAAfIxb/pf GqVkxGHqAxiSq+x352d387BHqhY+97CZC0OK0yvLf3k25izNP2RCijSu+Q4vxQKFHHCM 9c6w== 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=9FKhX1zkRf91PgEjFSZ7NPzGGZ11WxLbczaeSiO0ZeU=; b=W+L8muhRsnjbJC2HU1ff7vVbDRPJPDiVZ6eeT9PrXPGLTFP6pMWmoLd3X7NxPKZrWe UhvCqAtXXNX/6jAE73+9Sne+zHrUrbPZZdDqUvAJqKgRDhHNuTjAaOloJlReJaWmer8K jilXJgfPUwQ9tat0pmiRvZJMFTgtiYksrsEsSN62es+7oTAxkKUPNBRJrnGsvUlNAec2 bomLbQ2fAh1oelUqcC21iTyViehTtqQtzGoPDVZ4ciA9ZMM9mqIWkMQJtxDPA/r3Fc50 JHQe3YZA3cY8XGzlarsuR1qqmTLBbHsdSRBy5Bs98+S4JYgBt1XmARmrFAqGlVItLwOv gGIA== X-Gm-Message-State: AOAM530gOr1OOBjzwyFTicXMkeD4TVMZil1ZMO2j8Z38zTFo/zrZ96sv vxC1mmdAkk71uUxtAH9RCzqK0A== X-Google-Smtp-Source: ABdhPJx6junmiv0SwhWQ7/ALfhHDxmH+9AVjjXEV1dXZzz0LbfG8GAW70vUL+raD662H8skRdFps9g== X-Received: by 2002:a5d:650f:: with SMTP id x15mr21129004wru.100.1589824985025; Mon, 18 May 2020 11:03:05 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id c19sm18379382wrb.89.2020.05.18.11.03.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 May 2020 11:03:04 -0700 (PDT) Date: Mon, 18 May 2020 19:03:02 +0100 From: "Leif Lindholm" To: Ming Huang Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org, lidongzhan@huawei.com, songdongkuang@huawei.com, wanghuiqiang@huawei.com, qiuliangen@huawei.com Subject: Re: [RFC edk2-platforms v1 1/2] Hisilicon/Library: Move two functions to RtcHelperLib Message-ID: <20200518180302.GH10467@vanye> References: <1589809044-59210-1-git-send-email-huangming23@huawei.com> <1589809044-59210-2-git-send-email-huangming23@huawei.com> MIME-Version: 1.0 In-Reply-To: <1589809044-59210-2-git-send-email-huangming23@huawei.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, May 18, 2020 at 21:37:23 +0800, Ming Huang wrote: > The functions of acquiring ownership of RTC will be used for other > RTC library, so move them to RtcHelperLib. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ming Huang > --- > Platform/Hisilicon/D06/D06.dsc | 1 + > Silicon/Hisilicon/Include/Library/RtcHelperLib.h | 28 ++++++ > Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c | 80 +---------------- > Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf | 1 + > Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.c | 94 ++++++++++++++++++++ > Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.inf | 32 +++++++ > 6 files changed, 157 insertions(+), 79 deletions(-) > > diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc > index 000a4ee..eb20224 100644 > --- a/Platform/Hisilicon/D06/D06.dsc > +++ b/Platform/Hisilicon/D06/D06.dsc > @@ -54,6 +54,7 @@ > CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf > > TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf > + RtcHelperLib|Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.inf > RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf > OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf > OemAddressMapLib|Platform/Hisilicon/D06/Library/OemAddressMapD06/OemAddressMapD06.inf > diff --git a/Silicon/Hisilicon/Include/Library/RtcHelperLib.h b/Silicon/Hisilicon/Include/Library/RtcHelperLib.h > new file mode 100644 > index 0000000..d6c1d39 > --- /dev/null > +++ b/Silicon/Hisilicon/Include/Library/RtcHelperLib.h > @@ -0,0 +1,28 @@ > +/** @file > + > + Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> + Copyright (c) 2018, Linaro Limited. All rights reserved.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef __RTC_HELPER_LIB_H__ > +#define __RTC_HELPER_LIB_H__ Please drop leading __ from macros. Those are reserved for toolchain use. > + > +// The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need. > +#define RTC_DELAY_30_MS 30000 > +// The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need. > +#define RTC_DELAY_2_MACROSECOND 2 I guess MACROSECOND is a typo for MICROSECOND? But we are now duplicating these macros between M41T83RealTimeClock.h and RtcHelperLib.h. And M41T83RealTimeClock.h still holds a bunch of other _MACROSECOND #defines. Could we move all of the macro definitions to this file instead? Could we also fix the spelling, in a patch preceding this one? / Leif > + > +EFI_STATUS > +SwitchRtcI2cChannelAndLock ( > + VOID > + ); > + > +VOID > +ReleaseOwnershipOfRtc ( > + VOID > + ); > + > +#endif > diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c > index 5131ce7..e3fecac 100644 > --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c > +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c > @@ -11,10 +11,9 @@ > #include > #include > #include > -#include > -#include > #include > #include > +#include > #include > #include > #include > @@ -26,70 +25,6 @@ extern I2C_DEVICE gRtcDevice; > > STATIC EFI_LOCK mRtcLock; > > -EFI_STATUS > -SwitchRtcI2cChannelAndLock ( > - VOID > - ) > -{ > - UINT8 Temp; > - UINT8 Count; > - > - for (Count = 0; Count < 100; Count++) { > - // To get the other side's state is idle first > - Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > - if ((Temp & BIT3) != 0) { > - (VOID) MicroSecondDelay (RTC_DELAY_30_MS); > - // Try 100 times, if BMC has not released the bus, return preemption failed > - if (Count == 99) { > - if (!EfiAtRuntime ()) { > - DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Clear cpu_i2c_rtc_state 100 times fail!\n", > - __FUNCTION__, __LINE__)); > - } > - return EFI_DEVICE_ERROR; > - } > - continue; > - } > - > - // if BMC free the bus, can be set 1 preemption > - Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > - Temp = Temp | CPU_GET_I2C_CONTROL; > - // CPU occupied RTC I2C State > - WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp); > - (VOID) MicroSecondDelay (RTC_DELAY_2_MACROSECOND); > - Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > - // Is preempt success > - if(CPU_GET_I2C_CONTROL == (Temp & CPU_GET_I2C_CONTROL)) { > - break; > - } > - if (Count == 99) { > - if (!EfiAtRuntime ()) { > - DEBUG((DEBUG_ERROR, "[%a]:[%dL] Clear cpu_i2c_rtc_state fail !!! \n", > - __FUNCTION__, __LINE__)); > - } > - return EFI_DEVICE_ERROR; > - } > - (VOID) MicroSecondDelay (RTC_DELAY_30_MS); > - } > - > - //Polling BMC RTC I2C status > - for (Count = 0; Count < 100; Count++) { > - Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > - if ((Temp & BIT3) == 0) { > - return EFI_SUCCESS; > - } > - (VOID) MicroSecondDelay (RTC_DELAY_30_MS); > - } > - > - //If the BMC occupies the RTC I2C Channel, write back the CPU side is idle > - // or the subsequent BMC will not preempt > - Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > - Temp = Temp & (~CPU_GET_I2C_CONTROL); > - WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp); > - > - return EFI_NOT_READY; > -} > - > - > /** > Read RTC content through its registers. > > @@ -136,19 +71,6 @@ RtcWrite ( > return Status; > } > > -VOID > -ReleaseOwnershipOfRtc ( > - VOID > - ) > -{ > - UINT8 Temp; > - > - Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > - Temp = Temp & ~CPU_GET_I2C_CONTROL; > - WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp); > -} > - > - > EFI_STATUS > InitializeM41T83 ( > VOID > diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf > index 9344c6d..5970c0e 100644 > --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf > +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf > @@ -31,6 +31,7 @@ > I2CLib > IoLib > PcdLib > + RtcHelperLib > TimeBaseLib > TimerLib > UefiLib > diff --git a/Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.c b/Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.c > new file mode 100644 > index 0000000..bc8e6c5 > --- /dev/null > +++ b/Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.c > @@ -0,0 +1,94 @@ > +/** @file > + > + Copyright (c) 2020, Hisilicon Limited. All rights reserved.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +EFI_STATUS > +SwitchRtcI2cChannelAndLock ( > + VOID > + ) > +{ > + UINT8 Temp; > + UINT8 Count; > + > + for (Count = 0; Count < 100; Count++) { > + // To get the other side's state is idle first > + Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > + if ((Temp & BIT3) != 0) { > + (VOID) MicroSecondDelay (RTC_DELAY_30_MS); > + // Try 100 times, if BMC has not released the bus, return preemption failed > + if (Count == 99) { > + if (!EfiAtRuntime ()) { > + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Clear cpu_i2c_rtc_state 100 times fail!\n", > + __FUNCTION__, __LINE__)); > + } > + return EFI_DEVICE_ERROR; > + } > + continue; > + } > + > + // if BMC free the bus, can be set 1 preemption > + Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > + Temp = Temp | CPU_GET_I2C_CONTROL; > + // CPU occupied RTC I2C State > + WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp); > + (VOID)MicroSecondDelay (RTC_DELAY_2_MACROSECOND); > + Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > + // Is preempt success > + if (CPU_GET_I2C_CONTROL == (Temp & CPU_GET_I2C_CONTROL)) { > + break; > + } > + if (Count == 99) { > + if (!EfiAtRuntime ()) { > + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Clear cpu_i2c_rtc_state fail !!! \n", > + __FUNCTION__, __LINE__)); > + } > + return EFI_DEVICE_ERROR; > + } > + (VOID)MicroSecondDelay (RTC_DELAY_30_MS); > + } > + > + //Polling BMC RTC I2C status > + for (Count = 0; Count < 100; Count++) { > + Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > + if ((Temp & BIT3) == 0) { > + return EFI_SUCCESS; > + } > + (VOID)MicroSecondDelay (RTC_DELAY_30_MS); > + } > + > + //If the BMC occupies the RTC I2C Channel, write back the CPU side is idle > + // or the subsequent BMC will not preempt > + Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > + Temp = Temp & (~CPU_GET_I2C_CONTROL); > + WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp); > + > + return EFI_NOT_READY; > +} > + > +VOID > +ReleaseOwnershipOfRtc ( > + VOID > + ) > +{ > + UINT8 Temp; > + > + Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); > + Temp = Temp & ~CPU_GET_I2C_CONTROL; > + WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp); > + return ; > +} > diff --git a/Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.inf b/Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.inf > new file mode 100644 > index 0000000..1a36e64 > --- /dev/null > +++ b/Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.inf > @@ -0,0 +1,32 @@ > +#/** @file > +# > +# Copyright (c) 2020, Hisilicon Limited. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +#**/ > + > +[Defines] > + INF_VERSION = 0x0001001A > + BASE_NAME = RtcHelperLib > + FILE_GUID = 5cb1a98f-2408-4fef-b68f-d5d04ff6a91f > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = RtcHelperLib > + > +[Sources.common] > + RtcHelperLib.c > + > +[Packages] > + EmbeddedPkg/EmbeddedPkg.dec > + MdePkg/MdePkg.dec > + Platform/Hisilicon/D06/D06.dec > + Silicon/Hisilicon/HisiPkg.dec > + > +[LibraryClasses] > + CpldIoLib > + DebugLib > + IoLib > + > +[Depex] > + TRUE > -- > 2.8.1 >