On 2015-07-06 19:25:37, Ni, Ruiyu wrote: > Jordan, Laszlo, > I personally strongly disagree using PCD to split code to make the > code "common".
Well, at least a definitive statement. :) Previously you did not seem to have much of an opinion on it: "Simply using PCD to split the code is very straightforward. Another approach is to introduce a new library class with carefully defined library APIs so that different platforms can use one common driver plus different instances of libraries." I'm really doubtful that a library class will ever be developed to allow OVMF to return to using the PcAtChipsetPkg version of the driver. I guess we will leave PcAtChipsetPkg/PciHostBridgeDxe to you and all those closed source platforms. I still hope you'll look into it more and consider deleting PcAtChipsetPkg/PciHostBridgeDxe if no one is actually using it. Thanks, -Jordan > PCD is a very fashion word in EDKII world but most of > the time it downgrades to C macro. Do you think one piece of > "common" code heavily using macro is better than two pieces of code? > I doubt that! > So unless someone (either you or Laszlo) can propose a better > solution to generalize the code, I would prefer to "duplicate" the > code in OvmfPkg first. > From the view of core packages, there is no change. > From OvmfPkg's view, it contains a new driver similar to a core driver. > Since I more care of the core packages, I prefer keep the change in > OvmfPkg for now. Your solution to introducing PCD in > PcAtChipSetPkg/PciHostBridgeDxe, in my point of view (as I said in > above), will make the core driver more ugly. > > We should make the code better and better. But maybe in one step, > maybe in several steps. > > Thanks, > Ray > > > -----Original Message----- > > From: Justen, Jordan L > > Sent: Tuesday, July 7, 2015 8:53 AM > > To: Laszlo Ersek; Ni, Ruiyu > > Cc: edk2-devel@lists.sourceforge.net; Kinney, Michael D; Gerd Hoffmann > > Subject: Re: [edk2] [PATCH v2 02/24] PcAtChipsetPkg: remove > > PciHostBridgeDxe > > > > On 2015-07-06 15:10:45, Laszlo Ersek wrote: > > > On 06/28/15 01:36, Jordan Justen wrote: > > > > On 2015-06-25 21:26:20, Ni, Ruiyu wrote: > > > >> Jordan, > > > >> Simply using PCD to split the code is very straightforward. Another > > > >> approach is to introduce a new library class with carefully defined > > > >> library APIs so that different platforms can use one common driver > > > >> plus different instances of libraries. > > > >> After all, abstracting is never a easy work. > > > >> So let's firstly make OVMF work by duplicating a PciHostBridgeDxe. > > > > > > > > Is this the preferred method of making progress then? Duplicate code > > > > first, then figure out how to make it more general? I don't see > > > > drivers duplicated all that often, so it seems a little strange. > > > > > > > > I haven't heard you say the PCD idea can't work, so can you spend a > > > > little more time considering it to see if it seems reasonable to you? > > > > If you are still concerned about the PCDs, then fine we'll just > > > > duplicate the code. > > > > > > > > And, Laszlo, is this really that urgent that you'd rather duplicate a > > > > driver under the platform rather than allowing for some discussion on > > > > a possible common solution? > > > > > > I'm back from my PTO. After processing my email backlog, I can't find > > > any news related to this patchset, so please consider it dropped, for > > > the reasons described elsewhere in this thread. > > > > This continues to be very frustrating to me. I strongly disagree with > > forking this driver. I have instead argued that we should make other > > EDK II platforms use the same driver. > > > > But, it appears that Ray will not budge. (Or *something* is preventing > > us from coming up with a solution in PcAtChipsetPkg.) This is doubly > > frustrating since he is saying that we should keep the PcAtChipsetPkg > > version that no one else is using because 'maybe some closed source > > platform is using it'. > > > > I don't expect that the PcAtChipsetPkg version will ever be updated to > > allow OVMF to go back to using it at the common location. Instead, it > > will just be orphaned under PcAtChipsetPkg. > > > > I think Ray is just taking the easy/safe route with his decision. :( > > > > But, as much as this all annoys me, I find the prospect of you having > > to carry downstream patches for this even worse. > > > > I'm just having trouble giving a Reviewed-by or Acked-by for patches > > that are doing the opposite of what I want to happen. > > > > How about? > > Acked-under-duress-by: Jordan Justen <jordan.l.jus...@intel.com> > > > > Better yet, how about I say that I'm specifically not NAK'ing the > > patches, but I'd rather not have my name on them. > > > > -Jordan > > > > > We'll carry the patches (with slight updates) in our downstream OVMF > > > package, for <https://bugzilla.redhat.com/show_bug.cgi?id=1193080>. > > > > > > I'll ask Gerd too to add them to his <https://www.kraxel.org/repos/> > > > builds, for the benefit of the wider public. ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
