DUET platform is a emulate platform that has chipset and platform specific initialization done before its boot. SataControllerDxe was introduced for it to support AHCI mode. Now OVMF is a virtual platform that wants to leverage the SataControllerDxe in DuetPkg. And sharing code is a good point in EDK2 and other projects I think.
But the real hardware platforms all I worked and am working on have their own SataControllerDxe driver with some chipset specific policies or platform configurations, for example, policy to control how many ports to be exposed, specific timing configurations, only support specific vendor's controllers, and etc. If for only sharing between DUET and OVMF, the SataControllerDxe is to be into PcAtChipsetPkg that is a common package, but all the real platform code bases will have an unused SataControllerDxe since they have their own. I don't think it is good. I agree Feng's suggestion to have a SataController copy in OVMF, at least for now. If more and more emulate and virtual platforms or even real platforms are going to share the SataControllerDxe implementation in future, then we can have discussion to move it into a common package. Thanks, Star -----Original Message----- From: Reza Jelveh [mailto:reza.jel...@tuhh.de] Sent: Thursday, August 21, 2014 12:10 AM To: Tian, Feng Cc: edk2-devel@lists.sourceforge.net; ag...@suse.de Subject: Re: [edk2] [PATCH 4/6] MdeModulePkg: IdeMode should not depend on SataController On 18/08/14 00:25, Tian, Feng wrote: > Hi, Reza > > I don't get your meaning about " the ChannelCount is initialized too early ". > These drivers are UEFI driver binding drivers, which means they get > initialized by pre-assigned order. I fixed the IDE issue. ChannelCount too early means the following(unless I misunderstanding something): The controller Start is called after AtaAtapiPassThruSupported opens the ideinit protocol. SataController is an extremely misleading name. SataController and IdeController are just implementing the init protocol. For ide the channel count is not detected, but defined inside of idecontroller or satacontroller. Currently to IdeMax which is 2. The channelcount for ahci however is loaded from the cap register. The cap register needs ahci to be initialized. If AhciMode is not initialized the cap register read in SataController->Start returns 0. If you initialize AhciMode in the start of SataController ahcimode initialization will subsequently fail. I have addressed the issue you had with removing the IdeInit.ChannelCount and will post a new patch, but I can frankly not imagine a single instance where the ide controller would actually define that to be 1. When I say loaded too early I'm saying that, because SataControllerStart does memory allocations which are later used as an optimization to reduce hardware ops. But those memory allocations are based on ChannelCount which is only known after ahcimode is initialized. > > Why we did put SataController Driver at DuetPkg before rather than at > PcAtChipsetPkg is because it's a platform driver and may vary at different > platforms with different configurations. > > So I don't think moving SataController driver from Duet to PcAtChipsetPkg is > a good idea. If you want to enable Ovmf, I would like to suggest you > implement a similar on in Ovmf platform package. I completely disagree. SataController implements so basic a functionality that I don't see why it was duetpkg only to begin with. I see different projects using SataController for different hardware. Why would we copy the exact same over to Ovmf? ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
