Patchwork Revert parts of r1397

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2011-08-02 22:07:14
Message ID <4E387512.9020908@gmx.net>
Download mbox | patch
Permalink /patch/3331/
State Superseded
Headers show

Comments

Carl-Daniel Hailfinger - 2011-08-02 22:07:14
Hi,

the whitespace and coding-style fixes in r1397 changed a few places
which should have been left untouched or changed differently for various
reasons:
- Mixing uninitialized and initialized local variables leads to confusion.
- ft2232_spi error cases should have gotten some error handling, and
that's the reason the curly braces were there.
- Depending on whom you ask about the C99 standard, a comma after the
last element of an array initializer will cause another empty array
element to be allocated, and that increases array size by 1 and may
break some code which assumes certain fields to be nonzero.
- Sometimes the introduced linebreaks look worse than before.
- Fixing typos/wording in some places would have been nice given that
those places were touched anyway.
- If we insist on spelling "ASUS" all uppercase because the vendor said
so, we have to spell "LPC Bridge" with an uppercase B because the
datasheet said so.
- #if/#else/#endif are sometimes hard to follow, and a comment at the
#endif can improve readability a lot.

Comments welcome.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Stefan Tauner - 2011-08-02 23:27:36
On Wed, 03 Aug 2011 00:07:14 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:

> Index: flashrom-revert_r1397_partially/ft2232_spi.c
> ===================================================================
> --- flashrom-revert_r1397_partially/ft2232_spi.c	(Revision 1402)
> +++ flashrom-revert_r1397_partially/ft2232_spi.c	(Arbeitskopie)
> @@ -246,17 +246,21 @@
>  				ftdic->error_str);
>  	}
>  
> -	if (ftdi_usb_reset(ftdic) < 0)
> +	if (ftdi_usb_reset(ftdic) < 0) {
>  		msg_perr("Unable to reset FTDI device\n");
> +	}
>  
> -	if (ftdi_set_latency_timer(ftdic, 2) < 0)
> +	if (ftdi_set_latency_timer(ftdic, 2) < 0) {
>  		msg_perr("Unable to set latency timer\n");
> +	}
>  
> -	if (ftdi_write_data_set_chunksize(ftdic, 256))
> +	if (ftdi_write_data_set_chunksize(ftdic, 256)) {
>  		msg_perr("Unable to set chunk size\n");
> +	}
>  
> -	if (ftdi_set_bitmode(ftdic, 0x00, BITMODE_BITBANG_SPI) < 0)
> +	if (ftdi_set_bitmode(ftdic, 0x00, BITMODE_BITBANG_SPI) < 0) {
>  		msg_perr("Unable to set bitmode to SPI\n");
> +	}

those could be skipped until we really need them.
but since you have done the work already i think it is ok.

> Index: flashrom-revert_r1397_partially/chipset_enable.c
> […]
> @@ -1028,8 +1029,8 @@
>  			flashbase = parx << 12;
>  		}
>  	} else {
> -		msg_pinfo("AMD Elan SC520 detected, but no BOOTCS. "
> -			  "Assuming flash at 4G\n");
> +		msg_pinfo("AMD Elan SC520 detected, but no BOOTCS. Assuming "
> +			  "flash at 4G.\n");
>  	}

i also like breaking full sentences like it was done here previously.
filling the whole line just because we can does not mean it is useful.
reading one sentence per line is much more natural and as long as the
line count does not change i prefer it.

> Index: flashrom-revert_r1397_partially/board_enable.c
> […]
> -	dev = pci_dev_find(0x10DE, 0x0050);	/* NVIDIA CK804 ISA bridge. */
> +	dev = pci_dev_find(0x10DE, 0x0050);	/* NVIDIA CK804 ISA Bridge. */

imo it should be bridge. there is all kind of crap in datasheets. this
includes orthographic nonsense like this. but we have our own quirks
too... so i dont really care :)

>  	if (!dev) {
>  		msg_perr("\nERROR: NVIDIA nForce4 ISA bridge not found.\n");
>  		return -1;
> @@ -860,7 +860,7 @@
>  		return -1;
>  	}
>  
> -	/* First, check the ISA bridge */
> +	/* First, check the ISA Bridge */
First, check (or look) _for_ the ISA [Bb]ridge?

> […]
>  	if (!dev) {
> -		msg_perr("\nERROR: No known Intel LPC bridge found.\n");
> +		msg_perr("\nERROR: No Known Intel LPC Bridge found.\n");

well we can discuss (or not) about [Bb]ridge, but that 'Known' slipped
in i hope? :)

> […]

> -static const struct board_pciid_enable *board_match_coreboot_name(
> -					const char *vendor, const char *part)
> +static const struct board_pciid_enable *board_match_coreboot_name(const char *vendor,
> +							    const char *part)

both ugly :)

some comments apply of course to all other instances of the discussed
problem too.

Acked-by: Stefan Tauner <stefan.tauner@student.tuwien.ac.at>
Carl-Daniel Hailfinger - 2011-08-02 23:51:37
Am 03.08.2011 01:27 schrieb Stefan Tauner:
> On Wed, 03 Aug 2011 00:07:14 +0200
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
>
>   
>> Index: flashrom-revert_r1397_partially/ft2232_spi.c
>> ===================================================================
>> --- flashrom-revert_r1397_partially/ft2232_spi.c	(Revision 1402)
>> +++ flashrom-revert_r1397_partially/ft2232_spi.c	(Arbeitskopie)
>> @@ -246,17 +246,21 @@
>>  				ftdic->error_str);
>>  	}
>>  
>> -	if (ftdi_usb_reset(ftdic) < 0)
>> +	if (ftdi_usb_reset(ftdic) < 0) {
>>  		msg_perr("Unable to reset FTDI device\n");
>> +	}
>>  
>> -	if (ftdi_set_latency_timer(ftdic, 2) < 0)
>> +	if (ftdi_set_latency_timer(ftdic, 2) < 0) {
>>  		msg_perr("Unable to set latency timer\n");
>> +	}
>>  
>> -	if (ftdi_write_data_set_chunksize(ftdic, 256))
>> +	if (ftdi_write_data_set_chunksize(ftdic, 256)) {
>>  		msg_perr("Unable to set chunk size\n");
>> +	}
>>  
>> -	if (ftdi_set_bitmode(ftdic, 0x00, BITMODE_BITBANG_SPI) < 0)
>> +	if (ftdi_set_bitmode(ftdic, 0x00, BITMODE_BITBANG_SPI) < 0) {
>>  		msg_perr("Unable to set bitmode to SPI\n");
>> +	}
>>     
> those could be skipped until we really need them.
> but since you have done the work already i think it is ok.
>
>   
>> Index: flashrom-revert_r1397_partially/chipset_enable.c
>> […]
>> @@ -1028,8 +1029,8 @@
>>  			flashbase = parx << 12;
>>  		}
>>  	} else {
>> -		msg_pinfo("AMD Elan SC520 detected, but no BOOTCS. "
>> -			  "Assuming flash at 4G\n");
>> +		msg_pinfo("AMD Elan SC520 detected, but no BOOTCS. Assuming "
>> +			  "flash at 4G.\n");
>>  	}
>>     
> i also like breaking full sentences like it was done here previously.
> filling the whole line just because we can does not mean it is useful.
> reading one sentence per line is much more natural and as long as the
> line count does not change i prefer it.
>   

OK. My point was about adding a . at the end of the second sentence.


>> Index: flashrom-revert_r1397_partially/board_enable.c
>> […]
>> -	dev = pci_dev_find(0x10DE, 0x0050);	/* NVIDIA CK804 ISA bridge. */
>> +	dev = pci_dev_find(0x10DE, 0x0050);	/* NVIDIA CK804 ISA Bridge. */
>>     
> imo it should be bridge. there is all kind of crap in datasheets. this
> includes orthographic nonsense like this. but we have our own quirks
> too... so i dont really care :)
>   

I think Uwe was the one who wanted "ASUS" instead of "Asus", so I
thought the "Bridge"->"bridge" conversion was an unintended mistake on
his side.
Then again, I don't really care about capitalization here.

Uwe?


>>  	if (!dev) {
>>  		msg_perr("\nERROR: NVIDIA nForce4 ISA bridge not found.\n");
>>  		return -1;
>> @@ -860,7 +860,7 @@
>>  		return -1;
>>  	}
>>  
>> -	/* First, check the ISA bridge */
>> +	/* First, check the ISA Bridge */
>>     
> First, check (or look) _for_ the ISA [Bb]ridge?
>   

/* Check for the ISA bridge first. */



>> […]
>>  	if (!dev) {
>> -		msg_perr("\nERROR: No known Intel LPC bridge found.\n");
>> +		msg_perr("\nERROR: No Known Intel LPC Bridge found.\n");
>>     
> well we can discuss (or not) about [Bb]ridge, but that 'Known' slipped
> in i hope? :)
>   

I was reading though the reversed patch and missed that, thanks for the
hint.



>> […]
>>     
>   
>> -static const struct board_pciid_enable *board_match_coreboot_name(
>> -					const char *vendor, const char *part)
>> +static const struct board_pciid_enable *board_match_coreboot_name(const char *vendor,
>> +							    const char *part)
>>     
> both ugly :)
>   

Indeed. Unfortunately I didn't find any beautiful solution which fit in
80 columns. The second line should be moved a bit to the right, but then
we violate the 80 column limit even more.
Ideas anyone?


> some comments apply of course to all other instances of the discussed
> problem too.
>
> Acked-by: Stefan Tauner <stefan.tauner@student.tuwien.ac.at>
>   

Thanks for the review.

I'l wait a bit so Uwe has a chance to comment as well.


Regards,
Carl-Daniel
Stefan Tauner - 2011-08-03 00:27:18
On Wed, 03 Aug 2011 01:51:37 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:

> Am 03.08.2011 01:27 schrieb Stefan Tauner:
> > On Wed, 03 Aug 2011 00:07:14 +0200
> > Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
> >> Index: flashrom-revert_r1397_partially/chipset_enable.c
> >> […]
> >> @@ -1028,8 +1029,8 @@
> >>  			flashbase = parx << 12;
> >>  		}
> >>  	} else {
> >> -		msg_pinfo("AMD Elan SC520 detected, but no BOOTCS. "
> >> -			  "Assuming flash at 4G\n");
> >> +		msg_pinfo("AMD Elan SC520 detected, but no BOOTCS. Assuming "
> >> +			  "flash at 4G.\n");
> >>  	}
> >>     
> > i also like breaking full sentences like it was done here previously.
> > filling the whole line just because we can does not mean it is useful.
> > reading one sentence per line is much more natural and as long as the
> > line count does not change i prefer it.
> >   
> 
> OK. My point was about adding a . at the end of the second sentence.

ah! missed that completely :)

> >   
> >> -static const struct board_pciid_enable *board_match_coreboot_name(
> >> -					const char *vendor, const char *part)
> >> +static const struct board_pciid_enable *board_match_coreboot_name(const char *vendor,
> >> +							    const char *part)
> >>     
> > both ugly :)
> >   
> 
> Indeed. Unfortunately I didn't find any beautiful solution which fit in
> 80 columns. The second line should be moved a bit to the right, but then
> we violate the 80 column limit even more.
> Ideas anyone?

yes, violate the §$&§$ 80 column limit (scnr :)
the function could be renamed... and board_pciid_enable is also a very
long name for a struct that's used a lot.
Carl-Daniel Hailfinger - 2011-08-03 07:02:52
Am 03.08.2011 02:27 schrieb Stefan Tauner:
> On Wed, 03 Aug 2011 01:51:37 +0200
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
>
>   
>> Am 03.08.2011 01:27 schrieb Stefan Tauner:
>>     
>>> On Wed, 03 Aug 2011 00:07:14 +0200
>>> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
>>>> -static const struct board_pciid_enable *board_match_coreboot_name(
>>>> -					const char *vendor, const char *part)
>>>> +static const struct board_pciid_enable *board_match_coreboot_name(const char *vendor,
>>>> +							    const char *part)
>>>>     
>>>>         
>>> both ugly :)
>>>   
>>>       
>> Indeed. Unfortunately I didn't find any beautiful solution which fit in
>> 80 columns. The second line should be moved a bit to the right, but then
>> we violate the 80 column limit even more.
>> Ideas anyone?
>>     
> yes, violate the §$&§$ 80 column limit (scnr :)
> the function could be renamed... and board_pciid_enable is also a very
> long name for a struct that's used a lot.
>   

Suggestion:
struct board_pciid_enable -> struct board_match
board_match_coreboot_name() -> board_match_cbname()

What do you think?

Regards,
Carl-Daniel
Stefan Tauner - 2011-08-03 11:40:14
On Wed, 03 Aug 2011 09:02:52 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:

> Suggestion:
> struct board_pciid_enable -> struct board_match
> board_match_coreboot_name() -> board_match_cbname()
> 
> What do you think?

looks fine imo.
board_match_pci_card_ids could also be renamed to board_match_pci_ids
and if you are at it please change the comment of it:
/*
 * Match boards on PCI IDs and subsystem IDs.
 * Second set of IDs can be main only or missing completely.
 */
const static struct board_pciid_enable *board_match_pci_card_ids(enum board_match_phase phase)

- * Second set of IDs can be main only or missing completely.
+ * Second set of IDs can contain primary IDs only or be missing completely.

aaaand if we change board_match_pci_card_ids we should also change
pci_card_find to pci_dev_find or something like that... :)

and now i better stop looking for such stuff.

Patch

Index: flashrom-revert_r1397_partially/it87spi.c
===================================================================
--- flashrom-revert_r1397_partially/it87spi.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/it87spi.c	(Arbeitskopie)
@@ -203,7 +203,8 @@ 
 
 int init_superio_ite(void)
 {
-	int i, ret = 0;
+	int i;
+	int ret = 0;
 
 	for (i = 0; i < superio_count; i++) {
 		if (superios[i].vendor != SUPERIO_VENDOR_ITE)
Index: flashrom-revert_r1397_partially/cli_classic.c
===================================================================
--- flashrom-revert_r1397_partially/cli_classic.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/cli_classic.c	(Arbeitskopie)
@@ -104,13 +104,14 @@ 
 	struct flashchip flashes[3];
 	struct flashchip *fill_flash;
 	const char *name;
-	int startchip = 0, chipcount = 0, namelen, opt, option_index = 0;
-	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
-	int dont_verify_it = 0, list_supported = 0, force = 0;
+	int namelen, opt, i;
+	int startchip = 0, chipcount = 0, option_index = 0, force = 0;
 #if CONFIG_PRINT_WIKI == 1
 	int list_supported_wiki = 0;
 #endif
-	int operation_specified = 0, i, ret = 0;
+	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
+	int dont_verify_it = 0, list_supported = 0, operation_specified = 0;
+	int ret = 0;
 
 	static const char optstring[] = "r:Rw:v:nVEfc:m:l:i:p:Lzh";
 	static const struct option long_options[] = {
Index: flashrom-revert_r1397_partially/dmi.c
===================================================================
--- flashrom-revert_r1397_partially/dmi.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/dmi.c	(Arbeitskopie)
@@ -196,7 +196,8 @@ 
  */
 static int dmi_compare(const char *value, const char *pattern)
 {
-	int anchored = 0, patternlen;
+	int anchored = 0;
+	int patternlen;
 
 	msg_pspew("matching %s against %s\n", value, pattern);
 	/* The empty string is part of all strings! */
Index: flashrom-revert_r1397_partially/dediprog.c
===================================================================
--- flashrom-revert_r1397_partially/dediprog.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/dediprog.c	(Arbeitskopie)
@@ -496,7 +496,8 @@ 
 static int parse_voltage(char *voltage)
 {
 	char *tmp = NULL;
-	int i, millivolt, fraction = 0;
+	int i;
+	int millivolt = 0, fraction = 0;
 
 	if (!voltage || !strlen(voltage)) {
 		msg_perr("Empty voltage= specified.\n");
@@ -574,7 +575,8 @@ 
 {
 	struct usb_device *dev;
 	char *voltage;
-	int millivolt = 3500, ret;
+	int millivolt = 3500;
+	int ret;
 
 	msg_pspew("%s\n", __func__);
 
Index: flashrom-revert_r1397_partially/it85spi.c
===================================================================
--- flashrom-revert_r1397_partially/it85spi.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/it85spi.c	(Arbeitskopie)
@@ -84,15 +84,15 @@ 
 static int it85xx_scratch_rom_reenter = 0;
 
 /* This function will poll the keyboard status register until either
- *   an expected value shows up, or
- *   timeout reaches.
+ * an expected value shows up, or the timeout is reached.
+ * timeout is in usec.
  *
- * Returns: 0 -- the expected value has shown.
- *          1 -- timeout reached.
+ * Returns: 0 -- the expected value showed up.
+ *          1 -- timeout.
  */
 static int wait_for(const unsigned int mask, const unsigned int expected_value,
-		const int timeout /* in usec */, const char *error_message,
-		const char *function_name, const int lineno)
+		    const int timeout, const char * error_message,
+		    const char * function_name, const int lineno)
 {
 	int time_passed;
 
@@ -317,8 +317,9 @@ 
 	int i;
 
 	it85xx_enter_scratch_rom();
-	/* exit scratch ROM ONLY when programmer shuts down. Otherwise, the
-	 * temporary flash state may halt EC. */
+	/* Exit scratch ROM ONLY when programmer shuts down. Otherwise, the
+	 * temporary flash state may halt the EC.
+	 */
 
 #ifdef LPC_IO
 	INDIRECT_A1(shm_io_base, (((unsigned long int)ce_high) >> 8) & 0xff);
Index: flashrom-revert_r1397_partially/buspirate_spi.c
===================================================================
--- flashrom-revert_r1397_partially/buspirate_spi.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/buspirate_spi.c	(Arbeitskopie)
@@ -109,7 +109,7 @@ 
 	{"2.6M",	0x5},
 	{"4M",		0x6},
 	{"8M",		0x7},
-	{NULL,		0x0},
+	{NULL,		0x0}
 };
 
 static int buspirate_spi_shutdown(void *data)
@@ -150,9 +150,11 @@ 
 int buspirate_spi_init(void)
 {
 	unsigned char buf[512];
-	int ret = 0, i, spispeed = 0x7;
 	char *dev = NULL;
 	char *speed = NULL;
+	int spispeed = 0x7;
+	int ret = 0;
+	int i;
 
 	dev = extract_programmer_param("dev");
 	if (!dev || !strlen(dev)) {
Index: flashrom-revert_r1397_partially/physmap.c
===================================================================
--- flashrom-revert_r1397_partially/physmap.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/physmap.c	(Arbeitskopie)
@@ -76,7 +76,7 @@ 
 
 	mi.address = phys_addr;
 	mi.size = len;
-	ret = __dpmi_physical_address_mapping (&mi);
+	ret = __dpmi_physical_address_mapping(&mi);
 
 	if (ret != 0)
 		return ERROR_PTR;
Index: flashrom-revert_r1397_partially/ft2232_spi.c
===================================================================
--- flashrom-revert_r1397_partially/ft2232_spi.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/ft2232_spi.c	(Arbeitskopie)
@@ -246,17 +246,21 @@ 
 				ftdic->error_str);
 	}
 
-	if (ftdi_usb_reset(ftdic) < 0)
+	if (ftdi_usb_reset(ftdic) < 0) {
 		msg_perr("Unable to reset FTDI device\n");
+	}
 
-	if (ftdi_set_latency_timer(ftdic, 2) < 0)
+	if (ftdi_set_latency_timer(ftdic, 2) < 0) {
 		msg_perr("Unable to set latency timer\n");
+	}
 
-	if (ftdi_write_data_set_chunksize(ftdic, 256))
+	if (ftdi_write_data_set_chunksize(ftdic, 256)) {
 		msg_perr("Unable to set chunk size\n");
+	}
 
-	if (ftdi_set_bitmode(ftdic, 0x00, BITMODE_BITBANG_SPI) < 0)
+	if (ftdi_set_bitmode(ftdic, 0x00, BITMODE_BITBANG_SPI) < 0) {
 		msg_perr("Unable to set bitmode to SPI\n");
+	}
 
 	if (clock_5x) {
 		msg_pdbg("Disable divide-by-5 front stage\n");
@@ -323,7 +327,8 @@ 
 	struct ftdi_context *ftdic = &ftdic_context;
 	static unsigned char *buf = NULL;
 	/* failed is special. We use bitwise ops, but it is essentially bool. */
-	int i = 0, ret = 0, failed = 0, bufsize;
+	int i = 0, ret = 0, failed = 0;
+	int bufsize;
 	static int oldbufsize = 0;
 
 	if (writecnt > 65536 || readcnt > 65536)
Index: flashrom-revert_r1397_partially/chipset_enable.c
===================================================================
--- flashrom-revert_r1397_partially/chipset_enable.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/chipset_enable.c	(Arbeitskopie)
@@ -309,8 +309,9 @@ 
 static int enable_flash_ich_dc(struct pci_dev *dev, const char *name)
 {
 	uint32_t fwh_conf;
+	int i, tmp;
 	char *idsel = NULL;
-	int i, tmp, max_decode_fwh_idsel = 0, max_decode_fwh_decode = 0;
+	int max_decode_fwh_idsel = 0, max_decode_fwh_decode = 0;
 	int contiguous = 1;
 
 	idsel = extract_programmer_param("fwh_idsel");
@@ -1028,8 +1029,8 @@ 
 			flashbase = parx << 12;
 		}
 	} else {
-		msg_pinfo("AMD Elan SC520 detected, but no BOOTCS. "
-			  "Assuming flash at 4G\n");
+		msg_pinfo("AMD Elan SC520 detected, but no BOOTCS. Assuming "
+			  "flash at 4G.\n");
 	}
 
 	/* 4. Clean up */
Index: flashrom-revert_r1397_partially/flashrom.c
===================================================================
--- flashrom-revert_r1397_partially/flashrom.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/flashrom.c	(Arbeitskopie)
@@ -600,8 +600,7 @@ 
 	size_t size = flash->total_size * 1024;
 	/* Flash registers live 4 MByte below the flash. */
 	/* FIXME: This is incorrect for nonstandard flashbase. */
-	flash->virtual_registers = (chipaddr)programmer_map_flash_region(
-	    "flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size);
+	flash->virtual_registers = (chipaddr)programmer_map_flash_region("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size);
 }
 
 int read_memmapped(struct flashchip *flash, uint8_t *buf, int start, int len)
@@ -753,8 +752,9 @@ 
 int verify_range(struct flashchip *flash, uint8_t *cmpbuf, int start, int len,
 		 const char *message)
 {
-	int i, ret = 0, failcount = 0;
+	int i;
 	uint8_t *readbuf = malloc(len);
+	int ret = 0, failcount = 0;
 
 	if (!len)
 		goto out_free;
@@ -832,7 +832,8 @@ 
  */
 int need_erase(uint8_t *have, uint8_t *want, int len, enum write_granularity gran)
 {
-	int result = 0, i, j, limit;
+	int result = 0;
+	int i, j, limit;
 
 	switch (gran) {
 	case write_gran_1bit:
@@ -898,7 +899,8 @@ 
 static int get_next_write(uint8_t *have, uint8_t *want, int len,
 			  int *first_start, enum write_granularity gran)
 {
-	int need_write = 0, rel_start = 0, first_len = 0, i, limit, stride;
+	int need_write = 0, rel_start = 0, first_len = 0;
+	int i, limit, stride;
 
 	switch (gran) {
 	case write_gran_1bit:
@@ -1326,7 +1328,8 @@ 
  */
 static int selfcheck_eraseblocks(const struct flashchip *flash)
 {
-	int i, j, k, ret = 0;
+	int i, j, k;
+	int ret = 0;
 
 	for (k = 0; k < NUM_ERASEFUNCTIONS; k++) {
 		unsigned int done = 0;
@@ -1451,7 +1454,8 @@ 
 			     void *param1, void *param2)
 {
 	int i, j;
-	unsigned int start = 0, len;
+	unsigned int start = 0;
+	unsigned int len;
 	struct block_eraser eraser = flash->block_erasers[erasefunction];
 
 	for (i = 0; i < NUM_ERASEREGIONS; i++) {
@@ -1603,8 +1607,10 @@ 
 void list_programmers_linebreak(int startcol, int cols, int paren)
 {
 	const char *pname;
-	int pnamelen, remaining = 0, firstline = 1, i;
+	int pnamelen;
+	int remaining = 0, firstline = 1;
 	enum programmer p;
+	int i;
 
 	for (p = 0; p < PROGRAMMER_INVALID; p++) {
 		pname = programmer_table[p].name;
@@ -1737,7 +1743,7 @@ 
 		msg_gerr("Known laptops table does not exist!\n");
 		ret = 1;
 	}
-#endif
+#endif // CONFIG_INTERNAL == 1
 	return ret;
 }
 
Index: flashrom-revert_r1397_partially/board_enable.c
===================================================================
--- flashrom-revert_r1397_partially/board_enable.c	(Revision 1402)
+++ flashrom-revert_r1397_partially/board_enable.c	(Arbeitskopie)
@@ -179,7 +179,7 @@ 
 static const struct winbond_port w83627hf[3] = {
 	UNIMPLEMENTED_PORT,
 	{w83627hf_port2_mux, 0x08, 0, 0xF0},
-	UNIMPLEMENTED_PORT,
+	UNIMPLEMENTED_PORT
 };
 
 static const struct winbond_mux w83627ehf_port2_mux[8] = {
@@ -190,7 +190,7 @@ 
 	{0x2A, 0x01, 0x01},	/* or keyboard/mouse interface */
 	{0x2A, 0x01, 0x01},
 	{0x2A, 0x01, 0x01},
-	{0x2A, 0x01, 0x01},
+	{0x2A, 0x01, 0x01}
 };
 
 static const struct winbond_port w83627ehf[6] = {
@@ -199,7 +199,7 @@ 
 	UNIMPLEMENTED_PORT,
 	UNIMPLEMENTED_PORT,
 	UNIMPLEMENTED_PORT,
-	UNIMPLEMENTED_PORT,
+	UNIMPLEMENTED_PORT
 };
 
 static const struct winbond_mux w83627thf_port4_mux[8] = {
@@ -210,7 +210,7 @@ 
 	{0x2D, 0x10, 0x10},	/* or PWROK */
 	{0x2D, 0x20, 0x20},	/* or suspend LED */
 	{0x2D, 0x40, 0x40},	/* or panel switch input */
-	{0x2D, 0x80, 0x80},	/* or panel switch output */
+	{0x2D, 0x80, 0x80}	/* or panel switch output */
 };
 
 static const struct winbond_port w83627thf[5] = {
@@ -218,7 +218,7 @@ 
 	UNIMPLEMENTED_PORT,	/* GPIO2 */
 	UNIMPLEMENTED_PORT,	/* GPIO3 */
 	{w83627thf_port4_mux, 0x09, 1, 0xF4},
-	UNIMPLEMENTED_PORT,	/* GPIO5 */
+	UNIMPLEMENTED_PORT	/* GPIO5 */
 };
 
 static const struct winbond_chip winbond_chips[] = {
@@ -812,7 +812,7 @@ 
 {
 	struct pci_dev *dev;
 
-	dev = pci_dev_find(0x10DE, 0x0050);	/* NVIDIA CK804 ISA bridge. */
+	dev = pci_dev_find(0x10DE, 0x0050);	/* NVIDIA CK804 ISA Bridge. */
 	if (!dev) {
 		msg_perr("\nERROR: NVIDIA nForce4 ISA bridge not found.\n");
 		return -1;
@@ -860,7 +860,7 @@ 
 		return -1;
 	}
 
-	/* First, check the ISA bridge */
+	/* First, check the ISA Bridge */
 	dev = pci_dev_find_vendorclass(0x10DE, 0x0601);
 	switch (dev->device_id) {
 	case 0x0030: /* CK804 */
@@ -1101,7 +1101,7 @@ 
 	struct pci_dev *dev;
 	uint32_t reg;
 
-	dev = pci_dev_find(0x1002, 0x4372); /* AMD SMBus controller */
+	dev = pci_dev_find(0x1002, 0x4372); /* AMD SMBus Controller */
 	if (!dev) {
 		msg_perr("\nERROR: AMD SMBus Controller (0x4372) not found.\n");
 		return -1;
@@ -1128,8 +1128,8 @@ 
 	struct pci_dev *dev;
 	uint32_t tmp, base;
 
-	/* GPPO {0,8,27,28,30} are always available */
-	static const uint32_t nonmuxed_gpos  = 0x58000101;
+	/* GPO{0,8,27,28,30} are always available. */
+	static const uint32_t nonmuxed_gpos = 0x58000101;
 
 	static const struct {unsigned int reg, mask, value; } piix4_gpo[] = {
 		{0},
@@ -1178,10 +1178,9 @@ 
 	}
 
 	if ((((1 << gpo) & nonmuxed_gpos) == 0) &&
-	     (pci_read_word(dev, piix4_gpo[gpo].reg)
-	     & piix4_gpo[gpo].mask) != piix4_gpo[gpo].value) {
-		msg_perr("\nERROR: PIIX4 GPO%d not programmed for output.\n",
-			 gpo);
+	    ((pci_read_word(dev, piix4_gpo[gpo].reg) & piix4_gpo[gpo].mask) !=
+	     piix4_gpo[gpo].value)) {
+		msg_perr("\nERROR: PIIX4 GPO%d not programmed for output.\n", gpo);
 		return -1;
 	}
 
@@ -1317,7 +1316,7 @@ 
 		/* libpci before version 2.2.4 does not store class info. */
 		device_class = pci_read_word(dev, PCI_CLASS_DEVICE);
 		if ((dev->vendor_id == 0x8086) &&
-		    (device_class == 0x0601)) { /* ISA bridge */
+		    (device_class == 0x0601)) { /* ISA Bridge */
 			/* Is this device in our list? */
 			for (i = 0; intel_ich_gpio_table[i].id; i++)
 				if (dev->device_id == intel_ich_gpio_table[i].id)
@@ -1329,7 +1328,7 @@ 
 	}
 
 	if (!dev) {
-		msg_perr("\nERROR: No known Intel LPC bridge found.\n");
+		msg_perr("\nERROR: No Known Intel LPC Bridge found.\n");
 		return -1;
 	}
 
@@ -1349,12 +1348,12 @@ 
 		allowed = (intel_ich_gpio_table[i].bank2 >> (gpio - 64)) & 0x01;
 
 	if (!allowed) {
-		msg_perr("\nERROR: This Intel LPC bridge does not allow"
-			 " setting GPIO%02d\n", gpio);
+		msg_perr("\nERROR: This Intel LPC Bridge does not allow"
+			" setting GPIO%02d\n", gpio);
 		return -1;
 	}
 
-	msg_pdbg("\nIntel ICH LPC bridge: %sing GPIO%02d.\n",
+	msg_pdbg("\nIntel ICH LPC Bridge: %sing GPIO%02d.\n",
 		 raise ? "Rais" : "Dropp", gpio);
 
 	if (gpio < 32) {
@@ -1373,7 +1372,7 @@ 
 		if (dev->device_id > 0x2800) {
 			tmp = INL(base);
 			if (!(tmp & (1 << gpio))) {
-				msg_perr("\nERROR: This Intel LPC bridge"
+				msg_perr("\nERROR: This Intel LPC Bridge"
 					" does not allow setting GPIO%02d\n",
 					gpio);
 				return -1;
@@ -1405,7 +1404,7 @@ 
 		if (dev->device_id > 0x2800) {
 			tmp = INL(base + 30);
 			if (!(tmp & (1 << gpio))) {
-				msg_perr("\nERROR: This Intel LPC bridge"
+				msg_perr("\nERROR: This Intel LPC Bridge"
 					" does not allow setting GPIO%02d\n",
 					gpio + 32);
 				return -1;
@@ -1434,7 +1433,7 @@ 
 
 		tmp = INL(base + 40);
 		if (!(tmp & (1 << gpio))) {
-			msg_perr("\nERROR: This Intel LPC bridge does "
+			msg_perr("\nERROR: This Intel LPC Bridge does "
 				"not allow setting GPIO%02d\n", gpio + 64);
 			return -1;
 		}
@@ -2091,8 +2090,8 @@ 
  * Match boards on coreboot table gathered vendor and part name.
  * Require main PCI IDs to match too as extra safety.
  */
-static const struct board_pciid_enable *board_match_coreboot_name(
-					const char *vendor, const char *part)
+static const struct board_pciid_enable *board_match_coreboot_name(const char *vendor,
+							    const char *part)
 {
 	const struct board_pciid_enable *board = board_pciid_enables;
 	const struct board_pciid_enable *partmatch = NULL;
@@ -2144,8 +2143,7 @@ 
  * Match boards on PCI IDs and subsystem IDs.
  * Second set of IDs can be main only or missing completely.
  */
-const static struct board_pciid_enable *board_match_pci_card_ids(
-						enum board_match_phase phase)
+const static struct board_pciid_enable *board_match_pci_card_ids(enum board_match_phase phase)
 {
 	const struct board_pciid_enable *board = board_pciid_enables;