Hi,
> > Hmm. Unlike patches 17+18 which are pure code motion (except the
> > function renaming but that doesn't change the workflow) this patch mixes
> > code changes and code moving which makes it hard to review.
> >
> > It should be splitted into one (or more) patches changing the functions as
> > needed (and keeping the code in PlatformPei), and one patch moving things
> > over to PlatformInitLib without functional changes.
> Ok. Looks like #21 & #22 in tdvf_wave2.v6?
> https://github.com/mxu9/edk2/commit/ef0615ca5665b2058e4352a322dfa74d258f9f31
> https://github.com/mxu9/edk2/commit/25f356a0bf7ee347be30e270aeffe6cbd8e0b464
No. The idea is to changes to the code in PlatformPei with small &
one-patch-per-update patches, which allow easy review. Also helps
debugging in case something go wrong, when bisecting found the broken
patch it's *much* easier to find the actual bug when the patch is small.
Rough plan:
(1) a patch allocating PLATFORM_INFO struct in PlatformPei.
(2) one or more patches moving global variables into PLATFORM_INFO
struct.
(3) one or more patches restructing functions. Stuff like like
splitting functions which set PCDs into two, one for
PlatformInitLib and one for PlatformPei.
Final step is a pure move from PlatformPei to PlatformInitLib without
changing code.
> > > + // Fetch the lower memory size (Below 4G) // mLowerMemorySize =
> > > + PlatformGetSystemMemorySizeBelow4gb ();
> This is in function InitializePlatform().
> >
> > Can't you just use TopOfLowRam here?
> TopOfLowRam is a local variable in function MemMapInitialization(). It cannot
> be used in function InitialziePlatform().
Ah, didn't notice it is another function. High time to introduce
PLATFORM_INFO->TopOfLowRam ;)
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87218): https://edk2.groups.io/g/devel/message/87218
Mute This Topic: https://groups.io/mt/89446168/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
