Re: FW: [edk2-devel] [PATCH V2 4/5] ArmPlatformPkg: Update some Runtime Service functions header

["Leif Lindholm" <quic_llindhol@quicinc.com>]

On 2024-01-31 20:04, Kinney, Michael D wrote:
> Hi Leif,
> 
> The only reference to these APIs are in the .c files in ArmPlatformPkg.
> 
> RealTimeClockLib.h that defines these APIs is in EmbeddedPkg and this
> patch series also has changes against EmbeddedPkg.

Oh, you're absolutely correct.
My bad, I must have been distracted while grepping. Apologies.

> So other than the commit message, are the code changes correct?

The commit message is all that needs changing.

/
     Leif

> Mike
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif
>> Lindholm
>> Sent: Wednesday, January 31, 2024 11:13 AM
>> To: Ren, SuqiangX <suqiangx.ren@intel.com>; ardb+tianocore@kernel.org;
>> devel@edk2.groups.io
>> Subject: Re: FW: [edk2-devel] [PATCH V2 4/5] ArmPlatformPkg: Update some
>> Runtime Service functions header
>>
>> On 2024-01-30 08:33, Ren, SuqiangX wrote:
>>> Hi All,
>>>
>>> 	Could you please help to review this patch on edk2?
>>> 	https://edk2.groups.io/g/devel/message/114661
>>>
>>> Thanks
>>> Ren, Suqiang
>>
>> Hi,
>>
>> Sorry, your original email seems to be corrupted in my mailbox, so
>> replying here.
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ren,
>> Suqiang
>>> Sent: Monday, January 29, 2024 10:30 AM
>>> To: devel@edk2.groups.io
>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>; Ard Biesheuvel
>> <ardb+tianocore@kernel.org>
>>> Subject: [edk2-devel] [PATCH V2 4/5] ArmPlatformPkg: Update some
>> Runtime Service functions header
>>>
>>> Update some Runtime Service functions header to align with UEFI spec
>> 2.10.
>>>
>>> REF: UEFI spec 2.10 section 8 Services — Runtime Services
>>
>> Nothing that is being changed by this patch is described by the UEFI
>> specification. The commit message should describe what a patch does,
>> the overall purpose of the set should be described in the cover letter
>> (and you do describe that here).
>>
>> Here is my proposed changed commit message:
>> ---
>> ArmPlatformPkg: align PL031 library function headers with UEFI return
>> values
>>
>> RealTimeClockLib is used to back the runtime services time functions,
>> so align the description of the function return values with the defined
>> values for these services as described in UEFI 2.10.
>> ---
>>
>> Apart from that, the function comment headers exist both in the .h and
>> the .c. If the .h is being updated, so should the .c.
>>
>> Best Regards,
>>
>> Leif
>>
>>> Signed-off-by: Suqiang Ren <suqiangx.ren@intel.com>
>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>> ---
>>>    .../PL031RealTimeClockLib.c                   | 22 +++++++++++++++--
>> --
>>>    1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git
>> a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
>> b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
>>> index 1896f9d16d3b..6ab3e9948335 100644
>>> ---
>> a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
>>> +++
>> b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
>>> @@ -111,7 +111,9 @@ EXIT:
>>>      @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.
>>> -  @retval EFI_SECURITY_VIOLATION The time could not be retrieved due
>> to an authentication failure.
>>> +  @retval EFI_UNSUPPORTED        This call is not supported by this
>> platform at the time the call is made.
>>> +                                 The platform should describe this
>> runtime service as unsupported at runtime
>>> +                                 via an EFI_RT_PROPERTIES_TABLE
>> configuration table.
>>>
>>>    **/
>>>    EFI_STATUS
>>> @@ -172,6 +174,9 @@ LibGetTime (
>>>      @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 to
>> hardware error.
>>> +  @retval EFI_UNSUPPORTED       This call is not supported by this
>> platform at the time the call is made.
>>> +                                The platform should describe this
>> runtime service as unsupported at runtime
>>> +                                via an EFI_RT_PROPERTIES_TABLE
>> configuration table.
>>>
>>>    **/
>>>    EFI_STATUS
>>> @@ -224,8 +229,13 @@ LibSetTime (
>>>      @param  Time                  The current alarm setting.
>>>
>>>      @retval EFI_SUCCESS           The alarm settings were returned.
>>> -  @retval EFI_INVALID_PARAMETER Any parameter is NULL.
>>> +  @retval EFI_INVALID_PARAMETER Enabled is NULL.
>>> +  @retval EFI_INVALID_PARAMETER Pending is NULL.
>>> +  @retval EFI_INVALID_PARAMETER Time is NULL.
>>>      @retval EFI_DEVICE_ERROR      The wakeup time could not be
>> retrieved due to a hardware error.
>>> +  @retval EFI_UNSUPPORTED       This call is not supported by this
>> platform at the time the call is made.
>>> +                                The platform should describe this
>> runtime service as unsupported at runtime
>>> +                                via an EFI_RT_PROPERTIES_TABLE
>> configuration table.
>>>
>>>    **/
>>>    EFI_STATUS
>>> @@ -248,9 +258,13 @@ LibGetWakeupTime (
>>>
>>>      @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_INVALID_PARAMETER Enabled is NULL.
>>> +  @retval EFI_INVALID_PARAMETER Pending is NULL.
>>> +  @retval EFI_INVALID_PARAMETER Time is NULL.
>>>      @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.
>>> +  @retval EFI_UNSUPPORTED       This call is not supported by this
>> platform at the time the call is made.
>>> +                                The platform should describe this
>> runtime service as unsupported at runtime
>>> +                                via an EFI_RT_PROPERTIES_TABLE
>> configuration table.
>>>
>>>    **/
>>>    EFI_STATUS
>>
>>
>>
>> 
>>
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114953): https://edk2.groups.io/g/devel/message/114953
Mute This Topic: https://groups.io/mt/104081753/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-