On 05/29/15 17:54, Olivier Martin wrote: > For the community information, I commented Eugene’s patch into Github > (https://github.com/tianocore/edk2/pull/10) 3 days ago. > > As I told Eugene there is no chance his pull-request would be accepted > as EDK2 project is not supported through github. But once I would be > happy with his patch I would be happy to commit them into SVN. > > To come back on the current discussion about code review. Github is not > the prefect tool to review patches because it is difficult to compare > changes between pull-request (or at least it was not last time I tried). > But email code review is not better on this point neither. And Gerrit do > it pretty well.
I compare v(n) patchsets against v(n+1) all the time. I apply both to separate local branches that are forked from the same master branch commit. Commits (trees) that constitute the same "stage" of the series in each branch can be easily diffed against each other. I also tend to format the individual patch files for both v(n) and v(n+1), and compare the corresponding patch files across the versions with plain "diff". (Well, "colordiff" to be precise, and a number of shell utilities.) It gives an interdiff-like result (two columns of +/- at the left). Its interpretation is different from that of "git-diff for merges" (*), but it's not hard to understand. Some of Ard's early series went through many iterations, and this was a very efficient way to focus on the changes. ((*) "git-diff for merges" (= combined diff) assigns each column to a separate parent. Whereas diffing patch files gives a diff of diffs.) Thanks Laszlo > > > > > > *From:*Cohen, Eugene [mailto:eug...@hp.com] > *Sent:* 16 May 2015 16:03 > *To:* edk2-devel@lists.sourceforge.net > *Subject:* [edk2] [PATCH] ArmPlatformPkg: Fix boot on FVP with > ArmPlatformPkg SEC at EL3 > > > > Olivier (or if you prefer “Dear ArmPkg and ArmPlatformPkg maintainer”), > > > > Here are three patches (in addition to the one I sent earlier) to fix > issues I encountered booting the FVP Base platform using SEC from > ArmPlatformPkg instead of Trusted Firmware. This boot path is still one > where SEC runs at EL3 and the remaining boot stages run at EL2/EL1. > > > > As a preface to these patch sets I should add that I’m unclear about the > structure of the ARM platforms. From a directory naming perspective I > see an ArmVExpressPkg which supports an FVP-AArch64 platform but I don’t > know whether this platform is intended to support both the FVP_Base > platforms as well as the FVP_VE (Versatile Express) platforms. There > are significant differences between these platforms, for example the GIC > revision, the memory maps and peripherals are all different. As such > it’s not clear whether I’m making changes in the right place but that > being said I am able to boot to the EFI Shell after these few fixes. > Olivier, please advise what the proper way is to handle Versatile > Express and Base platform differences. > > > > The fixes break down as follows: > > 1. The intent on the FVP is to use main memory at 0x880000000 but > a PCD value causes PEI and the DXE core to be loaded at a a sub-4GB > address. This patch brings all of the DRAM usage into the same area. > > 2. The architectural timer was not advancing because the FVP > Reference Counter, which feeds the processor’s timer, must be started by > EL3 firmware on the FVP Base platforms > > 3. Interrupts were not being delivered to the core because EL3 > firmware must clear the ProcessorSleep bit in the GICv3 redistributor. > > > > I’ve been trying to use the github development model for this so I’ve > created a pull request with these changes for your convenience: > https://github.com/tianocore/edk2/pull/10 . This tool has a > significantly better code review capability than what is possible on a > mailing list. > > > > (My next efforts will be around extending EL3 into DXE to support > SMM-style initialization but that’s a future subject.) > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Eugene Cohen <eug...@hp.com <mailto:eug...@hp.com>> > > > > > -- IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy > the information in any medium. Thank you. > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2557590 > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 > 9NJ, Registered in England & Wales, Company No: 2548782 > > > ------------------------------------------------------------------------------ > > > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
