* [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual
@ 2023-12-01 1:56 Nate DeSimone
2023-12-01 1:56 ` [edk2-devel] [PATCH v1] MdeModulePkg/Bus: Fix XhciDxe Linker Issues Nate DeSimone
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Nate DeSimone @ 2023-12-01 1:56 UTC (permalink / raw)
To: devel
Cc: Ray Ni, Chasel Chiu, Duggapu Chinni B, Ray Han Lim Ng, Star Zeng,
Ted Kuo, Ashraf Ali S, Susovan Mohapatra
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [edk2-devel] [PATCH v1] MdeModulePkg/Bus: Fix XhciDxe Linker Issues
2023-12-01 1:56 [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual Nate DeSimone
@ 2023-12-01 1:56 ` Nate DeSimone
2023-12-01 1:56 ` [edk2-devel] [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe Nate DeSimone
2023-12-01 6:09 ` [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual Ashraf Ali S
2 siblings, 0 replies; 7+ messages in thread
From: Nate DeSimone @ 2023-12-01 1:56 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Michael D Kinney
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [edk2-devel] [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
2023-12-01 1:56 [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual Nate DeSimone
2023-12-01 1:56 ` [edk2-devel] [PATCH v1] MdeModulePkg/Bus: Fix XhciDxe Linker Issues Nate DeSimone
@ 2023-12-01 1:56 ` Nate DeSimone
2023-12-01 3:13 ` Ni, Ray
2023-12-01 6:09 ` [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual Ashraf Ali S
2 siblings, 1 reply; 7+ messages in thread
From: Nate DeSimone @ 2023-12-01 1:56 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Michael D Kinney
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
2023-12-01 1:56 ` [edk2-devel] [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe Nate DeSimone
@ 2023-12-01 3:13 ` Ni, Ray
2023-12-01 4:39 ` Michael D Kinney
0 siblings, 1 reply; 7+ messages in thread
From: Ni, Ray @ 2023-12-01 3:13 UTC (permalink / raw)
To: Desimone, Nathaniel L, devel@edk2.groups.io; +Cc: Kinney, Michael D
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
2023-12-01 3:13 ` Ni, Ray
@ 2023-12-01 4:39 ` Michael D Kinney
2023-12-01 5:24 ` Ni, Ray
0 siblings, 1 reply; 7+ messages in thread
From: Michael D Kinney @ 2023-12-01 4:39 UTC (permalink / raw)
To: Ni, Ray, Desimone, Nathaniel L, devel@edk2.groups.io; +Cc: Kinney, Michael D
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
2023-12-01 4:39 ` Michael D Kinney
@ 2023-12-01 5:24 ` Ni, Ray
0 siblings, 0 replies; 7+ messages in thread
From: Ni, Ray @ 2023-12-01 5:24 UTC (permalink / raw)
To: Kinney, Michael D, Desimone, Nathaniel L, devel@edk2.groups.io
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual
2023-12-01 1:56 [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual Nate DeSimone
2023-12-01 1:56 ` [edk2-devel] [PATCH v1] MdeModulePkg/Bus: Fix XhciDxe Linker Issues Nate DeSimone
2023-12-01 1:56 ` [edk2-devel] [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe Nate DeSimone
@ 2023-12-01 6:09 ` Ashraf Ali S
2 siblings, 0 replies; 7+ messages in thread
From: Ashraf Ali S @ 2023-12-01 6:09 UTC (permalink / raw)
To: Desimone, Nathaniel L, devel@edk2.groups.io
Cc: Ni, Ray, Chiu, Chasel, Duggapu, Chinni B, Ng, Ray Han Lim,
Zeng, Star, Kuo, Ted, Mohapatra, Susovan
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-12-01 6:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-01 1:56 [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual Nate DeSimone
2023-12-01 1:56 ` [edk2-devel] [PATCH v1] MdeModulePkg/Bus: Fix XhciDxe Linker Issues Nate DeSimone
2023-12-01 1:56 ` [edk2-devel] [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe Nate DeSimone
2023-12-01 3:13 ` Ni, Ray
2023-12-01 4:39 ` Michael D Kinney
2023-12-01 5:24 ` Ni, Ray
2023-12-01 6:09 ` [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual Ashraf Ali S
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox