On Sat, 05 Jan 2013 18:56:33 +0100 Carl-Daniel Hailfinger <[email protected]> wrote:
> Am 05.01.2013 03:46 schrieb Carl-Daniel Hailfinger: > > Am 01.01.2013 19:46 schrieb Carl-Daniel Hailfinger: > >> > Am 04.09.2012 12:14 schrieb Stefan Tauner: > >>> >> On Tue, 31 Jul 2012 09:27:47 +0200 > >>> >> Carl-Daniel Hailfinger <[email protected]> wrote: > >>> >> > >>>> >>> Ping? > >>>> >>> > >>>> >>> IMHO this patch fixes a few structural problems, and although it > >>>> >>> probably isn't the final desired result, it is a step in the right > >>>> >>> direction. > >>> >> After my short adventure into trying to understand the PCI code while > >>> >> refining the atavia patch by Jonathan, i have to agree 100% with > >>> >> Michael's first mail regarding the old code base. > >>> >> > >>> >> Something that hasn't been mentioned (or i have missed) is that there > >>> >> are some drivers that don't need a BAR at all because they work by > >>> >> accessing the configuration space only. atavia is one case (at least i > >>> >> think so). and satasii is even more special as it needs different BARs > >>> >> for different PCI IDs, but you are aware of that afaics. > > Yes, there is still quite a bit of work ahead. > > > > That said, your PCI init cleanup patch had a tremendous effect on code > > readability. Thanks for creating that patch. > > > > > >>> >> I still don't have a complete overview on how all pcidev_init > >>> >> callers work, but the current patch seems to improve things and hence > >>> >> should go in ASAP (you can read that as an ACK if you think it is > >>> >> safe). > > This patch has changed quite a bit since then... it would be great if > > you could take another look. > > > > > >>> >> Since you touch all pcidev_init calls in this patch, it would be great > >>> >> if you could switch the parameters though. The PCI dev table should be > >>> >> first since it is the most important argument and the bar should IMHO > >>> >> even be optional in the future or some kind of flag/mask as you > >>> >> discussed... that argument apparently confused not only me but also > >>> >> Jonathan and Idwer in the past. > > Very good point. I have changed the argument order, and it really looks > > nicer and more consistent > > > > > >>> >> A comment explaining the parameters would certainly improve the > >>> >> situation too (e.g. mention that the bar parameter is not the number of > >>> >> the BAR!). > > Hm yes... to be honest, I want to get rid of the bar parameter in a > > followup patch, and supply a validator function instead. That one can > > handle everything we might ever need in that direction. > > I have a followup patch which adds a validator function and should > address all remaining comments raised during the various reviews. > > Updated patch: Fix a compile failure with disabled internal programmer. > We were using a struct before declaring it first, so comapred to the > previous patch this is only some code rearranging in programmer.h. > > Signed-off-by: Carl-Daniel Hailfinger <[email protected]> looks good enough to me to commit it ;) Acked-by: Stefan Tauner <[email protected]> -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
