From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::543; helo=mail-pg1-x543.google.com; envelope-from=ming.huang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pg1-x543.google.com (mail-pg1-x543.google.com [IPv6:2607:f8b0:4864:20::543]) (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 2FBA62118FF2E for ; Wed, 14 Nov 2018 06:31:14 -0800 (PST) Received: by mail-pg1-x543.google.com with SMTP id d72so4005685pga.9 for ; Wed, 14 Nov 2018 06:31:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=2ygdcPzUoDP13HNnVkOROG6gKfdGVCzfv5esqKaovRY=; b=HeHXnzBTIgR+AMKhndrqHInC+Et45Zy3eYBarotpL9ITbx7Tw6iJYiHXKOuxIaUCHB oiWoT9qnxjKmVfoMeLXV/aQ0J9aLmNXkDBO+Rd07ogRm8m3g7vnH7Xjgp3mySDj3unV/ AEAcOoD9ckNQu2njFXMYKx1rrtJucVeerVUSs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=2ygdcPzUoDP13HNnVkOROG6gKfdGVCzfv5esqKaovRY=; b=VHgZnrHXjOsZQJ1QB8GAJrzOCc5v0mXT8PKJ4ZPhGPTsI5h/Kj1LEvF26o7ERmY9d0 f9swDDdFnH6BP5Yqwqdyax6ySEOoqPJuayDkadwrhgaUj6pnzZjPVxgNauEF2pjleNhU ZQ9dH1VAVePcc/HHaRzoqcQJ5mRpwp7WLNIzB5RrkJpE2mZlk95FIimcRDqshqlwDwi9 nwncvIK103DGz6yK2WeHl8V3OEM28TiziO6aKuB9oAd4Kp/HDBgaouEsIR8S9W4F4hgY bebLyGI+fVvP78n0GoQw+9BzES/1gIw8Mxq/VSC4PMLGLMhB7j7rg3PTCcQQinAd2eU5 hliQ== X-Gm-Message-State: AGRZ1gKIoG5Zeq8sCt/1pp+wGll56ONgj9lte+oWUMmM3f/0YSk8ECUH vNilsBSrq+a9BI7eZm6qBE+Qaw== X-Google-Smtp-Source: AJdET5dviQjiJA54ILyR7jg3I6julMRUOofjN9rO5ZxZSjEfUr7lzfnzr4JIiBgW4zsMOJKhCGk2Lg== X-Received: by 2002:a62:8915:: with SMTP id v21-v6mr2226693pfd.137.1542205874195; Wed, 14 Nov 2018 06:31:14 -0800 (PST) Received: from [10.39.0.150] ([64.64.108.117]) by smtp.gmail.com with ESMTPSA id m20sm20446066pgb.56.2018.11.14.06.31.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Nov 2018 06:31:13 -0800 (PST) To: Leif Lindholm Cc: linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org, graeme.gregory@linaro.org, ard.biesheuvel@linaro.org, michael.d.kinney@intel.com, lersek@redhat.com, wanghuiqiang@huawei.com, huangming23@huawei.com, zhangjinsong2@huawei.com, huangdaode@hisilicon.com, john.garry@huawei.com, xinliang.liu@linaro.org, zhangfeng56@huawei.com References: <20181029033249.45363-1-ming.huang@linaro.org> <20181029033249.45363-6-ming.huang@linaro.org> <20181114000452.gk366schvazwpzrm@bivouac.eciton.net> From: Ming Huang Message-ID: Date: Wed, 14 Nov 2018 22:30:57 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <20181114000452.gk366schvazwpzrm@bivouac.eciton.net> Subject: Re: [PATCH edk2-platforms v1 05/12] Hisilicon/D06: Move some functions to OemMiscLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Nov 2018 14:31:15 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/14/2018 8:04 AM, Leif Lindholm wrote: > On Mon, Oct 29, 2018 at 11:32:42AM +0800, Ming Huang wrote: >> As M41T83RealTimeClockLib is common library, so move two cpld >> relative functions to OemMiscLib and rename this two functions. > > This would be more clear as "platform specific" than "cpld relative". > > I did not realise this wasn't a Hisilicon component when reviewing the > original set. > > I approve of this change, but can you tell me why it is included in > this set? If the goal is to make the M41T83 support platform > independent, should the library also move to Silicon/ST/? This change had make in internal master and the next patch is base on this patch, so I add this patch. Main gist is remove platform specific functions from M41T83RealTimeClockLib. > > / > Leif > >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ming Huang >> --- >> Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf | 1 - >> Silicon/Hisilicon/Include/Library/OemMiscLib.h | 9 ++ >> Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h | 4 - >> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c | 82 ++++++++++++++++++ >> Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c | 90 ++------------------ >> 5 files changed, 98 insertions(+), 88 deletions(-) >> >> diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf >> index e0bf6b3f24..4e963fd453 100644 >> --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf >> +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf >> @@ -27,7 +27,6 @@ >> [Packages] >> EmbeddedPkg/EmbeddedPkg.dec >> MdePkg/MdePkg.dec >> - Platform/Hisilicon/D06/D06.dec >> Silicon/Hisilicon/HisiPkg.dec >> >> [LibraryClasses] >> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h >> index 86ea6a1b3d..0d7bf71b17 100644 >> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h >> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h >> @@ -53,4 +53,13 @@ BOOLEAN OemIsNeedDisableExpanderBuffer(VOID); >> >> extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM]; >> EFI_HII_HANDLE EFIAPI OemGetPackages (); >> + >> +VOID >> +OemReleaseOwnershipOfRtc ( >> + VOID >> + ); >> +EFI_STATUS >> +OemSwitchRtcI2cChannelAndLock ( >> + VOID >> + ); >> #endif >> diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h >> index d985055d9b..f329108858 100644 >> --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h >> +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h >> @@ -16,12 +16,8 @@ >> #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_MACROSECOND 1000 >> -// The delay is need for cpld and I2C. This is a empirical value. MemoryFence is no need. >> -#define RTC_DELAY_2_MACROSECOND 2 >> >> #define M41T83_REGADDR_DOTSECONDS 0x00 >> #define M41T83_REGADDR_SECONDS 0x01 >> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c >> index 2a9db46d1f..64d167d18a 100644 >> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c >> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c >> @@ -17,6 +17,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -27,6 +28,12 @@ >> #include >> #include >> #include >> +#include >> + >> +// 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 >> >> REPORT_PCIEDIDVID2BMC PcieDeviceToReport[PCIEDEVICE_REPORT_MAX] = { >> {67,0,0,0}, >> @@ -207,3 +214,78 @@ OemIsNeedDisableExpanderBuffer ( >> { >> return TRUE; >> } >> + >> +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_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 >> +OemReleaseOwnershipOfRtc ( >> + VOID >> + ) >> +{ >> + UINT8 Temp; >> + >> + Temp = ReadCpldReg (CPLD_I2C_SWITCH_FLAG); >> + Temp = Temp & ~CPU_GET_I2C_CONTROL; >> + WriteCpldReg (CPLD_I2C_SWITCH_FLAG, Temp); >> +} >> diff --git a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c >> index 0670f9c5f4..1f50ad4b64 100644 >> --- a/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c >> +++ b/Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c >> @@ -17,10 +17,10 @@ >> #include >> #include >> #include >> -#include >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -32,70 +32,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. >> >> @@ -142,18 +78,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 ( >> @@ -178,7 +102,7 @@ InitializeM41T83 ( >> return Status; >> } >> >> - Status = SwitchRtcI2cChannelAndLock (); >> + Status = OemSwitchRtcI2cChannelAndLock (); >> if (EFI_ERROR (Status)) { >> DEBUG ((DEBUG_ERROR, "Get i2c preemption failed: %r\n", Status)); >> if (!EfiAtRuntime ()) { >> @@ -231,7 +155,7 @@ InitializeM41T83 ( >> >> Exit: >> // Release RTC Lock. >> - ReleaseOwnershipOfRtc (); >> + OemReleaseOwnershipOfRtc (); >> if (!EfiAtRuntime ()) { >> EfiReleaseLock (&mRtcLock); >> } >> @@ -274,7 +198,7 @@ LibSetTime ( >> return EFI_INVALID_PARAMETER; >> } >> >> - Status = SwitchRtcI2cChannelAndLock (); >> + Status = OemSwitchRtcI2cChannelAndLock (); >> if (EFI_ERROR (Status)) { >> return Status; >> } >> @@ -332,7 +256,7 @@ LibSetTime ( >> } >> >> Exit: >> - ReleaseOwnershipOfRtc (); >> + OemReleaseOwnershipOfRtc (); >> // Release RTC Lock. >> if (!EfiAtRuntime ()) { >> if (EFI_ERROR (Status)) { >> @@ -377,7 +301,7 @@ LibGetTime ( >> return EFI_INVALID_PARAMETER; >> } >> >> - Status = SwitchRtcI2cChannelAndLock (); >> + Status = OemSwitchRtcI2cChannelAndLock (); >> if (EFI_ERROR (Status)) { >> return Status; >> } >> @@ -422,7 +346,7 @@ LibGetTime ( >> } >> >> Exit: >> - ReleaseOwnershipOfRtc (); >> + OemReleaseOwnershipOfRtc (); >> // Release RTC Lock. >> if (!EfiAtRuntime ()) { >> if (EFI_ERROR (Status)) { >> -- >> 2.18.0 >>