On Mon, Apr 02, 2007 at 12:36:28AM +0200, Uwe Hermann wrote: > Here's a quick review: > > On Mon, Mar 26, 2007 at 04:59:18PM +0200, Luc Verhaegen wrote: > > been relegated to flash_rom.c. Then there's only some extraneous > > whitespace removal and replacing // with /* */. I'm not sure how svn > > handles moving of files, but that's usually a good point to do such > > function-less changes. > > Nope, please don't. I suggest to split this up in multiple > patches/steps. First, split flash_enable.c into board_enable.c and > chipset_enable.c without _any_ changes in content (only Makefile fixes > and related adaptions). > > Then do the other code changes (with no random cosmetics such as changing > comment style in the patch), then as a last step change any cosmetics > you don't like... > > This way all patches only contain related changes and are easily > readable an reviewable. > > > > * Fixes GPIO15 being raised on every VT8235 southbridge, regardless of what > > that line actually controls; rom on EPIA-M, backlight on mitac 8999 > > laptop. > > Please sumbit an extra patch for this please, it's not related to the > rest AFAICS. > > > > * Adds flashrom support for Asus A7V400-MX (KM400 + VT8235) > > Ditto. In general, please send rather multiple smaller patches than one > huge patch, that's a lot easier to review and merge. Don't mix unrelated > changes into one huge patch. > > > > signed-off-by: Luc Verhaegen <[EMAIL PROTECTED]> > > Capital 'S'. > > > > - rm -f *.o *~ > > + rm -f *.o *~ flashrom > > Why? 'make distclean' delete?? the binary already. > > > > distclean: clean > > rm -f $(PROGRAM) .dependencies > > > > > +struct pci_dev * > > +pci_dev_find(uint16_t vendor, uint16_t device) > > +{ > > + struct pci_dev *temp; > > + struct pci_filter filter; > > + > > + pci_filter_init(NULL, &filter); > > + filter.vendor = vendor; > > + filter.device = device; > > + > > + for (temp = pacc->devices; temp; temp = temp->next) > > + if (pci_filter_match(&filter, temp)) > > + return temp; > > + > > + return NULL; > > +} > > Please use tabs for indenting (yes, much of the current LinuxBIOS code > needs fixing too, but we should properly indent _new_ code at least). > > See also: > http://linuxbios.org/Development_Guidelines#Coding_Style > > > Uwe. Ok, will do when i have some time again.
Luc Verhaegen. -- linuxbios mailing list linuxbios@linuxbios.org http://www.linuxbios.org/mailman/listinfo/linuxbios