Mike, Thanks! Thanks, Ray > -----Original Message----- > From: Kinney, Michael D <michael.d.kin...@intel.com> > Sent: Friday, December 1, 2023 12:39 PM > To: Ni, Ray <ray...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kin...@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...@intel.com> > > Sent: Thursday, November 30, 2023 7:13 PM > > To: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; > > devel@edk2.groups.io > > Cc: Kinney, Michael D <michael.d.kin...@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.desim...@intel.com> > > > Sent: Friday, December 1, 2023 9:56 AM > > > To: devel@edk2.groups.io > > > Cc: Ni, Ray <ray...@intel.com>; Kinney, Michael D > > > <michael.d.kin...@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...@intel.com> > > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > > Signed-off-by: Nate DeSimone <nathaniel.l.desim...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-