From: Ray Ni <ray.ni@intel.com>

According to the markdown language syntax, headings should be after
number signs (#). The number of number signs correspond to the heading
level.
But current PatchFvUserManual.md doesn't insert a space between the
number signs and the heading title, resulting the markdown file is not
rendered well in markdown viewers.

The patch doesn't change any content but only adds spaces to ensure
the headings are correctly recognized.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
Cc: Ray Han Lim Ng <ray.han.lim.ng@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ted Kuo <ted.kuo@intel.com>
Reviewed-by: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Susovan Mohapatra <susovan.mohapatra@intel.com>
---
 .../Tools/UserManuals/PatchFvUserManual.md    | 38 +++++++++----------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md b/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
index f28eedf625..205ad57773 100644
--- a/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
+++ b/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
@@ -1,9 +1,9 @@
-#Name
+# Name
 **_PatchFv.py_** - The python script that patches the firmware volumes (**FV**)
 with in the flash device (**FD**) file post FSP build.
 From version 0.60, script is capable of patching flash device (**FD**) directly.
 
-#Synopsis
+# Synopsis
 
 ```
 PatchFv FvBuildDir [FvFileBaseNames:]FdFileBaseNameToPatch ["Offset, Value"]+
@@ -18,32 +18,32 @@ PatchFv FdFileDir FdFileName ["Offset, Value"]+
   | ["Offset, Value, $Command, @Comment"]+
 ```
 
-#Description
+# Description
 The **_PatchFv.py_** tool allows the developer to fix up FD images to follow the
 Intel FSP Architecture specification.  It also makes the FD image relocatable.
 The tool is written in Python and uses Python 2.7 or later to run.
 Consider using the tool in a build script.
 
-#FvBuildDir (Argument 1)
+# FvBuildDir (Argument 1)
 This is the first argument that **_PatchFv.py_** requires.  It is the build
 directory for all firmware volumes created during the FSP build. The path must
 be either an absolute path or a relevant path, relevant to the top level of the
 FSP tree.
 
-####Example usage:
+#### Example usage:
 ```
  Build\YouPlatformFspPkg\%BD_TARGET%_%VS_VERSION%%VS_X86%\FV
 ```
 
 The example used contains Windows batch script %VARIABLES%.
 
-#FvFileBaseNames (Argument 2: Optional Part 1)
+# FvFileBaseNames (Argument 2: Optional Part 1)
 The firmware volume file base names (**_FvFileBaseNames_**) are the independent
-Fv?s that are to be patched within the FD. (0 or more in the form
-**FVFILEBASENAME:**) The colon **:** is used for delimiting the single
+FVs that are to be patched within the FD. (0 or more in the form
+**FvFileBaseNames:**) The colon **:** is used for delimiting the single
 argument and must be appended to the end of each (**_FvFileBaseNames_**).
 
-####Example usage:
+#### Example usage:
 ```
 STAGE1:STAGE2:MANIFEST:YOURPLATFORM
 ```
@@ -55,14 +55,14 @@ In the example **STAGE1** is **STAGE1.Fv** in **YOURPLATFORM.fd**.
 Firmware device file name to patch (**_FdFileNameToPatch_**) is the base name of
 the FD file that is to be patched. (1 only, in the form **YOURPLATFORM**)
 
-####Example usage:
+#### Example usage:
 ```
 STAGE1:STAGE2:MANIFEST:YOURPLATFORM
 ```
 
 In the example **YOURPLATFORM** is from **_YOURPLATFORM.fd_**
 
-#"Offset, Value[, Command][, Comment]" (Argument 3)
+# "Offset, Value[, Command][, Comment]" (Argument 3)
 The **_Offset_** can be a positive or negative number and represents where the
 **_Value_** to be patched is located within the FD. The **_Value_** is what
 will be written at the given **_Offset_** in the FD. Constants may be used for
@@ -79,10 +79,10 @@ The entire argument includes the quote marks like in the example argument below:
 0xFFFFFFC0, SomeCore:__EntryPoint - [0x000000F0],@SomeCore Entry
 ```
 
-###Constants:
+### Constants:
  Hexadecimal (use **0x** as prefix) | Decimal
 
-####Examples:
+#### Examples:
 
 | **Positive Hex** | **Negative Hex** | **Positive Decimal** | **Negative Decimal** |
 | ---------------: | ---------------: | -------------------: | -------------------: |
@@ -93,7 +93,7 @@ ModuleName:FunctionName | ModuleName:GlobalVariableName
 ModuleGuid:Offset
 ```
 
-###Operators:
+### Operators:
 
 ```
 
@@ -113,7 +113,7 @@ From version 0.60 tool allows to pass flash device file path as Argument 1 and
 flash device name as Argument 2 and rules for passing offset & value are same
 as explained in the previous sections.
 
-####Example usage:
+#### Example usage:
 Argument 1
 ```
  YouPlatformFspBinPkg\
@@ -123,21 +123,21 @@ Argument 2
  Fsp_Rebased_T
 ```
 
-###Special Commands:
+### Special Commands:
 Special commands must use the **$** symbol as a prefix to the command itself.
 There is only one command available at this time.
 
 ```
-$COPY ? Copy a binary block from source to destination.
+$COPY   Copy a binary block from source to destination.
 ```
 
-####Example:
+#### Example:
 
 ```
 0x94, [PlatformInit:__gPcd_BinPatch_FvRecOffset] + 0x94, [0x98], $COPY, @Sync up 2nd FSP Header
 ```
 
-###Comments:
+### Comments:
 Comments are allowed in the **Offset, Value [, Comment]** argument. Comments
 must use the **@** symbol as a prefix. The comment will output to the build
 window upon successful completion of patching along with the offset and value data.
-- 
2.39.1.windows.1
GitPatchExtractor 1.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111960): https://edk2.groups.io/g/devel/message/111960
Mute This Topic: https://groups.io/mt/102907649/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Reviewed-by: S, Ashraf Ali <ashraf.ali.s@intel.com>

Thanks.,
S, Ashraf Ali

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com> 
Sent: Friday, December 1, 2023 7:26 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Ng, Ray Han Lim <ray.han.lim.ng@intel.com>; Zeng, Star <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; Mohapatra, Susovan <susovan.mohapatra@intel.com>
Subject: [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual

From: Ray Ni <ray.ni@intel.com>

According to the markdown language syntax, headings should be after number signs (#). The number of number signs correspond to the heading level.
But current PatchFvUserManual.md doesn't insert a space between the number signs and the heading title, resulting the markdown file is not rendered well in markdown viewers.

The patch doesn't change any content but only adds spaces to ensure the headings are correctly recognized.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
Cc: Ray Han Lim Ng <ray.han.lim.ng@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ted Kuo <ted.kuo@intel.com>
Reviewed-by: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Susovan Mohapatra <susovan.mohapatra@intel.com>
---
 .../Tools/UserManuals/PatchFvUserManual.md    | 38 +++++++++----------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md b/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
index f28eedf625..205ad57773 100644
--- a/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
+++ b/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
@@ -1,9 +1,9 @@
-#Name
+# Name
 **_PatchFv.py_** - The python script that patches the firmware volumes (**FV**)  with in the flash device (**FD**) file post FSP build.
 From version 0.60, script is capable of patching flash device (**FD**) directly.
 
-#Synopsis
+# Synopsis
 
 ```
 PatchFv FvBuildDir [FvFileBaseNames:]FdFileBaseNameToPatch ["Offset, Value"]+ @@ -18,32 +18,32 @@ PatchFv FdFileDir FdFileName ["Offset, Value"]+
   | ["Offset, Value, $Command, @Comment"]+  ```
 
-#Description
+# Description
 The **_PatchFv.py_** tool allows the developer to fix up FD images to follow the  Intel FSP Architecture specification.  It also makes the FD image relocatable.
 The tool is written in Python and uses Python 2.7 or later to run.
 Consider using the tool in a build script.
 
-#FvBuildDir (Argument 1)
+# FvBuildDir (Argument 1)
 This is the first argument that **_PatchFv.py_** requires.  It is the build  directory for all firmware volumes created during the FSP build. The path must  be either an absolute path or a relevant path, relevant to the top level of the  FSP tree.
 
-####Example usage:
+#### Example usage:
 ```
  Build\YouPlatformFspPkg\%BD_TARGET%_%VS_VERSION%%VS_X86%\FV
 ```
 
 The example used contains Windows batch script %VARIABLES%.
 
-#FvFileBaseNames (Argument 2: Optional Part 1)
+# FvFileBaseNames (Argument 2: Optional Part 1)
 The firmware volume file base names (**_FvFileBaseNames_**) are the independent -Fv?s that are to be patched within the FD. (0 or more in the form
-**FVFILEBASENAME:**) The colon **:** is used for delimiting the single
+FVs that are to be patched within the FD. (0 or more in the form
+**FvFileBaseNames:**) The colon **:** is used for delimiting the single
 argument and must be appended to the end of each (**_FvFileBaseNames_**).
 
-####Example usage:
+#### Example usage:
 ```
 STAGE1:STAGE2:MANIFEST:YOURPLATFORM
 ```
@@ -55,14 +55,14 @@ In the example **STAGE1** is **STAGE1.Fv** in **YOURPLATFORM.fd**.
 Firmware device file name to patch (**_FdFileNameToPatch_**) is the base name of  the FD file that is to be patched. (1 only, in the form **YOURPLATFORM**)
 
-####Example usage:
+#### Example usage:
 ```
 STAGE1:STAGE2:MANIFEST:YOURPLATFORM
 ```
 
 In the example **YOURPLATFORM** is from **_YOURPLATFORM.fd_**
 
-#"Offset, Value[, Command][, Comment]" (Argument 3)
+# "Offset, Value[, Command][, Comment]" (Argument 3)
 The **_Offset_** can be a positive or negative number and represents where the
 **_Value_** to be patched is located within the FD. The **_Value_** is what  will be written at the given **_Offset_** in the FD. Constants may be used for @@ -79,10 +79,10 @@ The entire argument includes the quote marks like in the example argument below:
 0xFFFFFFC0, SomeCore:__EntryPoint - [0x000000F0],@SomeCore Entry  ```
 
-###Constants:
+### Constants:
  Hexadecimal (use **0x** as prefix) | Decimal
 
-####Examples:
+#### Examples:
 
 | **Positive Hex** | **Negative Hex** | **Positive Decimal** | **Negative Decimal** |  | ---------------: | ---------------: | -------------------: | -------------------: | @@ -93,7 +93,7 @@ ModuleName:FunctionName | ModuleName:GlobalVariableName  ModuleGuid:Offset  ```
 
-###Operators:
+### Operators:
 
 ```
 
@@ -113,7 +113,7 @@ From version 0.60 tool allows to pass flash device file path as Argument 1 and  flash device name as Argument 2 and rules for passing offset & value are same  as explained in the previous sections.
 
-####Example usage:
+#### Example usage:
 Argument 1
 ```
  YouPlatformFspBinPkg\
@@ -123,21 +123,21 @@ Argument 2
  Fsp_Rebased_T
 ```
 
-###Special Commands:
+### Special Commands:
 Special commands must use the **$** symbol as a prefix to the command itself.
 There is only one command available at this time.
 
 ```
-$COPY ? Copy a binary block from source to destination.
+$COPY   Copy a binary block from source to destination.
 ```
 
-####Example:
+#### Example:
 
 ```
 0x94, [PlatformInit:__gPcd_BinPatch_FvRecOffset] + 0x94, [0x98], $COPY, @Sync up 2nd FSP Header  ```
 
-###Comments:
+### Comments:
 Comments are allowed in the **Offset, Value [, Comment]** argument. Comments  must use the **@** symbol as a prefix. The comment will output to the build  window upon successful completion of patching along with the offset and value data.
--
2.39.1.windows.1
GitPatchExtractor 1.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111970): https://edk2.groups.io/g/devel/message/111970
Mute This Topic: https://groups.io/mt/102907649/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

The DXE & MM standalone variant of AcpiTimerLib defines a global
named mPerformanceCounterFrequency. A global with an identical
name is also present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c

Since XhciDxe has a dependency on TimerLib, this can cause link
errors due to the same symbol being defined twice if the platform
DSC chooses to use AcpiTimerLib as the TimerLib implementation for
any given platform.

To resolve this, I have changed made the definition of
mPerformanceCounterFrequency to STATIC. Since this variable is not
used outside of the DxeStandaloneMmAcpiTimerLib.c compilation unit,
there is no reason to have it exported as a global.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
 .../Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c        | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
index 16ac48938f..41d2af7d55 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
@@ -1,7 +1,7 @@
 /** @file
   ACPI Timer implements one instance of Timer Library.
 
-  Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2013 - 2023, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -51,7 +51,7 @@ InternalCalculateTscFrequency (
 //
 // Cached performance counter frequency
 //
-UINT64  mPerformanceCounterFrequency = 0;
+STATIC UINT64 mPerformanceCounterFrequency = 0;
 
 /**
   Internal function to retrieves the 64-bit frequency in Hz.
-- 
2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111962): https://edk2.groups.io/g/devel/message/111962
Mute This Topic: https://groups.io/mt/102907651/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Mike,
Does today's EDK2 C coding style spec allow using "STATIC" for global variables?
Or lower case "static"?
Or changing the variable to a name with lib name prefix, e.g.: " mTimerLibPerformanceCounterFrequency"?


Thanks,
Ray
> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Friday, December 1, 2023 9:56 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with
> XhciDxe
> 
> The DXE & MM standalone variant of AcpiTimerLib defines a global
> named mPerformanceCounterFrequency. A global with an identical
> name is also present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> 
> Since XhciDxe has a dependency on TimerLib, this can cause link
> errors due to the same symbol being defined twice if the platform
> DSC chooses to use AcpiTimerLib as the TimerLib implementation for
> any given platform.
> 
> To resolve this, I have changed made the definition of
> mPerformanceCounterFrequency to STATIC. Since this variable is not
> used outside of the DxeStandaloneMmAcpiTimerLib.c compilation unit,
> there is no reason to have it exported as a global.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> ---
>  .../Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c        | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> index 16ac48938f..41d2af7d55 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> +++
> b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>    ACPI Timer implements one instance of Timer Library.
> 
> -  Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2013 - 2023, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -51,7 +51,7 @@ InternalCalculateTscFrequency (
>  //
>  // Cached performance counter frequency
>  //
> -UINT64  mPerformanceCounterFrequency = 0;
> +STATIC UINT64 mPerformanceCounterFrequency = 0;
> 
>  /**
>    Internal function to retrieves the 64-bit frequency in Hz.
> --
> 2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111965): https://edk2.groups.io/g/devel/message/111965
Mute This Topic: https://groups.io/mt/102907651/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi Ray,

'static' is allowed and actually preferred for module globals 
that start with 'm' that are scoped to a single C file.

Globals that start with 'g' that need to be access by multiple
C files can not be static.

Since 'm' globals could be changed to 'g' globals due to
maintenance, we want to avoid symbol collisions between
'g' globals.  Prefixing all globals with a module name helps
prevent symbol collisions.

Summary
========
* Use lower case 'static'
* Use 'static' for 'm' globals
* Do not use 'static' for 'g' globals
* Add module/lib specific prefix to 'm' and 'g' global

Mike

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, November 30, 2023 7:13 PM
> To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>;
> devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility
> with XhciDxe
> 
> Mike,
> Does today's EDK2 C coding style spec allow using "STATIC" for global
> variables?
> Or lower case "static"?
> Or changing the variable to a name with lib name prefix, e.g.: "
> mTimerLibPerformanceCounterFrequency"?
> 
> 
> Thanks,
> Ray
> > -----Original Message-----
> > From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> > Sent: Friday, December 1, 2023 9:56 AM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with
> > XhciDxe
> >
> > The DXE & MM standalone variant of AcpiTimerLib defines a global
> > named mPerformanceCounterFrequency. A global with an identical
> > name is also present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> >
> > Since XhciDxe has a dependency on TimerLib, this can cause link
> > errors due to the same symbol being defined twice if the platform
> > DSC chooses to use AcpiTimerLib as the TimerLib implementation for
> > any given platform.
> >
> > To resolve this, I have changed made the definition of
> > mPerformanceCounterFrequency to STATIC. Since this variable is not
> > used outside of the DxeStandaloneMmAcpiTimerLib.c compilation unit,
> > there is no reason to have it exported as a global.
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > ---
> >  .../Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c        | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > index 16ac48938f..41d2af7d55 100644
> > --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > +++
> > b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    ACPI Timer implements one instance of Timer Library.
> >
> > -  Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
> > +  Copyright (c) 2013 - 2023, Intel Corporation. All rights reserved.<BR>
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -51,7 +51,7 @@ InternalCalculateTscFrequency (
> >  //
> >  // Cached performance counter frequency
> >  //
> > -UINT64  mPerformanceCounterFrequency = 0;
> > +STATIC UINT64 mPerformanceCounterFrequency = 0;
> >
> >  /**
> >    Internal function to retrieves the 64-bit frequency in Hz.
> > --
> > 2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111966): https://edk2.groups.io/g/devel/message/111966
Mute This Topic: https://groups.io/mt/102907651/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Mike,
Thanks!

Thanks,
Ray
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Friday, December 1, 2023 12:39 PM
> To: Ni, Ray <ray.ni@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with
> XhciDxe
> 
> Hi Ray,
> 
> 'static' is allowed and actually preferred for module globals
> that start with 'm' that are scoped to a single C file.
> 
> Globals that start with 'g' that need to be access by multiple
> C files can not be static.
> 
> Since 'm' globals could be changed to 'g' globals due to
> maintenance, we want to avoid symbol collisions between
> 'g' globals.  Prefixing all globals with a module name helps
> prevent symbol collisions.
> 
> Summary
> ========
> * Use lower case 'static'
> * Use 'static' for 'm' globals
> * Do not use 'static' for 'g' globals
> * Add module/lib specific prefix to 'm' and 'g' global
> 
> Mike
> 
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: Thursday, November 30, 2023 7:13 PM
> > To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>;
> > devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility
> > with XhciDxe
> >
> > Mike,
> > Does today's EDK2 C coding style spec allow using "STATIC" for global
> > variables?
> > Or lower case "static"?
> > Or changing the variable to a name with lib name prefix, e.g.: "
> > mTimerLibPerformanceCounterFrequency"?
> >
> >
> > Thanks,
> > Ray
> > > -----Original Message-----
> > > From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> > > Sent: Friday, December 1, 2023 9:56 AM
> > > To: devel@edk2.groups.io
> > > Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Subject: [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with
> > > XhciDxe
> > >
> > > The DXE & MM standalone variant of AcpiTimerLib defines a global
> > > named mPerformanceCounterFrequency. A global with an identical
> > > name is also present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > >
> > > Since XhciDxe has a dependency on TimerLib, this can cause link
> > > errors due to the same symbol being defined twice if the platform
> > > DSC chooses to use AcpiTimerLib as the TimerLib implementation for
> > > any given platform.
> > >
> > > To resolve this, I have changed made the definition of
> > > mPerformanceCounterFrequency to STATIC. Since this variable is not
> > > used outside of the DxeStandaloneMmAcpiTimerLib.c compilation unit,
> > > there is no reason to have it exported as a global.
> > >
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > > ---
> > >  .../Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c        | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git
> > > a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > > b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > > index 16ac48938f..41d2af7d55 100644
> > > ---
> a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > > +++
> > > b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > > @@ -1,7 +1,7 @@
> > >  /** @file
> > >    ACPI Timer implements one instance of Timer Library.
> > >
> > > -  Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
> > > +  Copyright (c) 2013 - 2023, Intel Corporation. All rights reserved.<BR>
> > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -51,7 +51,7 @@ InternalCalculateTscFrequency (
> > >  //
> > >  // Cached performance counter frequency
> > >  //
> > > -UINT64  mPerformanceCounterFrequency = 0;
> > > +STATIC UINT64 mPerformanceCounterFrequency = 0;
> > >
> > >  /**
> > >    Internal function to retrieves the 64-bit frequency in Hz.
> > > --
> > > 2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111967): https://edk2.groups.io/g/devel/message/111967
Mute This Topic: https://groups.io/mt/102907651/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

The DXE & MM standalone variant of AcpiTimerLib defines a global
named mPerformanceCounterFrequency. A global with an identical
name is also present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c

Since XhciDxe has a dependency on TimerLib, this can cause link
errors due to the same symbol being defined twice if the platform
DSC chooses to use AcpiTimerLib as the TimerLib implementation for
any given platform.

To resolve this, I noted that some of the globals in Xhci.c are not
used outside of the Xhci.c compilation unit:

- mPerformanceCounterStartValue
- mPerformanceCounterEndValue
- mPerformanceCounterFrequency
- mPerformanceCounterValuesCached

I have changed the definition for all of these to STATIC. Since they
are not used outside of the Xhci.c compilation unit, there is no
reason to have them exported as globals.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 7a2e32a9dd..cc8332500b 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -2,7 +2,7 @@
   The XHCI controller driver.
 
 (C) Copyright 2023 Hewlett Packard Enterprise Development LP<BR>
-Copyright (c) 2011 - 2022, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -86,10 +86,10 @@ EFI_USB2_HC_PROTOCOL  gXhciUsb2HcTemplate = {
   0x0
 };
 
-UINT64   mPerformanceCounterStartValue;
-UINT64   mPerformanceCounterEndValue;
-UINT64   mPerformanceCounterFrequency;
-BOOLEAN  mPerformanceCounterValuesCached = FALSE;
+STATIC UINT64   mPerformanceCounterStartValue;
+STATIC UINT64   mPerformanceCounterEndValue;
+STATIC UINT64   mPerformanceCounterFrequency;
+STATIC BOOLEAN  mPerformanceCounterValuesCached = FALSE;
 
 /**
   Retrieves the capability of root hub ports.
-- 
2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111961): https://edk2.groups.io/g/devel/message/111961
Mute This Topic: https://groups.io/mt/102907650/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-