* Re: [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
2019-12-26 6:53 [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment Marc W Chen
@ 2019-12-31 6:17 ` Zhang, Shenglei
2020-01-03 1:04 ` Liming Gao
2020-01-06 8:33 ` Zhang, Shenglei
2020-01-08 19:49 ` Kubacki, Michael A
2 siblings, 1 reply; 5+ messages in thread
From: Zhang, Shenglei @ 2019-12-31 6:17 UTC (permalink / raw)
To: Chen, Marc W, devel@edk2.groups.io
Cc: Kubacki, Michael A, Chaganty, Rangasai V, Gao, Liming
Reviewed-by: Shenglei Zhang<shenglei.zhang@intel.com>
> -----Original Message-----
> From: Chen, Marc W
> Sent: Thursday, December 26, 2019 2:53 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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]
> IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2436
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@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 | 19 ++++++++----------
> -
> .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.c | 6 +++---
> .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.h | 13 +++++----
> ----
> .../IntelSiliconPkg/Include/Library/SmmAccessLib.h | 5 +----
> 4 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> index da141cfa0e..61da7ea0bd 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> @@ -46,9 +46,9 @@ typedef struct {
> The use of "open" means that the memory is visible from all PEIM
> and SMM agents.
>
> + @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.
> - @param[in] PeiServices - General purpose services available to every
> PEIM.
>
> @retval EFI_SUCCESS - The region was successfully opened.
> @retval EFI_DEVICE_ERROR - The region could not be opened because
> locked by
> @@ -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 a PEI_MM_ACCESS_PPI PPI for controlling SMM mode
> memory access basically for S3 resume usage.
>
> @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/SmmAc
> cessDriver.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> index 3d3c4ab206..9409345f6b 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> @@ -18,7 +18,7 @@ static SMM_ACCESS_PRIVATE_DATA mSmmAccess;
> @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.
> **/
> @@ -229,9 +229,9 @@ 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.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> index c0ff3a250b..a2ea6332fa 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> @@ -59,9 +59,6 @@ typedef struct {
> 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.
>
> @@ -69,7 +66,7 @@ typedef struct {
> @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
> **/
> EFI_STATUS
> EFIAPI
> @@ -142,10 +139,10 @@ 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.
> diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> index f658bac68c..0a287b75b2 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> @@ -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 a PEI_MM_ACCESS_PPI PPI for controlling SMM mode
> memory access basically for S3 resume usage.
>
> @retval EFI_SUCCESS - Ppi successfully started and installed.
> @retval EFI_NOT_FOUND - Ppi can't be found.
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
2019-12-31 6:17 ` Zhang, Shenglei
@ 2020-01-03 1:04 ` Liming Gao
0 siblings, 0 replies; 5+ messages in thread
From: Liming Gao @ 2020-01-03 1:04 UTC (permalink / raw)
To: Zhang, Shenglei, Chen, Marc W, devel@edk2.groups.io
Cc: Kubacki, Michael A, Chaganty, Rangasai V, Ni, Ray
Reviewed-by: Liming Gao <liming.gao@intel.com>
-----Original Message-----
From: Zhang, Shenglei <shenglei.zhang@intel.com>
Sent: 2019年12月31日 14:18
To: Chen, Marc W <marc.w.chen@intel.com>; devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
Reviewed-by: Shenglei Zhang<shenglei.zhang@intel.com>
> -----Original Message-----
> From: Chen, Marc W
> Sent: Thursday, December 26, 2019 2:53 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chaganty,
> Rangasai V <rangasai.v.chaganty@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]
> IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2436
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@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 | 19 ++++++++----------
> -
> .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.c | 6 +++---
> .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.h | 13 +++++----
> ----
> .../IntelSiliconPkg/Include/Library/SmmAccessLib.h | 5 +----
> 4 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccess
> Li
> b/PeiSmmAccessLib.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccess
> Li
> b/PeiSmmAccessLib.c
> index da141cfa0e..61da7ea0bd 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccess
> Li
> b/PeiSmmAccessLib.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccess
> Li
> b/PeiSmmAccessLib.c
> @@ -46,9 +46,9 @@ typedef struct {
> The use of "open" means that the memory is visible from all PEIM
> and SMM agents.
>
> + @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.
> - @param[in] PeiServices - General purpose services available to every
> PEIM.
>
> @retval EFI_SUCCESS - The region was successfully opened.
> @retval EFI_DEVICE_ERROR - The region could not be opened because
> locked by
> @@ -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 a PEI_MM_ACCESS_PPI PPI for controlling SMM
> + mode
> memory access basically for S3 resume usage.
>
> @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/SmmAc
> cessDriver.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> index 3d3c4ab206..9409345f6b 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> @@ -18,7 +18,7 @@ static SMM_ACCESS_PRIVATE_DATA mSmmAccess;
> @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.
> **/
> @@ -229,9 +229,9 @@ 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.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> index c0ff3a250b..a2ea6332fa 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> @@ -59,9 +59,6 @@ typedef struct {
> 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.
>
> @@ -69,7 +66,7 @@ typedef struct {
> @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
> **/
> EFI_STATUS
> EFIAPI
> @@ -142,10 +139,10 @@ 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.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> index f658bac68c..0a287b75b2 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> @@ -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 a PEI_MM_ACCESS_PPI PPI for controlling SMM
> + mode
> memory access basically for S3 resume usage.
>
> @retval EFI_SUCCESS - Ppi successfully started and installed.
> @retval EFI_NOT_FOUND - Ppi can't be found.
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
2019-12-26 6:53 [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment Marc W Chen
2019-12-31 6:17 ` Zhang, Shenglei
@ 2020-01-06 8:33 ` Zhang, Shenglei
2020-01-08 19:49 ` Kubacki, Michael A
2 siblings, 0 replies; 5+ messages in thread
From: Zhang, Shenglei @ 2020-01-06 8:33 UTC (permalink / raw)
To: devel@edk2.groups.io, Ni, Ray
Cc: Kubacki, Michael A, Chaganty, Rangasai V, Gao, Liming,
Chen, Marc W
Hi Ray,
Could you help review this patch?
Thanks,
Shenglei
> -----Original Message-----
> From: Chen, Marc W
> Sent: Thursday, December 26, 2019 2:53 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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]
> IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2436
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@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 | 19 ++++++++----------
> -
> .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.c | 6 +++---
> .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.h | 13 +++++----
> ----
> .../IntelSiliconPkg/Include/Library/SmmAccessLib.h | 5 +----
> 4 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> index da141cfa0e..61da7ea0bd 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> @@ -46,9 +46,9 @@ typedef struct {
> The use of "open" means that the memory is visible from all PEIM
> and SMM agents.
>
> + @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.
> - @param[in] PeiServices - General purpose services available to every
> PEIM.
>
> @retval EFI_SUCCESS - The region was successfully opened.
> @retval EFI_DEVICE_ERROR - The region could not be opened because
> locked by
> @@ -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 a PEI_MM_ACCESS_PPI PPI for controlling SMM mode
> memory access basically for S3 resume usage.
>
> @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/SmmAc
> cessDriver.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> index 3d3c4ab206..9409345f6b 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> @@ -18,7 +18,7 @@ static SMM_ACCESS_PRIVATE_DATA mSmmAccess;
> @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.
> **/
> @@ -229,9 +229,9 @@ 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.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> index c0ff3a250b..a2ea6332fa 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> @@ -59,9 +59,6 @@ typedef struct {
> 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.
>
> @@ -69,7 +66,7 @@ typedef struct {
> @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
> **/
> EFI_STATUS
> EFIAPI
> @@ -142,10 +139,10 @@ 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.
> diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> index f658bac68c..0a287b75b2 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> @@ -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 a PEI_MM_ACCESS_PPI PPI for controlling SMM mode
> memory access basically for S3 resume usage.
>
> @retval EFI_SUCCESS - Ppi successfully started and installed.
> @retval EFI_NOT_FOUND - Ppi can't be found.
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
2019-12-26 6:53 [edk2-devel][edk2-platforms][PATCH] IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment Marc W Chen
2019-12-31 6:17 ` Zhang, Shenglei
2020-01-06 8:33 ` Zhang, Shenglei
@ 2020-01-08 19:49 ` Kubacki, Michael A
2 siblings, 0 replies; 5+ messages in thread
From: Kubacki, Michael A @ 2020-01-08 19:49 UTC (permalink / raw)
To: Chen, Marc W, devel@edk2.groups.io
Cc: Chaganty, Rangasai V, Gao, Liming, Zhang, Shenglei
edk2-platforms\Silicon\Intel\IntelSiliconPkg\Include\Library\SmmAccessLib.h
* I think the description would be clearer by making the following change:
"An API to install a PEI_MM_ACCESS_PPI PPI for controlling SMM mode memory access basically for S3 resume usage."
to
"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."
edk2-platforms\Silicon\Intel\IntelSiliconPkg\Feature\SmmAccess\Library\PeiSmmAccessLib\PeiSmmAccessLib.c
* I suggest aligning the " General purpose services available to every PEIM." on the same vertical column as the other parameter text in the function description.
* Same update to the function description for PeiInstallSmmAccessPei () that was suggested in SmmAccessLib.h
edk2-platforms\Silicon\Intel\IntelSiliconPkg\Feature\SmmAccess\SmmAccessDxe\SmmAccessDriver.h
edk2-platforms\Silicon\Intel\IntelSiliconPkg\Feature\SmmAccess\SmmAccessDxe\SmmAccessDriver.c
* The function description for SmmAccessDriverEntryPoint () is inconsistent in these files.
* Some return values in the description in SmmAccessDriver.c are missing from the description in SmmAccessDriver.h
With these updates:
Reviewed-by: Michael Kubacki <michael.a.kubacki@intel.com>
Thanks,
Michael
> -----Original Message-----
> From: Chen, Marc W <marc.w.chen@intel.com>
> Sent: Wednesday, December 25, 2019 10:53 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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]
> IntelSiliconPkg/Feature/SmmAccess/*: Fix incorrect Docygen comment
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2436
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@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 | 19 ++++++++----------
> -
> .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.c | 6 +++---
> .../Feature/SmmAccess/SmmAccessDxe/SmmAccessDriver.h | 13 +++++----
> ----
> .../IntelSiliconPkg/Include/Library/SmmAccessLib.h | 5 +----
> 4 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> index da141cfa0e..61da7ea0bd 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAcce
> +++ ssLib/PeiSmmAccessLib.c
> @@ -46,9 +46,9 @@ typedef struct {
> The use of "open" means that the memory is visible from all PEIM
> and SMM agents.
>
> + @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.
> - @param[in] PeiServices - General purpose services available to every
> PEIM.
>
> @retval EFI_SUCCESS - The region was successfully opened.
> @retval EFI_DEVICE_ERROR - The region could not be opened because
> locked by
> @@ -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 a PEI_MM_ACCESS_PPI PPI for controlling SMM mode
> memory access basically for S3 resume usage.
>
> @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/SmmAc
> cessDriver.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> index 3d3c4ab206..9409345f6b 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> +++ cessDriver.c
> @@ -18,7 +18,7 @@ static SMM_ACCESS_PRIVATE_DATA mSmmAccess;
> @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.
> **/
> @@ -229,9 +229,9 @@ 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.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> index c0ff3a250b..a2ea6332fa 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> cessDriver.h
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAc
> +++ cessDriver.h
> @@ -59,9 +59,6 @@ typedef struct {
> 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.
>
> @@ -69,7 +66,7 @@ typedef struct {
> @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
> **/
> EFI_STATUS
> EFIAPI
> @@ -142,10 +139,10 @@ 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.
> diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> index f658bac68c..0a287b75b2 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmAccessLib.h
> @@ -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 a PEI_MM_ACCESS_PPI PPI for controlling SMM mode
> memory access basically for S3 resume usage.
>
> @retval EFI_SUCCESS - Ppi successfully started and installed.
> @retval EFI_NOT_FOUND - Ppi can't be found.
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread