Thanks a lot Mike for your detail reviewing and providing better implementation suggestions!
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D
> Kinney
> Sent: Wednesday, May 10, 2023 9:00 AM
> To: Guo, Gua <gua.guo@intel.com>; devel@edk2.groups.io; Gao, Liming
> <gaoliming@byosoft.com.cn>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v5 0/4] MdePkg: Add MipiSysTLib library
>
> Series Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
>
> Liming, this code review started well before the soft freeze. It has now passed
> review.
>
> We should include this in this stable-tag release.
>
> Mike
>
>
> > -----Original Message-----
> > From: Guo, Gua <gua.guo@intel.com>
> > Sent: Wednesday, May 10, 2023 2:20 AM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Guo, Gua
> > <gua.guo@intel.com>
> > Subject: [PATCH v5 0/4] MdePkg: Add MipiSysTLib library
> >
> > From: Gua Guo <gua.guo@intel.com>
> >
> > V5: if no other open, it will be final change
> > - https://github.com/tianocore/edk2/pull/3901
> > Fix random exception when long run catalog debug message
> >
> > V4
> > - https://github.com/tianocore/edk2/pull/3901 - Done
> > Enhance SwapBytesGuid to use CopyGuid instead of CopyMem, to make
> > implement code more simple.
> >
> > V3
> > - https://github.com/tianocore/edk2/pull/3901 - Done
> > - Open: MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h: why
> > MAX_TRACE_HUB_DEBUG_INSTANCE hardcoded to 5?
> > Solution: Remove this macro, use Library Constructor to allocate
> > it dynamiclly.
> > - Open:
> > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon
> > .c: SwapBytesGuid () algorithm wrong.
> > Solution: Follow correct algorithm to implement it.
> > VOID
> > EFIAPI
> > SwapBytesGuid (
> > IN GUID *Guid, <----------- In PreMem, guid is global data so region
> > is readonly, add output data to support it.
> > OUT GUID *ConvertedGuid
> > );
> >
> > - Open: Merge MSFT and GCC CC_FLAGS as they both supports -D
> > Solution: use *_*_*_CC_FLAGS = -DMIPI_SYST_STATIC to unified both.
> >
> >
> > V2
> > - https://github.com/tianocore/edk2/pull/3901
> > - Open: MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h: why
> > MAX_TRACE_HUB_DEBUG_INSTANCE hardcoded to 5?
> > - Open:
> > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon
> > .c: SwapBytesGuid () algorithm wrong.
> > - Open: Merge MSFT and GCC CC_FLAGS as they both supports -D
> >
> > V1
> > Previous PR:
> > - https://github.com/tianocore/edk2/pull/3613
> > - TraceHubDebugLib without submodule - Reject
> >
> > - https://github.com/tianocore/edk2/pull/3793
> > - TraceHubDebugLib with submodule and without seperate into
> > MipiSysTLib and TraceHubDebugLib - Reject
> >
> > Gua Guo (4):
> > MdePkg: Add MipiSysTLib library
> > MdePkg: Add NULL library of TraceHubDebugSysTLib
> > MdeModulePkg: Add TraceHubDebugSysTLib library
> > Maintainers.txt: Update reviewers and maintainers for
> > TraceHubDebugLib.
> >
> > .gitmodules | 11 +-
> > .pytool/CISettings.py | 2 +
> > Maintainers.txt | 18 +
> > .../Include/Guid/TraceHubDebugInfoHob.h | 24 +
> > .../BaseTraceHubDebugSysTLib.c | 245 ++++++
> > .../BaseTraceHubDebugSysTLib.inf | 44 +
> > .../DxeSmmTraceHubDebugSysTLib.c | 263 ++++++
> > .../DxeSmmTraceHubDebugSysTLib.inf | 51 ++
> > .../InternalTraceHubApi.c | 74 ++
> > .../InternalTraceHubApi.h | 37 +
> > .../InternalTraceHubApiCommon.c | 200 +++++
> > .../InternalTraceHubApiCommon.h | 119 +++
> > .../PeiTraceHubDebugSysTLib.c | 282 +++++++
> > .../PeiTraceHubDebugSysTLib.inf | 50 ++
> > .../Library/TraceHubDebugSysTLib/Readme.md | 26 +
> > MdeModulePkg/MdeModulePkg.dec | 21 +
> > MdeModulePkg/MdeModulePkg.dsc | 3 +
> > MdeModulePkg/MdeModulePkg.uni | 18 +
> > MdePkg/Include/Library/MipiSysTLib.h | 66 ++
> > MdePkg/Include/Library/TraceHubDebugSysTLib.h | 81 ++
> > MdePkg/Library/MipiSysTLib/GenMipiSystH.py | 132 +++
> > MdePkg/Library/MipiSysTLib/MipiSysTLib.c | 123 +++
> > MdePkg/Library/MipiSysTLib/MipiSysTLib.inf | 52 ++
> > MdePkg/Library/MipiSysTLib/Platform.c | 164 ++++
> > MdePkg/Library/MipiSysTLib/Platform.h | 138 +++
> > MdePkg/Library/MipiSysTLib/Readme.md | 25 +
> > MdePkg/Library/MipiSysTLib/mipi_syst.h | 789 ++++++++++++++++++
> > MdePkg/Library/MipiSysTLib/mipisyst | 1 +
> > .../TraceHubDebugSysTLibNull.c | 76 ++
> > .../TraceHubDebugSysTLibNull.inf | 29 +
> > MdePkg/MdePkg.ci.yaml | 12 +-
> > MdePkg/MdePkg.dec | 9 +
> > MdePkg/MdePkg.dsc | 2 +
> > ReadMe.rst | 1 +
> > 34 files changed, 3181 insertions(+), 7 deletions(-) create mode
> > 100644 MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h
> > create mode 100644
> > MdeModulePkg/Library/TraceHubDebugSysTLib/BaseTraceHubDebugSysTLib.c
> > create mode 100644
> > MdeModulePkg/Library/TraceHubDebugSysTLib/BaseTraceHubDebugSysTLib.i
> > nf
> > create mode 100644
> >
> MdeModulePkg/Library/TraceHubDebugSysTLib/DxeSmmTraceHubDebugSysTL
> > ib.c
> > create mode 100644
> >
> MdeModulePkg/Library/TraceHubDebugSysTLib/DxeSmmTraceHubDebugSysTL
> > ib.inf
> > create mode 100644
> > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApi.c
> > create mode 100644
> > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApi.h
> > create mode 100644
> > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon
> > .c
> > create mode 100644
> > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon
> > .h
> > create mode 100644
> > MdeModulePkg/Library/TraceHubDebugSysTLib/PeiTraceHubDebugSysTLib.c
> > create mode 100644
> > MdeModulePkg/Library/TraceHubDebugSysTLib/PeiTraceHubDebugSysTLib.inf
> > create mode 100644
> > MdeModulePkg/Library/TraceHubDebugSysTLib/Readme.md
> > create mode 100644 MdePkg/Include/Library/MipiSysTLib.h
> > create mode 100644 MdePkg/Include/Library/TraceHubDebugSysTLib.h
> > create mode 100644 MdePkg/Library/MipiSysTLib/GenMipiSystH.py
> > create mode 100644 MdePkg/Library/MipiSysTLib/MipiSysTLib.c
> > create mode 100644 MdePkg/Library/MipiSysTLib/MipiSysTLib.inf
> > create mode 100644 MdePkg/Library/MipiSysTLib/Platform.c
> > create mode 100644 MdePkg/Library/MipiSysTLib/Platform.h
> > create mode 100644 MdePkg/Library/MipiSysTLib/Readme.md
> > create mode 100644 MdePkg/Library/MipiSysTLib/mipi_syst.h
> > create mode 160000 MdePkg/Library/MipiSysTLib/mipisyst
> > create mode 100644
> > MdePkg/Library/TraceHubDebugSysTLibNull/TraceHubDebugSysTLibNull.c
> > create mode 100644
> > MdePkg/Library/TraceHubDebugSysTLibNull/TraceHubDebugSysTLibNull.inf
> >
> > --
> > 2.39.2.windows.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104584): https://edk2.groups.io/g/devel/message/104584
Mute This Topic: https://groups.io/mt/98802140/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-