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

["Michael D Kinney" <michael.d.kinney@intel.com>]

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.

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

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 (#114919): https://edk2.groups.io/g/devel/message/114919
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]
-=-=-=-=-=-=-=-=-=-=-=-