Re: [edk2-devel] [PATCH v1] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe

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

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]
-=-=-=-=-=-=-=-=-=-=-=-