From: Ming Huang <huangming23@huawei.com>
To: Leif Lindholm <leif@nuviainc.com>
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 v2 2/3] Hisilicon/Library: Move two functions to RtcHelperLib
Date: Tue, 19 May 2020 21:26:13 +0800 [thread overview]
Message-ID: <2f416ae5-b3da-f580-673f-ac31fa8a3241@huawei.com> (raw)
In-Reply-To: <20200519121632.GO10467@vanye>
在 2020/5/19 20:16, Leif Lindholm 写道:
> On Tue, May 19, 2020 at 19:08:40 +0800, Ming Huang wrote:
>> The functions of acquiring ownership of RTC will be used for other
>> RTC library, so move them to RtcHelperLib. Rename them by add leading
>> Oem for uniform.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>
> We don't use this anymore - it is not required now the codebase is
> under BSD+Patent. Please drop the tag from v3.
Ok.
>
>> Signed-off-by: Ming Huang <huangming23@huawei.com>
>> ---
>> Platform/Hisilicon/D06/D06.dsc | 1 +
>> Silicon/Hisilicon/Include/Library/RtcHelperLib.h | 29 ++++++
>> Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h | 7 --
>> Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c | 92 ++-----------------
>> Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf | 1 +
>> Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.c | 94 ++++++++++++++++++++
>> Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.inf | 32 +++++++
>> 7 files changed, 164 insertions(+), 92 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..29ec900
>> --- /dev/null
>> +++ b/Silicon/Hisilicon/Include/Library/RtcHelperLib.h
>> @@ -0,0 +1,29 @@
>> +/** @file
>> +
>> + Copyright (c) 2018, Hisilicon Limited. All rights reserved.<BR>
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#ifndef RTC_HELPER_LIB_H__
>> +#define RTC_HELPER_LIB_H__
>> +
>> +// 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_1000_MICROSECOND 1000
>> +// The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need.
>> +#define RTC_DELAY_2_MICROSECOND 2
>> +
>> +EFI_STATUS
>> +OemSwitchRtcI2cChannelAndLock (
>> + VOID
>> + );
>> +
>> +VOID
>> +OemReleaseOwnershipOfRtc (
>
> These two helper functions have been renamed since v1, which is now
> being obscured by the move. If you want to rename them, that should be
> a separate patch submitted *after* this set has been merged.
>
> Undoing this will mean changes to both 2/3 and 3/3, so please send a
> v3 also dropping the Contributed-under tags - including in v1 which is
> otherwise fine.
Ok, I will send v3 out soon.
Thanks,
Ming
>
> /
> Leif
>
>> + VOID
>> + );
>> +
>> +#endif
>> diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h
>> index 81ff459..09329c4 100644
>> --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h
>> +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h
>> @@ -10,13 +10,6 @@
>> #ifndef __M41T83_REAL_TIME_CLOCK_H__
>> #define __M41T83_REAL_TIME_CLOCK_H__
>>
>> -// 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_1000_MICROSECOND 1000
>> -// The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need.
>> -#define RTC_DELAY_2_MICROSECOND 2
>> -
>> #define M41T83_REGADDR_DOTSECONDS 0x00
>> #define M41T83_REGADDR_SECONDS 0x01
>> #define M41T83_REGADDR_MINUTES 0x02
>> diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
>> index f339bcc..db356ca 100644
>> --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
>> +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
>> @@ -11,10 +11,9 @@
>> #include <PiDxe.h>
>> #include <Library/BaseLib.h>
>> #include <Library/BaseMemoryLib.h>
>> -#include <Library/CpldD06.h>
>> -#include <Library/CpldIoLib.h>
>> #include <Library/DebugLib.h>
>> #include <Library/I2CLib.h>
>> +#include <Library/RtcHelperLib.h>
>> #include <Library/TimeBaseLib.h>
>> #include <Library/TimerLib.h>
>> #include <Library/UefiLib.h>
>> @@ -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_MICROSECOND);
>> - 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
>> @@ -172,7 +94,7 @@ InitializeM41T83 (
>> return Status;
>> }
>>
>> - Status = SwitchRtcI2cChannelAndLock ();
>> + Status = OemSwitchRtcI2cChannelAndLock ();
>> if (EFI_ERROR (Status)) {
>> DEBUG ((DEBUG_ERROR, "Get i2c preemption failed: %r\n", Status));
>> if (!EfiAtRuntime ()) {
>> @@ -225,7 +147,7 @@ InitializeM41T83 (
>>
>> Exit:
>> // Release RTC Lock.
>> - ReleaseOwnershipOfRtc ();
>> + OemReleaseOwnershipOfRtc ();
>> if (!EfiAtRuntime ()) {
>> EfiReleaseLock (&mRtcLock);
>> }
>> @@ -268,7 +190,7 @@ LibSetTime (
>> return EFI_INVALID_PARAMETER;
>> }
>>
>> - Status = SwitchRtcI2cChannelAndLock ();
>> + Status = OemSwitchRtcI2cChannelAndLock ();
>> if (EFI_ERROR (Status)) {
>> return Status;
>> }
>> @@ -326,7 +248,7 @@ LibSetTime (
>> }
>>
>> Exit:
>> - ReleaseOwnershipOfRtc ();
>> + OemReleaseOwnershipOfRtc ();
>> // Release RTC Lock.
>> if (!EfiAtRuntime ()) {
>> if (EFI_ERROR (Status)) {
>> @@ -371,7 +293,7 @@ LibGetTime (
>> return EFI_INVALID_PARAMETER;
>> }
>>
>> - Status = SwitchRtcI2cChannelAndLock ();
>> + Status = OemSwitchRtcI2cChannelAndLock ();
>> if (EFI_ERROR (Status)) {
>> return Status;
>> }
>> @@ -416,7 +338,7 @@ LibGetTime (
>> }
>>
>> Exit:
>> - ReleaseOwnershipOfRtc ();
>> + OemReleaseOwnershipOfRtc ();
>> // Release RTC Lock.
>> if (!EfiAtRuntime ()) {
>> if (EFI_ERROR (Status)) {
>> 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..af6b8dc
>> --- /dev/null
>> +++ b/Silicon/Hisilicon/Library/RtcHelperLib/RtcHelperLib.c
>> @@ -0,0 +1,94 @@
>> +/** @file
>> +
>> + Copyright (c) 2020, Hisilicon Limited. All rights reserved.<BR>
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include <Uefi.h>
>> +#include <PiDxe.h>
>> +#include <Library/BaseLib.h>
>> +#include <Library/CpldD06.h>
>> +#include <Library/CpldIoLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/RtcHelperLib.h>
>> +#include <Library/TimeBaseLib.h>
>> +#include <Library/TimerLib.h>
>> +#include <Library/UefiRuntimeLib.h>
>> +
>> +EFI_STATUS
>> +OemSwitchRtcI2cChannelAndLock (
>> + 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_MICROSECOND);
>> + 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
>> +OemReleaseOwnershipOfRtc (
>> + 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.<BR>
>> +#
>> +# 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
>>
>
> .
>
next prev parent reply other threads:[~2020-05-19 13:26 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-19 11:08 [RFC edk2-platforms v2 0/3] Improve D0x Ming Huang
2020-05-19 11:08 ` [RFC edk2-platforms v2 1/3] Silicon/Hisilicon: Fix a typo issue in M41T83RealTimeClockLib Ming Huang
2020-05-19 11:08 ` [RFC edk2-platforms v2 2/3] Hisilicon/Library: Move two functions to RtcHelperLib Ming Huang
2020-05-19 12:16 ` Leif Lindholm
2020-05-19 13:26 ` Ming Huang [this message]
2020-05-19 11:08 ` [RFC edk2-platforms v2 3/3] Silicon/Hisilicon: Add RX8900RealTimeClockLib Ming Huang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2f416ae5-b3da-f580-673f-ac31fa8a3241@huawei.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox