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. -- http://www.hermann-uwe.de | http://www.holsham-traders.de http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
signature.asc
Description: Digital signature
-- linuxbios mailing list linuxbios@linuxbios.org http://www.linuxbios.org/mailman/listinfo/linuxbios