From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by mx.groups.io with SMTP id smtpd.web12.10901.1586351079842759323 for ; Wed, 08 Apr 2020 06:04:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=i18/s/J8; spf=pass (domain: nuviainc.com, ip: 209.85.128.68, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f68.google.com with SMTP id e26so5048925wmk.5 for ; Wed, 08 Apr 2020 06:04:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=0e2+mxQjRc3gZtanh+wc9MuhYWTRaXCqeCgXCs/s4Io=; b=i18/s/J8u7hhYxMsfTVwL60TDdSx3CwlKhxttC3o+j64p8LaT2lz4tZsOtq/WxQNOJ ZyL9nysJ2hLBNjW9NDs2qZOFGbiM49yCKjMfC9MP7RnPWrJo1XHwZp2+HzW0e0u+MEep MaOs1mJQu6WGHtKOfmkABc7hX3TjwARoxtxR9s66C1pifiSVoN4HDnGwbC40NN3hWhap VenpQJZg4aI5oz0yjgWSEbx7vFliPbQMy0RrAMykmZIL0GkhxHgF2LZgwGzlDDTyOUcz abA8VyxG6Sn0Kcfg3AABn3OqGS2F7KIEXcYwUurV+z/os2n3Uvkf8DSm4xa3o3JllmQX 0uow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=0e2+mxQjRc3gZtanh+wc9MuhYWTRaXCqeCgXCs/s4Io=; b=UdyCUO7FF+NGAa69w0bIZEq/NYzfb/0lBTXGWV718jGXgxbY/x49x/waG/8HsL34Js ZBWlaWdj1uwmnXDkE2FVlPBQgwsk65bBJ7hPx9+vHJch/Y/F5SflNh8yTvsupKeQInc9 4ZNciyt0fed4vBNwRzgVCFlwpyJhNFyJsjN0thIqsDr05mUk6n0p6usyi4aoVnFsc5cB XmQaX1EYvd4minTmlTdynZ2jEKW1n5rErlvy1lfGd7Bz8Q+BzYKqoU3Vrc/due76OOm6 lqOxn8Fj5DjN6C9ETeR+tmq2p4amwYFOsSMNAvTZuh+S6wmDeW7w7lx1NwJf039QPKFE dvmA== X-Gm-Message-State: AGi0PubTuHeKzclMyCiYGcdYq3yDucNY6GYaubqmQ7xTqPCxoOussgOe wlSZCNp4ggNEFSbsgh/E8I3D+/iuAmru0sXMF6mk1AAob7uhap95wPIq6gIjRP8L08pF2/m6pen VJQSH5No84cxtzD0qNokqAyW1RJpnMKbZP8uLwC+EXWX+NgUCQ2ou4wNoA+Xxz6A= X-Google-Smtp-Source: APiQypJEIN0IAFT5anb2yRF0wOl6/BJEC5ogxm5kfZDWc58J6y8MfcJt5ETaUEszoHmYI5DHaxQyZw== X-Received: by 2002:a1c:bd8b:: with SMTP id n133mr4777915wmf.175.1586351078122; Wed, 08 Apr 2020 06:04:38 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id u17sm7111682wmu.31.2020.04.08.06.04.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Apr 2020 06:04:37 -0700 (PDT) Date: Wed, 8 Apr 2020 14:04:35 +0100 From: "Leif Lindholm" To: devel@edk2.groups.io, pankaj.bansal@nxp.com Cc: "Pankaj Bansal (OSS)" , Meenakshi Aggarwal , Michael D Kinney , Varun Sethi , Samer El-Haj-Mahmoud , Jon Nettleton Subject: Re: [edk2-devel] [PATCH v2 21/28] Slicon/NXP: Add PlatformPei Lib Message-ID: <20200408130435.GU14075@vanye> References: <20200320143543.18615-1-pankaj.bansal@oss.nxp.com> <20200320143543.18615-22-pankaj.bansal@oss.nxp.com> <20200401145324.GD7468@vanye> <20200407125359.GO14075@vanye> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Apr 07, 2020 at 17:00:50 +0000, Pankaj Bansal wrote: > > OK, taking another look at this patch, this simply needs to be > > deleted. Here is the sum total relevant difference compared to the > > ArmPlatformPkg one. > > > > DEBUG ((DEBUG_INIT, "Edk2 version is %a\n", XPRINT > > (WORKSPACE_GIT_VERSION))); > > DEBUG ((DEBUG_INIT, "Edk2 platforms version is %a\n", XPRINT > > (PACKAGES_PATH_GIT_VERSION))); > > > > If all you want to do is to print that sort of thing, please don't > > fork a core library to do so. > > Not just the commit info. The plan is to print core/cluster info also from > this peim. As I have already stated in PATCH 10/28, that some print information would > be put into common code instead of SOC specific code. Sure, but there are 11 patches in between. There is no way I am going to recall what you stated was going to happen to something as basic as a DEBUG printout by the time I get here. And that comment stated "a common PEIM", not "a forked PlatformPeiLib containing only the printouts". > In fact the further plan is to fixup device tree in this library > before DXE phase starts. So, this fork is needed. Not at this time. See below. > > First of all, please do like most other platforms and override > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString > > if -D FIRMWARE_VER is specified on your build command line. > > > > You can then extract current top commits of your respective > > repositories and not worry about getting this. > > I would suggest iterating across all locations in PACKAGES_PATH and > > then doing something similar to > > https://git.linaro.org/uefi/uefi-tools.git/tree/edk2-build.sh#n400 > > appending together. > > > > OK, I can use this for printing git commit info. > > > If something like this should be integrated into the build system > > (which might not be a bad idea), then it needs to be so properly, > > rather than shoehorned in for each platform. > > I agree. Actually IMO, best place would be python script to do this. > I don't have much experience with python script. > That is why I made changes in shell script. Yes, but edk2 is a cross-os project, so Python is needed. To clarify, I think there shouldn't be a separate script to call, but that this should be integrated into the build command itself, and accessible as other predefined variables (like $ARCH). If you would like to raise a feature request in https://bugzilla.tianocore.org/enter_bug.cgi?product=EDK2, that would be a good start even if you're not able to do the work yourself. > > (In the past, this was difficult because we supported both git and > > svn, but I would say we have given up pretending that is possible.) > > > > For now, you could add the printout in a standalone Depex TRUE PEIM > > added to your [FV.FVMAIN_COMPACT]. > > As I said, that version print is just one of the many things that we wish to > do in platform PEIM. OK, but it makes no sense to split something out *now* to be given actual functionality at some later date. Please leave it out for now, either completely or moving the printout to some standalone Peim, and add it in when there is actual content. At that point, please give it a unique FILE_GUID. / Leif > > > > / > > Leif > > > > On Mon, Apr 06, 2020 at 14:53:02 +0000, Pankaj Bansal (OSS) wrote: > > > >