public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Pankaj Bansal <pankaj.bansal@nxp.com>
To: "Kinney, Michael D" <michael.d.kinney@intel.com>,
	Laszlo Ersek <lersek@redhat.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Gao, Liming" <liming.gao@intel.com>
Subject: Re: [RFC] Add Platform Include path in modules
Date: Tue, 27 Feb 2018 07:39:42 +0000	[thread overview]
Message-ID: <AM0PR0402MB394093EF8C1F239036F29725F1C00@AM0PR0402MB3940.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <E92EE9817A31E24EB0585FDF735412F5B896C4FD@ORSMSX113.amr.corp.intel.com>

Hi Laszlo/Michael,

Thanks for your feedback on this proposal.
I looked at the structured PCDs and UEFI Platform Initialization Distribution Packaging Specification.
Here is my take on these.

1. structured PCDs are good if we want to declare single complex structure.
    But consider a case where I want to keep device information in structure. (e.g. hardware settings, limitations etc)
    And we may want to tweak this information based on platform revision being used.
    And different platforms can have different number of such devices.
    In this case, when we want to add a new platform, we might need to introduce new PCDs in .dec files, which will not be needed for others.
    I don't know, will this even increase the PCD database size for existing platforms or not ?

2. To mitigate the "hidden" dependency of a module on platform, we can explicitly declare this dependency in module inf file.
     I am thinking something like gEfiCallerIdGuid, i.e. module can declare that that platform building(using) the module, supply this information.

3. Using Libraries and Protocols can also solve such use cases. I just felt that it's less cumbersome to use include files, and it also avoids code replication.
    Anyway, this is just my suggestion to have such mechanism in edk2 build process. I am more than happy to stick to platform libraries.

Thanks & Regards,
Pankaj Bansal

> -----Original Message-----
> From: Kinney, Michael D [mailto:michael.d.kinney@intel.com]
> Sent: Monday, February 26, 2018 10:56 PM
> To: Laszlo Ersek <lersek@redhat.com>; Pankaj Bansal
> <pankaj.bansal@nxp.com>; edk2-devel@lists.01.org; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [RFC] Add Platform Include path in modules
> 
> Hi Pankaj,
> 
> I agree with Laszlo that you should evaluate use of PCDs.  There are a few
> methods for a driver to use platform specific values/behavior.  These are:
> 
> * PCDs
> * Library class/Library Instance
> * Protocol/PPI
> 
> One issue with the proposal is that it adds a hidden dependency to modules.
> An EDK II INF file describes the external interfaces of a module along with
> produces/consumes usage.  This information is aligned with the XML schema
> that is documented in the UEFI Platform Initialization Distribution Packaging
> Specification.
> 
> 
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fuefi.
> org%2Fspecifications&data=02%7C01%7Cpankaj.bansal%40nxp.com%7C96c4
> 2dd7271d449d56e908d57d3df2d4%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C636552627376357192&sdata=sS7KT3haANF5TLHSeRGjIQHnLQ
> BtnTDLIZWntUhsk78%3D&reserved=0
> 
> If two modules have the same GUID/Version, then the external interfaces to
> those two modules are expected to be identical.
> With your proposal, two modules built for 2 different platforms would have
> the same GUID/Version but would not have the same external interfaces
> because a hidden dependency on a platform package was added.
> 
> If a module really needs to use content from a platform package, then a new
> copy of the module should be created with a new GUID/Version and the
> platform package added to the [Packages] section.  The other option is to use
> one of the supported interfaces (PCDs, Lib, Protocol, PPI).
> 
> Please let us know if any of these exiting methods do not work for your use
> case.
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Monday, February 26, 2018 7:55 AM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; edk2- devel@lists.01.org
> > Cc: Gao, Liming <liming.gao@intel.com>
> > Subject: Re: [edk2] [RFC] Add Platform Include path in modules
> >
> > On 02/26/18 11:55, Pankaj Bansal wrote:
> > > Hi,
> > >
> > > Consider a simple driver which needs that some data
> > structures be
> > > filled by the Platform, which is using the driver.
> > >
> > > Driver.c #include <Platform.h>
> > >
> > > Struct a = platformVal;
> > >
> > > We can define platformVal in Platform.h, which would
> > be unique to the
> > > platform being built. This Platform.h can be placed in
> > include
> > > directories, whose path would be defined in
> > Platform.dec file.
> > >
> > > Now, whenever we build driver for each unique
> > platform, we need not
> > > to mention Platform.dec file in driver.inf [packages]
> > section. We can
> > > append Platform.dec include paths to each driver. i.e.
> > look for the
> > > include files in [packages] section as well as in
> > Platform include
> > > directories.
> > >
> > > For this, I am looking for Platform.dec file in same
> > directory as
> > > Platform.dsc and using same name as Platform.dsc
> > >
> > > We can refine this change further. i.e. add Platform
> > include
> > > directories to driver's include paths based on some
> > condition in
> > > driver.inf file.
> >
> > (Apologies in advance if I failed to grasp the use
> > case.)
> >
> > If I understand correctly, you have multiple platforms (defined by DSC
> > and FDF files), and you build a given driver for several of these
> > platforms, separately. And, when building the driver for the separate
> > platforms, you'd like the driver to get different initializers for
> > various static (global) structure variables.
> >
> > Have you tried the structured PCD format? I think that could cover
> > your use case.
> >
> > Unfortunately I couldn't find anything about structured PCDs in the
> > edk2 specs, but there are several BZ references in the following
> > mailing list
> > message:
> >
> > [edk2] [Patch 00/14] Enable Structure PCD support in
> > edk2
> >
> >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.
> > mail-archive.com%2F1512140335-6932-1-git-send-
> &data=02%7C01%7Cpankaj.b
> >
> ansal%40nxp.com%7C96c42dd7271d449d56e908d57d3df2d4%7C686ea1d3b
> c2b4c6fa
> >
> 92cd99c5c301635%7C0%7C0%7C636552627376357192&sdata=iR50v%2F4Jg%
> 2BZQh7P
> > 1LB1bxeCryTangpA1SyVCwCEQW2U%3D&reserved=0
> > email-liming.gao@intel.com
> >
> > Thanks
> > Laszlo

  reply	other threads:[~2018-02-27  7:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-26  6:28 [RFC] Add Platform Include path in modules Pankaj Bansal
2018-02-26  7:42 ` Kinney, Michael D
2018-02-26 10:55   ` Pankaj Bansal
2018-02-26 15:55     ` Laszlo Ersek
2018-02-26 17:25       ` Kinney, Michael D
2018-02-27  7:39         ` Pankaj Bansal [this message]
2018-02-27 11:21           ` Gao, Liming
2018-03-09 10:54             ` Pankaj Bansal
2018-03-09 17:29               ` Kinney, Michael D
2018-02-26 12:55 ` [RFC v2] " Pankaj Bansal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR0402MB394093EF8C1F239036F29725F1C00@AM0PR0402MB3940.eurprd04.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox