Hi Sunil, I agree with your most of comments, and need some time to do the change. There are some opens below. 3) In the latest series, I see you have 2 commits where first commit adds .inc file and in the next commit itself it gets removed. I see no reason to do so. [Evan] I marked this change in commit log 'Unified .dsc files into a single' 4) Why would .inc files cause fragmentation? Rather, I think a common .inc (qemu and real servers) will reduce the effort and copying same for several platforms. [Evan] I acknowledge that the .inc files have extracted many common parts and reduced some porting effort. However, this approach has not been thoroughly implemented: 1. As I remember, some libraries are defined in multiple files; 2. Several libraries that should belong to the common part are still in the .dsc files for unknown reasons, such as CapsuleLib, BootLogoLib, FrameBufferBltLib, TPM, and so on. Given the unclear separation, it might be better not to implement it at all. The SBSA dsc file follows the traditional approach, and it doesn't seem to cause much inconvenience in usage. Additionally, what worries me is that many firmware engineers are not clear about which parts are common (platform-independent) and which are not. This separation approach might confuse them and make the codebase more chaotic. Anyway, I will approach this issue with an open mind and welcome any additional suggestions at any time. BR, Evan ------------------------------------------------------------------ 发件人：Sunil V L 发送时间：2024年7月10日(星期三) 22:33 收件人：devel; "evan.chai" 主　题：Re: [edk2-devel] 转发：[edk2-devel] [PATCH V2 0/2] Initial commit for RISC-V Qemu-based Server Hi Evan, On Wed, Jul 10, 2024 at 02:14:21AM -0700, EvanChai via groups.io wrote: > Hi Sunil, > I just saw two comments below. > 'These images are generic. I don't think we need to have a copy here. Ifyou need to refer to this in any documentation, please add the link tooriginal image/document.' > ‘It is better to split these documentation changes to a separate commit.’ > Oh OK. At high level, here is my feedback. 1) It is wonderful that you have added lot of details including design of qemu, todo etc. However, I think it is better to keep Readme simple and provide a reference to RISE wiki page where you can add all these details. That way, we can avoid having stale data in Readme or need to do several patches for updates. Better to keep readme with just build and test instructions and a link to more details. For example, adding qemu design (ex: memory map) in edk2 readme. Every time qemu design changes, the edk2 readme becomes stale and keeping them in sync may not be productive. 2) Please split documentation changes (readme) and platform changes (dsc etc) to separate commits. 3) In the latest series, I see you have 2 commits where first commit adds .inc file and in the next commit itself it gets removed. I see no reason to do so. 4) Why would .inc files cause fragmentation? Rather, I think a common .inc (qemu and real servers) will reduce the effort and copying same for several platforms. 5) I have few more comments on the contents of readme as well. But if it can be moved to wiki, we can discuss and correct there. > BTW, the only uploaded picture comes from my presentation on the RV server platform proposal. It doesn't have an online source because I drew it myself. Do you have any recommended places where I can upload it? Then we can just reference its link in the README. > My recommendation is to keep them in RISE wiki for now. Thanks, Sunil -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119900): https://edk2.groups.io/g/devel/message/119900 Mute This Topic: https://groups.io/mt/107161977/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-