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 libv@skynet.be
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.