On Mon, 2014-01-13 at 16:39 -0500, Kevin O'Connor wrote: > On Mon, Jan 13, 2014 at 07:46:33PM +0200, Marcel Apfelbaum wrote: > > On Mon, 2014-01-13 at 11:31 -0500, Kevin O'Connor wrote: > > > Thanks. SeaBIOS isn't responsible for PCI setup on CSM/coreboot, so > > > the patch must check for CONFIG_QEMU. > > Sure thing, Sorry I missed that, I'll add it to V2. > > > > > > Also, I think we can simplify this a bit - how about the patch below > > > (untested)? > > Hi Kevin, > > I followed the patch and indeed it is smaller and it does the job, but > > - We also have a q35 chipset and we'll need a global variable also for it. > > - If we will have other devices that need special attention on resume > > we will be ready for them (low chance, but you never know). > > - Finally, the pci_resume will look a little strange and unclear with > > the new "if" statements (that means, this is the board and we saved the > > value) > > Is this needed for q35? In general, the firmware is not responsible > for restoring hardware state, so I don't think resume fixups will be > common. The storing of PCI BDFs for use in resume is already done > elsewhere in the code (see shadow.c and smm.c). I checked and Q35 does not need this kind of hack, but it has other problems :( (Can't suspend/resume 2 times). I am going to find some time to look into it. > > > What do you think? > > I'm open to alternatives. However, the code needs to be run only when > CONFIG_QEMU is set, and I would ask that resume.c not be made more > complicated - so lets have it just call a function (eg, pci_resume()) > and put the rest of the logic in the fw/ directory. Thank you for the review! I sent a V2 following your review.
Thanks, Marcel > > -Kevin _______________________________________________ SeaBIOS mailing list [email protected] http://www.seabios.org/mailman/listinfo/seabios
