Re: [edk2-devel][edk2-platforms][PATCH v5] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Doxygen comment

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
> Sent: Friday, January 10, 2020 2:09 PM
> To: Chen, Marc W <marc.w.chen@intel.com>; devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang, Shenglei
> <shenglei.zhang@intel.com>
> Subject: RE: [edk2-devel][edk2-platforms][PATCH v5]
> IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Doxygen comment
> 
> Reviewed-by: Sai Chaganty <rangasai.v.chaganty@intel.com>
> 
> -----Original Message-----
> From: Chen, Marc W <marc.w.chen@intel.com>
> Sent: Thursday, January 09, 2020 9:20 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com>; Chen,
> Marc W <marc.w.chen@intel.com>
> Subject: [edk2-devel][edk2-platforms][PATCH v5]
> IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Doxygen comment
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2436
> 
> Fixed some doxygen comment that does not follow correct doxygen format.
> Updated some description of some functions.
> 
> Notes:
> v1:
> - Initial version of code change.
> v2:
> - Update some description of some functions.
> v3:
> - Update some description of some functions.
> v4:
> - Change "post" to "POST".
> v5:
> - Fixed typo of subject from "Doxcgen" to "Doxygen".
> 
> 
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Shenglei Zhang <shenglei.zhang@intel.com>
> Signed-off-by: Marc Chen <marc.w.chen@intel.com>
> ---
>  .../Library/PeiSmmAccessLib/PeiSmmAccessLib.c      | 37 +++++++++----------
>  .../SmmAccess/SmmAccessDxe/SmmAccessDriver.c       | 35 +++++++++++-------
>  .../SmmAccess/SmmAccessDxe/SmmAccessDriver.h       | 41 ++++++++++---------
> ---
>  .../IntelSiliconPkg/Include/Library/SmmAccessLib.h |  7 ++--
>  4 files changed, 60 insertions(+), 60 deletions(-)
> 
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/Pe
> iSmmAccessLib.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/Pe
> iSmmAccessLib.c
> index da141cfa0e..d9bf4fba98 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/Pe
> iSmmAccessLib.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/Pe
> iSmmAccessLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>    This is to publish the SMM Access Ppi instance.
> 
> -  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -46,9 +46,9 @@ typedef struct {
>    The use of "open" means that the memory is visible from all PEIM
>    and SMM agents.
> 
> -  @param[in] This             -  Pointer to the SMM Access Interface.
> -  @param[in] DescriptorIndex  -  Region of SMRAM to Open.
> -  @param[in] PeiServices      -  General purpose services available to every PEIM.
> +  @param[in] PeiServices         -  General purpose services available to every
> PEIM.
> +  @param[in] This                -  Pointer to the SMM Access Interface.
> +  @param[in] DescriptorIndex     -  Region of SMRAM to Open.
> 
>    @retval EFI_SUCCESS            -  The region was successfully opened.
>    @retval EFI_DEVICE_ERROR       -  The region could not be opened because
> locked by
> @@ -89,9 +89,9 @@ Open (
>    This routine accepts a request to "close" a region of SMRAM.  This is valid for
>    compatible SMRAM region.
> 
> -  @param[in] PeiServices      -  General purpose services available to every PEIM.
> -  @param[in] This             -  Pointer to the SMM Access Interface.
> -  @param[in] DescriptorIndex  -  Region of SMRAM to Close.
> +  @param[in] PeiServices         -  General purpose services available to every
> PEIM.
> +  @param[in] This                -  Pointer to the SMM Access Interface.
> +  @param[in] DescriptorIndex     -  Region of SMRAM to Close.
> 
>    @retval EFI_SUCCESS            -  The region was successfully closed.
>    @retval EFI_DEVICE_ERROR       -  The region could not be closed because
> locked by
> @@ -151,9 +151,9 @@ Close (
>    The use of "lock" means that the memory can no longer be opened
>    to PEIM.
> 
> -  @param[in] PeiServices      - General purpose services available to every PEIM.
> -  @param[in] This             -  Pointer to the SMM Access Interface.
> -  @param[in] DescriptorIndex  -  Region of SMRAM to Lock.
> +  @param[in] PeiServices         - General purpose services available to every
> PEIM.
> +  @param[in] This                -  Pointer to the SMM Access Interface.
> +  @param[in] DescriptorIndex     -  Region of SMRAM to Lock.
> 
>    @retval EFI_SUCCESS            -  The region was successfully locked.
>    @retval EFI_DEVICE_ERROR       -  The region could not be locked because at
> least
> @@ -193,12 +193,12 @@ Lock (
>    ranges that are possible for SMRAM access, based upon the
>    memory controller capabilities.
> 
> -  @param[in] PeiServices   - General purpose services available to every PEIM.
> -  @param[in] This          -  Pointer to the SMRAM Access Interface.
> -  @param[in] SmramMapSize  -  Pointer to the variable containing size of the
> -                              buffer to contain the description information.
> -  @param[in] SmramMap      -  Buffer containing the data describing the Smram
> -                              region descriptors.
> +  @param[in] PeiServices        - General purpose services available to every
> PEIM.
> +  @param[in] This               -  Pointer to the SMRAM Access Interface.
> +  @param[in, out] SmramMapSize  -  Pointer to the variable containing size of
> the
> +                                   buffer to contain the description information.
> +  @param[in, out] SmramMap      -  Buffer containing the data describing the
> Smram
> +                                   region descriptors.
> 
>    @retval EFI_BUFFER_TOO_SMALL  -  The user did not provide a sufficient
> buffer.
>    @retval EFI_SUCCESS           -  The user provided a sufficiently-sized buffer.
> @@ -234,10 +234,7 @@ GetCapabilities (
>  /**
>    This function is to install an SMM Access PPI
>    - <b>Introduction</b> \n
> -    A module to install a PPI for controlling SMM mode memory access basically
> for S3 resume usage.
> -
> -  - @result
> -    Publish _EFI_PEI_MM_ACCESS_PPI.
> +    An API to install an instance of EFI_PEI_MM_ACCESS_PPI. This PPI is
> commonly used to control SMM mode memory access for S3 resume.
> 
>      @retval EFI_SUCCESS           - Ppi successfully started and installed.
>      @retval EFI_NOT_FOUND         - Ppi can't be found.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.c
> index 3d3c4ab206..1cbce1907c 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.c
> @@ -2,7 +2,7 @@
>    This is the driver that publishes the SMM Access Protocol
>    instance for System Agent.
> 
> -  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -11,14 +11,24 @@
>  static SMM_ACCESS_PRIVATE_DATA  mSmmAccess;
> 
>  /**
> -  This is the standard EFI driver point that
> -  installs an SMM Access Protocol
> -
> -  @param[in] ImageHandle     - Handle for the image of this driver
> -  @param[in] SystemTable     - Pointer to the EFI System Table
> +  <b>SMM Access Driver Entry Point</b>
> +  This driver installs an SMM Access Protocol
> +  - <b>Introduction</b> \n
> +    This module publishes the SMM access protocol.  The protocol is used by the
> SMM Base driver to access the SMRAM region when the processor is not in SMM.
> +    The SMM Base driver uses the services provided by the SMM access protocol
> to open SMRAM during POST and copy the SMM handler.
> +    SMM access protocol is also used to close the SMRAM region once the
> copying is done.
> +    Finally, the SMM access protocol provides services to "Lock" the SMRAM
> region.
> +    Please refer the SMM Protocols section in the attached SMM CIS
> Specification version 0.9 for further details.
> +    This driver is required if SMM is supported. Proper configuration of SMM
> registers is recommended even if SMM is not supported.
> +
> +  - <b>Porting Recommendations</b> \n
> +    No modification of this module is recommended.  Any modification should
> be done in compliance with the _EFI_SMM_ACCESS_PROTOCOL protocol
> definition.
> +
> +  @param[in] ImageHandle        - Handle for the image of this driver
> +  @param[in] SystemTable        - Pointer to the EFI System Table
> 
>    @retval EFI_SUCCESS           - Protocol was installed successfully
> -  @exception EFI_UNSUPPORTED    - Protocol was not installed
> +  @retval EFI_UNSUPPORTED       - Protocol was not installed
>    @retval EFI_NOT_FOUND         - Protocol can't be found.
>    @retval EFI_OUT_OF_RESOURCES  - Protocol does not have enough resources
> to initialize the driver.
>  **/
> @@ -108,8 +118,7 @@ SmmAccessDriverEntryPoint (
>    @param[in] This               - Pointer to the SMM Access Interface.
> 
>    @retval EFI_SUCCESS           - The region was successfully opened.
> -  @retval EFI_DEVICE_ERROR      - The region could not be opened because
> locked by
> -                          chipset.
> +  @retval EFI_DEVICE_ERROR      - The region could not be opened because
> locked by chipset.
>    @retval EFI_INVALID_PARAMETER - The descriptor index was out of bounds.
>  **/
>  EFI_STATUS
> @@ -229,13 +238,13 @@ Lock (
>    memory controller capabilities.
> 
>    @param[in] This                  - Pointer to the SMRAM Access Interface.
> -  @param[in] SmramMapSize          - Pointer to the variable containing size of
> the
> +  @param[in, out] SmramMapSize     - Pointer to the variable containing size of
> the
>                                       buffer to contain the description information.
> -  @param[in] SmramMap              - Buffer containing the data describing the
> Smram
> +  @param[in, out] SmramMap         - Buffer containing the data describing the
> Smram
>                                       region descriptors.
> 
> -  @retval EFI_BUFFER_TOO_SMALL  - The user did not provide a sufficient
> buffer.
> -  @retval EFI_SUCCESS           - The user provided a sufficiently-sized buffer.
> +  @retval EFI_BUFFER_TOO_SMALL     - The user did not provide a sufficient
> buffer.
> +  @retval EFI_SUCCESS              - The user provided a sufficiently-sized buffer.
>  **/
>  EFI_STATUS
>  EFIAPI
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.h
> index c0ff3a250b..bcdaef7ba6 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.h
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess
> Driver.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Header file for SMM Access Driver.
> 
> -  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -53,23 +53,22 @@ typedef struct {
>    This driver installs an SMM Access Protocol
>    - <b>Introduction</b> \n
>      This module publishes the SMM access protocol.  The protocol is used by the
> SMM Base driver to access the SMRAM region when the processor is not in SMM.
> -    The SMM Base driver uses the services provided by the SMM access protocol
> to open SMRAM during post and copy the SMM handler.
> +    The SMM Base driver uses the services provided by the SMM access protocol
> to open SMRAM during POST and copy the SMM handler.
>      SMM access protocol is also used to close the SMRAM region once the
> copying is done.
>      Finally, the SMM access protocol provides services to "Lock" the SMRAM
> region.
>      Please refer the SMM Protocols section in the attached SMM CIS
> Specification version 0.9 for further details.
>      This driver is required if SMM is supported. Proper configuration of SMM
> registers is recommended even if SMM is not supported.
> 
> -  - @result
> -    Publishes the _EFI_SMM_ACCESS_PROTOCOL: Documented in the System
> Management Mode Core Interface Specification, available at the URL:
> http://www.intel.com/technology/framework/spec.htm
> -
>    - <b>Porting Recommendations</b> \n
>      No modification of this module is recommended.  Any modification should be
> done in compliance with the _EFI_SMM_ACCESS_PROTOCOL protocol definition.
> 
> -  @param[in] ImageHandle     - Handle for the image of this driver
> -  @param[in] SystemTable     - Pointer to the EFI System Table
> +  @param[in] ImageHandle        - Handle for the image of this driver
> +  @param[in] SystemTable        - Pointer to the EFI System Table
> 
> -  @retval EFI_SUCCESS     - Protocol was installed successfully
> -  @exception EFI_UNSUPPORTED - Protocol was not installed
> +  @retval EFI_SUCCESS           - Protocol was installed successfully
> +  @retval EFI_UNSUPPORTED       - Protocol was not installed
> +  @retval EFI_NOT_FOUND         - Protocol can't be found.
> +  @retval EFI_OUT_OF_RESOURCES  - Protocol does not have enough
> resources to initialize the driver.
>  **/
>  EFI_STATUS
>  EFIAPI
> @@ -87,8 +86,7 @@ SmmAccessDriverEntryPoint (
>    @param[in] This                  - Pointer to the SMM Access Interface.
> 
>    @retval EFI_SUCCESS           - The region was successfully opened.
> -  @retval EFI_DEVICE_ERROR      - The region could not be opened because
> locked by
> -                          chipset.
> +  @retval EFI_DEVICE_ERROR      - The region could not be opened because
> locked by chipset.
>    @retval EFI_INVALID_PARAMETER - The descriptor index was out of bounds.
>  **/
>  EFI_STATUS
> @@ -103,11 +101,10 @@ Open (
>    The use of "close" means that the memory is only visible from SMM agents,
>    not from BS or RT code.
> 
> -  @param[in] This                  - Pointer to the SMM Access Interface.
> +  @param[in] This               - Pointer to the SMM Access Interface.
> 
>    @retval EFI_SUCCESS           - The region was successfully closed.
> -  @retval EFI_DEVICE_ERROR      - The region could not be closed because
> locked by
> -                            chipset.
> +  @retval EFI_DEVICE_ERROR      - The region could not be closed because
> locked by chipset.
>    @retval EFI_INVALID_PARAMETER - The descriptor index was out of bounds.
>  **/
>  EFI_STATUS
> @@ -122,11 +119,11 @@ Close (
>    The use of "lock" means that the memory can no longer be opened
>    to BS state..
> 
> -  @param[in] This                  - Pointer to the SMM Access Interface.
> +  @param[in] This               - Pointer to the SMM Access Interface.
> 
>    @retval EFI_SUCCESS           - The region was successfully locked.
>    @retval EFI_DEVICE_ERROR      - The region could not be locked because at
> least
> -                          one range is still open.
> +                                  one range is still open.
>    @retval EFI_INVALID_PARAMETER - The descriptor index was out of bounds.
>  **/
>  EFI_STATUS
> @@ -142,13 +139,13 @@ Lock (
>    memory controller capabilities.
> 
>    @param[in] This                  - Pointer to the SMRAM Access Interface.
> -  @param[in] SmramMapSize          - Pointer to the variable containing size of
> the
> -                            buffer to contain the description information.
> -  @param[in] SmramMap              - Buffer containing the data describing the
> Smram
> -                            region descriptors.
> +  @param[in, out] SmramMapSize     - Pointer to the variable containing size of
> the
> +                                     buffer to contain the description information.
> +  @param[in, out] SmramMap         - Buffer containing the data describing the
> Smram
> +                                     region descriptors.
> 
> -  @retval EFI_BUFFER_TOO_SMALL  - The user did not provide a sufficient
> buffer.
> -  @retval EFI_SUCCESS           - The user provided a sufficiently-sized buffer.
> +  @retval EFI_BUFFER_TOO_SMALL     - The user did not provide a sufficient
> buffer.
> +  @retval EFI_SUCCESS              - The user provided a sufficiently-sized buffer.
>  **/
>  EFI_STATUS
>  EFIAPI
> diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> index f658bac68c..9792bc4099 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Header file for SMM Access Driver.
> 
> -  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -11,10 +11,7 @@
>  /**
>    This function is to install an SMM Access PPI
>    - <b>Introduction</b> \n
> -    A module to install a PPI for controlling SMM mode memory access basically
> for S3 resume usage.
> -
> -  - @result
> -    Publish _PEI_MM_ACCESS_PPI.
> +    An API to install an instance of EFI_PEI_MM_ACCESS_PPI. This PPI is
> commonly used to control SMM mode memory access for S3 resume.
> 
>      @retval EFI_SUCCESS           - Ppi successfully started and installed.
>      @retval EFI_NOT_FOUND         - Ppi can't be found.
> --
> 2.16.2.windows.1
>