[flashrom] [PATCH] convert source tree to use msg_*

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Wed Mar 24 22:46:18 CET 2010


On 24.03.2010 21:50, Sean Nelson wrote:
> Convert all print messages to the msg_* print-message interface.
> Split into 3 patches for easier review. Intended as a single commit.
>
> Signed-off-by: Sean Nelson <audiohacked at gmail.com>

Wow, that's a big patch. Thanks!

Review for the chip part follows. If you address the comments, the chip
part is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

> --- a/82802ab.c
> +++ b/82802ab.c
> @@ -24,72 +24,72 @@
>   *  - URL: http://www.intel.com/design/chipsets/datashts/290658.htm
>   *  - PDF: http://download.intel.com/design/chipsets/datashts/29065804.pdf
>   *  - Order number: 290658-004
>   */
>  
>  #include <string.h>
>  #include <stdlib.h>
>  #include "flash.h"
>  #include "chipdrivers.h"
>  
>  // I need that Berkeley bit-map printer
>  void print_status_82802ab(uint8_t status)
>  {
> -	printf_debug("%s", status & 0x80 ? "Ready:" : "Busy:");
> -	printf_debug("%s", status & 0x40 ? "BE SUSPEND:" : "BE RUN/FINISH:");
> -	printf_debug("%s", status & 0x20 ? "BE ERROR:" : "BE OK:");
> -	printf_debug("%s", status & 0x10 ? "PROG ERR:" : "PROG OK:");
> -	printf_debug("%s", status & 0x8 ? "VP ERR:" : "VPP OK:");
> -	printf_debug("%s", status & 0x4 ? "PROG SUSPEND:" : "PROG RUN/FINISH:");
> -	printf_debug("%s", status & 0x2 ? "WP|TBL#|WP#,ABORT:" : "UNLOCK:");
> +	msg_cspew("%s", status & 0x80 ? "Ready:" : "Busy:");
> +	msg_cspew("%s", status & 0x40 ? "BE SUSPEND:" : "BE RUN/FINISH:");
> +	msg_cspew("%s", status & 0x20 ? "BE ERROR:" : "BE OK:");
> +	msg_cspew("%s", status & 0x10 ? "PROG ERR:" : "PROG OK:");
> +	msg_cspew("%s", status & 0x8 ? "VP ERR:" : "VPP OK:");
> +	msg_cspew("%s", status & 0x4 ? "PROG SUSPEND:" : "PROG RUN/FINISH:");
> +	msg_cspew("%s", status & 0x2 ? "WP|TBL#|WP#,ABORT:" : "UNLOCK:");

cdbg maybe? I think this code is not called often

>  }
>  
>  int probe_82802ab(struct flashchip *flash)
>  {
>  	chipaddr bios = flash->virtual_memory;
>  	uint8_t id1, id2;
>  	uint8_t flashcontent1, flashcontent2;
>  
>  	/* Reset to get a clean state */
>  	chip_writeb(0xFF, bios);
>  	programmer_delay(10);
>  
>  	/* Enter ID mode */
>  	chip_writeb(0x90, bios);
>  	programmer_delay(10);
>  
>  	id1 = chip_readb(bios);
>  	id2 = chip_readb(bios + 0x01);
>  
>  	/* Leave ID mode */
>  	chip_writeb(0xFF, bios);
>  
>  	programmer_delay(10);
>  
> -	printf_debug("%s: id1 0x%02x, id2 0x%02x", __func__, id1, id2);
> +	msg_cdbg("%s: id1 0x%02x, id2 0x%02x", __func__, id1, id2);
>  
>  	if (!oddparity(id1))
> -		printf_debug(", id1 parity violation");
> +		msg_cspew(", id1 parity violation");

cdbg

>  
>  	/* Read the product ID location again. We should now see normal flash contents. */
>  	flashcontent1 = chip_readb(bios);
>  	flashcontent2 = chip_readb(bios + 0x01);
>  
>  	if (id1 == flashcontent1)
> -		printf_debug(", id1 is normal flash content");
> +		msg_cspew(", id1 is normal flash content");

cdbg please for consistency and to keep our logs easily analyzable

>  	if (id2 == flashcontent2)
> -		printf_debug(", id2 is normal flash content");
> +		msg_cspew(", id2 is normal flash content");

cdbg

>  
> -	printf_debug("\n");
> +	msg_cdbg("\n");
>  	if (id1 != flash->manufacture_id || id2 != flash->model_id)
>  		return 0;
>  
>  	if (flash->feature_bits & FEATURE_REGISTERMAP)
>  		map_flash_registers(flash);
>  
>  	return 1;
>  }
>  
>  uint8_t wait_82802ab(chipaddr bios)
>  {
>  	uint8_t status;
>  
> @@ -127,145 +127,146 @@ int erase_block_82802ab(struct flashchip *flash, unsigned int page, unsigned int
>  	// clear status register
>  	chip_writeb(0x50, bios + page);
>  
>  	// now start it
>  	chip_writeb(0x20, bios + page);
>  	chip_writeb(0xd0, bios + page);
>  	programmer_delay(10);
>  
>  	// now let's see what the register is
>  	status = wait_82802ab(bios);
>  	print_status_82802ab(status);
>  
>  	if (check_erased_range(flash, page, pagesize)) {
> -		fprintf(stderr, "ERASE FAILED!\n");
> +		msg_cerr("ERASE FAILED!\n");
>  		return -1;
>  	}
> -	printf("DONE BLOCK 0x%x\n", page);
> +	msg_cinfo("DONE BLOCK 0x%x\n", page);
>  
>  	return 0;
>  }
>  
>  int erase_82802ab(struct flashchip *flash)
>  {
>  	int i;
>  	unsigned int total_size = flash->total_size * 1024;
>  
> -	printf("total_size is %d; flash->page_size is %d\n",
> +	msg_cspew("total_size is %d; flash->page_size is %d\n",
>  	       total_size, flash->page_size);
>  	for (i = 0; i < total_size; i += flash->page_size)
>  		if (erase_block_82802ab(flash, i, flash->page_size)) {
> -			fprintf(stderr, "ERASE FAILED!\n");
> +			msg_cerr("ERASE FAILED!\n");
>  			return -1;
>  		}
> -	printf("DONE ERASE\n");
> +	msg_cinfo("DONE ERASE\n");
>  
>  	return 0;
>  }
>  
>  void write_page_82802ab(chipaddr bios, uint8_t *src,
>  			chipaddr dst, int page_size)
>  {
>  	int i;
>  
>  	for (i = 0; i < page_size; i++) {
>  		/* transfer data from source to destination */
>  		chip_writeb(0x40, dst);
>  		chip_writeb(*src++, dst++);
>  		wait_82802ab(bios);
>  	}
>  }
>  
>  int write_82802ab(struct flashchip *flash, uint8_t *buf)
>  {
>  	int i;
>  	int total_size = flash->total_size * 1024;
>  	int page_size = flash->page_size;
>  	chipaddr bios = flash->virtual_memory;
>  	uint8_t *tmpbuf = malloc(page_size);
>  
>  	if (!tmpbuf) {
> -		printf("Could not allocate memory!\n");
> +		msg_cerr("Could not allocate memory!\n");
>  		exit(1);
>  	}
> -	printf("Programming page: \n");
> +	msg_cinfo("Programming page: \n");
>  	for (i = 0; i < total_size / page_size; i++) {
> -		printf
> -		    ("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
> -		printf("%04d at address: 0x%08x", i, i * page_size);
> +		msg_cdbg("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
> +		msg_cdbg("%04d at address: 0x%08x", i, i * page_size);

Progress printing is cinfo for many other chips... I wrote a bit more
about it a few paragraphs below.

>  
>  		/* Auto Skip Blocks, which already contain the desired data
>  		 * Faster, because we only write, what has changed
>  		 * More secure, because blocks, which are excluded
>  		 * (with the exclude or layout feature)
>  		 * or not erased and rewritten; their data is retained also in
>  		 * sudden power off situations
>  		 */
>  		chip_readn(tmpbuf, bios + i * page_size, page_size);
>  		if (!memcmp((void *)(buf + i * page_size), tmpbuf, page_size)) {
> -			printf("SKIPPED\n");
> +			msg_cspew("SKIPPED\n");

bah. Progress printing special case. Not your fault. Maybe cdbg or even
cinfo to be more in line with the other progress printing.

>  			continue;
>  		}
>  
>  		/* erase block by block and write block by block; this is the most secure way */
>  		if (erase_block_82802ab(flash, i * page_size, page_size)) {
> -			fprintf(stderr, "ERASE FAILED!\n");
> +			msg_cerr("ERASE FAILED!\n");
>  			return -1;
>  		}
>  		write_page_82802ab(bios, buf + i * page_size,
>  				   bios + i * page_size, page_size);
>  	}
> -	printf("\n");
> +	msg_cinfo("DONE!\n");
>  	free(tmpbuf);
>  
>  	return 0;
>  }
>  
>  int unlock_28f004s5(struct flashchip *flash)
>  {
>  	chipaddr bios = flash->virtual_memory;
>  	uint8_t mcfg, bcfg, need_unlock = 0, can_unlock = 0;
>  	int i;
>  
>  	/* Clear status register */
>  	chip_writeb(0x50, bios);
>  
>  	/* Read identifier codes */
>  	chip_writeb(0x90, bios);
>  
>  	/* Read master lock-bit */
>  	mcfg = chip_readb(bios + 0x3);
> -	msg_cinfo("master lock is ");
> +	msg_cspew("master lock is ");
>  	if (mcfg) {
> -		msg_cdbg("locked!\n");
> +		msg_cspew("locked!\n");
>  	} else {
> -		msg_cdbg("unlocked!\n");
> +		msg_cspew("unlocked!\n");

locking is cdbg for all other chips, please use cdbg here as well

>  		can_unlock = 1;
>  	}
>  	
>  	/* Read block lock-bits */
>  	for (i = 0; i < flash->total_size * 1024; i+= (64 * 1024)) {
>  		bcfg = chip_readb(bios + i + 2); // read block lock config
> -		msg_cdbg("block lock at %06x is %slocked!\n", i, bcfg ? "" : "un");
> +		msg_cspew("block lock at %06x is %slocked!\n", i, bcfg ? "" : "un");

cdbg

>  		if (bcfg) {
>  			need_unlock = 1;
>  		}
>  	}
>  
>  	/* Reset chip */
>  	chip_writeb(0xFF, bios);
>  
>  	/* Unlock: clear block lock-bits, if needed */
>  	if (can_unlock && need_unlock) {
> +		msg_cinfo("Unlock: ");

hm. cdbg maybe?

>  		chip_writeb(0x60, bios);
>  		chip_writeb(0xD0, bios);
>  		chip_writeb(0xFF, bios);
> +		msg_cinfo("Done!\n");

same here

>  	}
>  
>  	/* Error: master locked or a block is locked */
>  	if (!can_unlock && need_unlock) {
>  		msg_cerr("At least one block is locked and lockdown is active!\n");
>  		return -1;
>  	}
>  
>  	return 0;
>  }
> diff --git a/jedec.c b/jedec.c
> index fee7302..fbea35f 100644
> --- a/jedec.c
> +++ b/jedec.c
> @@ -45,27 +45,27 @@ void toggle_ready_jedec_common(chipaddr dst, int delay)
>  
>  	tmp1 = chip_readb(dst) & 0x40;
>  
>  	while (i++ < 0xFFFFFFF) {
>  		if (delay)
>  			programmer_delay(delay);
>  		tmp2 = chip_readb(dst) & 0x40;
>  		if (tmp1 == tmp2) {
>  			break;
>  		}
>  		tmp1 = tmp2;
>  	}
>  	if (i > 0x100000)
> -		printf_debug("%s: excessive loops, i=0x%x\n", __func__, i);
> +		msg_cdbg("%s: excessive loops, i=0x%x\n", __func__, i);

cinfo maybe?

>  }
>  
>  void toggle_ready_jedec(chipaddr dst)
>  {
>  	toggle_ready_jedec_common(dst, 0);
>  }
>  
>  /* Some chips require a minimum delay between toggle bit reads.
>   * The Winbond W39V040C wants 50 ms between reads on sector erase toggle,
>   * but experiments show that 2 ms are already enough. Pick a safety factor
>   * of 4 and use an 8 ms delay.
>   * Given that erase is slow on all chips, it is recommended to use 
>   * toggle_ready_jedec_slow in erase functions.
> @@ -79,56 +79,56 @@ void data_polling_jedec(chipaddr dst, uint8_t data)
>  {
>  	unsigned int i = 0;
>  	uint8_t tmp;
>  
>  	data &= 0x80;
>  
>  	while (i++ < 0xFFFFFFF) {
>  		tmp = chip_readb(dst) & 0x80;
>  		if (tmp == data) {
>  			break;
>  		}
>  	}
>  	if (i > 0x100000)
> -		printf_debug("%s: excessive loops, i=0x%x\n", __func__, i);
> +		msg_cdbg("%s: excessive loops, i=0x%x\n", __func__, i);

cinfo maybe?

>  }
>  
>  void start_program_jedec_common(struct flashchip *flash, unsigned int mask)
>  {
>  	chipaddr bios = flash->virtual_memory;
>  	chip_writeb(0xAA, bios + (0x5555 & mask));
>  	chip_writeb(0x55, bios + (0x2AAA & mask));
>  	chip_writeb(0xA0, bios + (0x5555 & mask));
>  }
>  
>  int probe_jedec_common(struct flashchip *flash, unsigned int mask)
>  {
>  	chipaddr bios = flash->virtual_memory;
>  	uint8_t id1, id2;
>  	uint32_t largeid1, largeid2;
>  	uint32_t flashcontent1, flashcontent2;
>  	int probe_timing_enter, probe_timing_exit;
>  
>  	if (flash->probe_timing > 0) 
>  		probe_timing_enter = probe_timing_exit = flash->probe_timing;
>  	else if (flash->probe_timing == TIMING_ZERO) { /* No delay. */
>  		probe_timing_enter = probe_timing_exit = 0;
>  	} else if (flash->probe_timing == TIMING_FIXME) { /* == _IGNORED */
> -		printf_debug("Chip lacks correct probe timing information, "
> +		msg_cdbg("Chip lacks correct probe timing information, "
>  			     "using default 10mS/40uS. ");
>  		probe_timing_enter = 10000;
>  		probe_timing_exit = 40;
>  	} else {
> -		printf("Chip has negative value in probe_timing, failing "
> +		msg_cdbg("Chip has negative value in probe_timing, failing "
>  		       "without chip access\n");

cinfo or even cerr please

>  		return 0;
>  	}
>  
>  	/* Issue JEDEC Product ID Entry command */
>  	chip_writeb(0xAA, bios + (0x5555 & mask));
>  	if (probe_timing_enter)
>  		programmer_delay(10);
>  	chip_writeb(0x55, bios + (0x2AAA & mask));
>  	if (probe_timing_enter)
>  		programmer_delay(10);
>  	chip_writeb(0x90, bios + (0x5555 & mask));
>  	if (probe_timing_enter)
> @@ -156,50 +156,50 @@ int probe_jedec_common(struct flashchip *flash, unsigned int mask)
>  	if ((flash->feature_bits & FEATURE_SHORT_RESET) == FEATURE_LONG_RESET)
>  	{
>  		chip_writeb(0xAA, bios + (0x5555 & mask));
>  		if (probe_timing_exit)
>  			programmer_delay(10);
>  		chip_writeb(0x55, bios + (0x2AAA & mask));
>  		if (probe_timing_exit)
>  			programmer_delay(10);
>  	}
>  	chip_writeb(0xF0, bios + (0x5555 & mask));
>  	if (probe_timing_exit)
>  		programmer_delay(probe_timing_exit);
>  
> -	printf_debug("%s: id1 0x%02x, id2 0x%02x", __func__, largeid1, largeid2);
> +	msg_cdbg("%s: id1 0x%02x, id2 0x%02x", __func__, largeid1, largeid2);
>  	if (!oddparity(id1))
> -		printf_debug(", id1 parity violation");
> +		msg_cdbg(", id1 parity violation");
>  
>  	/* Read the product ID location again. We should now see normal flash contents. */
>  	flashcontent1 = chip_readb(bios);
>  	flashcontent2 = chip_readb(bios + 0x01);
>  
>  	/* Check if it is a continuation ID, this should be a while loop. */
>  	if (flashcontent1 == 0x7F) {
>  		flashcontent1 <<= 8;
>  		flashcontent1 |= chip_readb(bios + 0x100);
>  	}
>  	if (flashcontent2 == 0x7F) {
>  		flashcontent2 <<= 8;
>  		flashcontent2 |= chip_readb(bios + 0x101);
>  	}
>  
>  	if (largeid1 == flashcontent1)
> -		printf_debug(", id1 is normal flash content");
> +		msg_cdbg(", id1 is normal flash content");
>  	if (largeid2 == flashcontent2)
> -		printf_debug(", id2 is normal flash content");
> +		msg_cdbg(", id2 is normal flash content");
>  
> -	printf_debug("\n");
> +	msg_cdbg("\n");
>  	if (largeid1 != flash->manufacture_id || largeid2 != flash->model_id)
>  		return 0;
>  
>  	if (flash->feature_bits & FEATURE_REGISTERMAP)
>  		map_flash_registers(flash);
>  
>  	return 1;
>  }
>  
>  int erase_sector_jedec_common(struct flashchip *flash, unsigned int page,
>  			      unsigned int pagesize, unsigned int mask)
>  {
>  	chipaddr bios = flash->virtual_memory;
> @@ -213,27 +213,27 @@ int erase_sector_jedec_common(struct flashchip *flash, unsigned int page,
>  	programmer_delay(10);
>  
>  	chip_writeb(0xAA, bios + (0x5555 & mask));
>  	programmer_delay(10);
>  	chip_writeb(0x55, bios + (0x2AAA & mask));
>  	programmer_delay(10);
>  	chip_writeb(0x30, bios + page);
>  	programmer_delay(10);
>  
>  	/* wait for Toggle bit ready         */
>  	toggle_ready_jedec_slow(bios);
>  
>  	if (check_erased_range(flash, page, pagesize)) {
> -		fprintf(stderr,"ERASE FAILED!\n");
> +		msg_cerr("ERASE FAILED!\n");
>  		return -1;
>  	}
>  	return 0;
>  }
>  
>  int erase_block_jedec_common(struct flashchip *flash, unsigned int block,
>  			     unsigned int blocksize, unsigned int mask)
>  {
>  	chipaddr bios = flash->virtual_memory;
>  
>  	/*  Issue the Sector Erase command   */
>  	chip_writeb(0xAA, bios + (0x5555 & mask));
>  	programmer_delay(10);
> @@ -243,27 +243,27 @@ int erase_block_jedec_common(struct flashchip *flash, unsigned int block,
>  	programmer_delay(10);
>  
>  	chip_writeb(0xAA, bios + (0x5555 & mask));
>  	programmer_delay(10);
>  	chip_writeb(0x55, bios + (0x2AAA & mask));
>  	programmer_delay(10);
>  	chip_writeb(0x50, bios + block);
>  	programmer_delay(10);
>  
>  	/* wait for Toggle bit ready         */
>  	toggle_ready_jedec_slow(bios);
>  
>  	if (check_erased_range(flash, block, blocksize)) {
> -		fprintf(stderr,"ERASE FAILED!\n");
> +		msg_cerr("ERASE FAILED!\n");
>  		return -1;
>  	}
>  	return 0;
>  }
>  
>  int erase_chip_jedec_common(struct flashchip *flash, unsigned int mask)
>  {
>  	int total_size = flash->total_size * 1024;
>  	chipaddr bios = flash->virtual_memory;
>  
>  	/*  Issue the JEDEC Chip Erase command   */
>  	chip_writeb(0xAA, bios + (0x5555 & mask));
>  	programmer_delay(10);
> @@ -272,27 +272,27 @@ int erase_chip_jedec_common(struct flashchip *flash, unsigned int mask)
>  	chip_writeb(0x80, bios + (0x5555 & mask));
>  	programmer_delay(10);
>  
>  	chip_writeb(0xAA, bios + (0x5555 & mask));
>  	programmer_delay(10);
>  	chip_writeb(0x55, bios + (0x2AAA & mask));
>  	programmer_delay(10);
>  	chip_writeb(0x10, bios + (0x5555 & mask));
>  	programmer_delay(10);
>  
>  	toggle_ready_jedec_slow(bios);
>  
>  	if (check_erased_range(flash, 0, total_size)) {
> -		fprintf(stderr,"ERASE FAILED!\n");
> +		msg_cerr("ERASE FAILED!\n");
>  		return -1;
>  	}
>  	return 0;
>  }
>  
>  int write_byte_program_jedec_common(struct flashchip *flash, uint8_t *src,
>  			     chipaddr dst, unsigned int mask)
>  {
>  	int tried = 0, failed = 0;
>  	chipaddr bios = flash->virtual_memory;
>  
>  	/* If the data is 0xFF, don't program it and don't complain. */
>  	if (*src == 0xFF) {
> @@ -320,27 +320,27 @@ retry:
>  int write_sector_jedec_common(struct flashchip *flash, uint8_t *src,
>  		       chipaddr dst, unsigned int page_size, unsigned int mask)
>  {
>  	int i, failed = 0;
>  	chipaddr olddst;
>  
>  	olddst = dst;
>  	for (i = 0; i < page_size; i++) {
>  		if (write_byte_program_jedec_common(flash, src, dst, mask))
>  			failed = 1;
>  		dst++, src++;
>  	}
>  	if (failed)
> -		fprintf(stderr, " writing sector at 0x%lx failed!\n", olddst);
> +		msg_cdbg(" writing sector at 0x%lx failed!\n", olddst);

cerr please

>  
>  	return failed;
>  }
>  
>  int write_page_write_jedec_common(struct flashchip *flash, uint8_t *src,
>  			   int start, int page_size, unsigned int mask)
>  {
>  	int i, tried = 0, failed;
>  	uint8_t *s = src;
>  	chipaddr bios = flash->virtual_memory;
>  	chipaddr dst = bios + start;
>  	chipaddr d = dst;
>  
> @@ -354,121 +354,121 @@ retry:
>  		if (*src != 0xFF)
>  			chip_writeb(*src, dst);
>  		dst++;
>  		src++;
>  	}
>  
>  	toggle_ready_jedec(dst - 1);
>  
>  	dst = d;
>  	src = s;
>  	failed = verify_range(flash, src, start, page_size, NULL);
>  
>  	if (failed && tried++ < MAX_REFLASH_TRIES) {
> -		fprintf(stderr, "retrying.\n");
> +		msg_cdbg("retrying.\n");

cinfo or maybe even cerr

>  		goto retry;
>  	}
>  	if (failed) {
> -		fprintf(stderr, " page 0x%lx failed!\n",
> +		msg_cdbg(" page 0x%lx failed!\n",

cerr please

>  			(d - bios) / page_size);
>  	}
>  	return failed;
>  }
>  
>  int write_jedec(struct flashchip *flash, uint8_t *buf)
>  {
>  	int mask;
>  	int i, failed = 0;
>  	int total_size = flash->total_size * 1024;
>  	int page_size = flash->page_size;
>  
>  	mask = getaddrmask(flash);
>  
>  	if (erase_chip_jedec(flash)) {
> -		fprintf(stderr,"ERASE FAILED!\n");
> +		msg_cerr("ERASE FAILED!\n");
>  		return -1;
>  	}
>  	
> -	printf("Programming page: ");
> +	msg_cinfo("Programming page: ");
>  	for (i = 0; i < total_size / page_size; i++) {
> -		printf("%04d at address: 0x%08x", i, i * page_size);
> +		msg_cdbg("%04d at address: 0x%08x", i, i * page_size);
>  		if (write_page_write_jedec_common(flash, buf + i * page_size,
>  					   i * page_size, page_size, mask))
>  			failed = 1;
> -		printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
> +		msg_cdbg("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");

For the progress printing level (cbdg/cinfo) please see below.

>  	}
> -	printf("\n");
> +	msg_cinfo("DONE!\n");
>  
>  	return failed;
>  }
>  
>  int write_jedec_1(struct flashchip *flash, uint8_t * buf)
>  {
>  	int i;
>  	chipaddr bios = flash->virtual_memory;
>  	chipaddr dst = bios;
>  	int mask;
>  
>  	mask = getaddrmask(flash);
>  
>  	programmer_delay(10);
>  	if (erase_flash(flash)) {
> -		fprintf(stderr, "ERASE FAILED!\n");
> +		msg_cerr("ERASE FAILED!\n");
>  		return -1;
>  	}
>  
> -	printf("Programming page: ");
> +	msg_cinfo("Programming page: ");
>  	for (i = 0; i < flash->total_size; i++) {
>  		if ((i & 0x3) == 0)
> -			printf("address: 0x%08lx", (unsigned long)i * 1024);
> +			msg_cdbg("address: 0x%08lx", (unsigned long)i * 1024);
>  
>                  write_sector_jedec_common(flash, buf + i * 1024, dst + i * 1024, 1024, mask);
>  
>  		if ((i & 0x3) == 0)
> -			printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
> +			msg_cdbg("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");

progress printing in the loop above is inconsistent with other chips
which use cinfo instead of cdbg. To be honest, all that progress
printing should be converted to a decent show_progress() function anyway
once 0.9.2 is out. Can you decide to use either cinfo here (less changes
elsewhere) or cdbg in all other progress statements?

>  	}
>  
> -	printf("\n");
> +	msg_cinfo("DONE!\n");
>  	return 0;
>  }
>  
> diff --git a/m29f400bt.c b/m29f400bt.c
> index c394074..ec97386 100644
> --- a/m29f400bt.c
> +++ b/m29f400bt.c
> @@ -32,28 +32,27 @@ void write_page_m29f400bt(chipaddr bios, uint8_t *src,
>  {
>  	int i;
>  
>  	for (i = 0; i < page_size; i++) {
>  		chip_writeb(0xAA, bios + 0xAAA);
>  		chip_writeb(0x55, bios + 0x555);
>  		chip_writeb(0xA0, bios + 0xAAA);
>  
>  		/* transfer data from source to destination */
>  		chip_writeb(*src, dst);
>  		//chip_writeb(0xF0, bios);
>  		//programmer_delay(5);
>  		toggle_ready_jedec(dst);
> -		printf
> -		    ("Value in the flash at address 0x%lx = %#x, want %#x\n",
> +		msg_cinfo("Value in the flash at address 0x%lx = %#x, want %#x\n",

cerr please. This means a write failed

>  		     (dst - bios), chip_readb(dst), *src);
>  		dst++;
>  		src++;
>  	}
>  }
> --- a/sharplhf00l04.c
> +++ b/sharplhf00l04.c
> @@ -24,60 +24,60 @@
>  
>  /* FIXME: The datasheet is unclear whether we should use toggle_ready_jedec
>   * or wait_82802ab.
>   */
>  
>  int erase_lhf00l04_block(struct flashchip *flash, unsigned int blockaddr, unsigned int blocklen)
>  {
>  	chipaddr bios = flash->virtual_memory + blockaddr;
>  	chipaddr wrprotect = flash->virtual_registers + blockaddr + 2;
>  	uint8_t status;
>  
>  	// clear status register
>  	chip_writeb(0x50, bios);
> -	printf("Erase at 0x%lx\n", bios);
> +	msg_cinfo("Erase at 0x%lx\n", bios);

cdbg please

>  	status = wait_82802ab(flash->virtual_memory);
>  	print_status_82802ab(status);
>  	// clear write protect
> -	printf("write protect is at 0x%lx\n", (wrprotect));
> -	printf("write protect is 0x%x\n", chip_readb(wrprotect));
> +	msg_cinfo("write protect is at 0x%lx\n", (wrprotect));
> +	msg_cinfo("write protect is 0x%x\n", chip_readb(wrprotect));
>  	chip_writeb(0, wrprotect);
> -	printf("write protect is 0x%x\n", chip_readb(wrprotect));
> +	msg_cinfo("write protect is 0x%x\n", chip_readb(wrprotect));

cdbg or even cspew for all wrprotect lines above please

>  
>  	// now start it
>  	chip_writeb(0x20, bios);
>  	chip_writeb(0xd0, bios);
>  	programmer_delay(10);
>  	// now let's see what the register is
>  	status = wait_82802ab(flash->virtual_memory);
>  	print_status_82802ab(status);
> -	printf("DONE BLOCK 0x%x\n", blockaddr);
> +	msg_cinfo("DONE BLOCK 0x%x\n", blockaddr);
>  
>  	if (check_erased_range(flash, blockaddr, blocklen)) {
> -		fprintf(stderr, "ERASE FAILED!\n");
> +		msg_cerr("ERASE FAILED!\n");
>  		return -1;
>  	}
>  	return 0;
>  }
>  
> --- a/spi25.c
> +++ b/spi25.c
> @@ -29,125 +29,125 @@
>  #include "spi.h"
>  
>  void spi_prettyprint_status_register(struct flashchip *flash);
>  
>  static int spi_rdid(unsigned char *readarr, int bytes)
>  {
>  	const unsigned char cmd[JEDEC_RDID_OUTSIZE] = { JEDEC_RDID };
>  	int ret;
>  	int i;
>  
>  	ret = spi_send_command(sizeof(cmd), bytes, cmd, readarr);
>  	if (ret)
>  		return ret;
> -	printf_debug("RDID returned");
> +	msg_cdbg("RDID returned");

cspew please

>  	for (i = 0; i < bytes; i++)
> -		printf_debug(" 0x%02x", readarr[i]);
> -	printf_debug(". ");
> +		msg_cdbg(" 0x%02x", readarr[i]);

same here

> +	msg_cdbg(". ");

same here

>  	return 0;
>  }
>  
>  static int spi_rems(unsigned char *readarr)
>  {
>  	unsigned char cmd[JEDEC_REMS_OUTSIZE] = { JEDEC_REMS, 0, 0, 0 };
>  	uint32_t readaddr;
>  	int ret;
>  
>  	ret = spi_send_command(sizeof(cmd), JEDEC_REMS_INSIZE, cmd, readarr);
>  	if (ret == SPI_INVALID_ADDRESS) {
>  		/* Find the lowest even address allowed for reads. */
>  		readaddr = (spi_get_valid_read_addr() + 1) & ~1;
>  		cmd[1] = (readaddr >> 16) & 0xff,
>  		cmd[2] = (readaddr >> 8) & 0xff,
>  		cmd[3] = (readaddr >> 0) & 0xff,
>  		ret = spi_send_command(sizeof(cmd), JEDEC_REMS_INSIZE, cmd, readarr);
>  	}
>  	if (ret)
>  		return ret;
> -	printf_debug("REMS returned %02x %02x. ", readarr[0], readarr[1]);
> +	msg_cdbg("REMS returned %02x %02x. ", readarr[0], readarr[1]);

cspew please

>  	return 0;
>  }
>  
>  static int spi_res(unsigned char *readarr)
>  {
>  	unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
>  	uint32_t readaddr;
>  	int ret;
>  
>  	ret = spi_send_command(sizeof(cmd), JEDEC_RES_INSIZE, cmd, readarr);
>  	if (ret == SPI_INVALID_ADDRESS) {
>  		/* Find the lowest even address allowed for reads. */
>  		readaddr = (spi_get_valid_read_addr() + 1) & ~1;
>  		cmd[1] = (readaddr >> 16) & 0xff,
>  		cmd[2] = (readaddr >> 8) & 0xff,
>  		cmd[3] = (readaddr >> 0) & 0xff,
>  		ret = spi_send_command(sizeof(cmd), JEDEC_RES_INSIZE, cmd, readarr);
>  	}
>  	if (ret)
>  		return ret;
> -	printf_debug("RES returned %02x. ", readarr[0]);
> +	msg_cdbg("RES returned %02x. ", readarr[0]);

cspew please

>  	return 0;
>  }
>  
>  int spi_write_enable(void)
>  {
>  	const unsigned char cmd[JEDEC_WREN_OUTSIZE] = { JEDEC_WREN };
>  	int result;
>  
>  	/* Send WREN (Write Enable) */
>  	result = spi_send_command(sizeof(cmd), 0, cmd, NULL);
>  
>  	if (result)
> -		fprintf(stderr, "%s failed\n", __func__);
> +		msg_cerr("%s failed\n", __func__);
>  
>  	return result;
>  }
>  
>  int spi_write_disable(void)
>  {
>  	const unsigned char cmd[JEDEC_WRDI_OUTSIZE] = { JEDEC_WRDI };
>  
>  	/* Send WRDI (Write Disable) */
>  	return spi_send_command(sizeof(cmd), 0, cmd, NULL);
>  }
>  
>  static int probe_spi_rdid_generic(struct flashchip *flash, int bytes)
>  {
>  	unsigned char readarr[4];
>  	uint32_t id1;
>  	uint32_t id2;
>  
>  	if (spi_rdid(readarr, bytes))
>  		return 0;
>  
>  	if (!oddparity(readarr[0]))
> -		printf_debug("RDID byte 0 parity violation. ");
> +		msg_cdbg("RDID byte 0 parity violation. ");
>  
>  	/* Check if this is a continuation vendor ID */
>  	if (readarr[0] == 0x7f) {
>  		if (!oddparity(readarr[1]))
> -			printf_debug("RDID byte 1 parity violation. ");
> +			msg_cdbg("RDID byte 1 parity violation. ");
>  		id1 = (readarr[0] << 8) | readarr[1];
>  		id2 = readarr[2];
>  		if (bytes > 3) {
>  			id2 <<= 8;
>  			id2 |= readarr[3];
>  		}
>  	} else {
>  		id1 = readarr[0];
>  		id2 = (readarr[1] << 8) | readarr[2];
>  	}
>  
> -	printf_debug("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);
> +	msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);
>  
>  	if (id1 == flash->manufacture_id && id2 == flash->model_id) {
>  		/* Print the status register to tell the
>  		 * user about possible write protection.
>  		 */
>  		spi_prettyprint_status_register(flash);
>  
>  		return 1;
>  	}
>  
>  	/* Test if this is a pure vendor match. */
>  	if (id1 == flash->manufacture_id &&
>  	    GENERIC_DEVICE_ID == flash->model_id)
> @@ -182,44 +182,44 @@ int probe_spi_rdid4(struct flashchip *flash)
>  	case SPI_CONTROLLER_FT2232:
>  #endif
>  #if DUMMY_SUPPORT == 1
>  	case SPI_CONTROLLER_DUMMY:
>  #endif
>  #if BUSPIRATE_SPI_SUPPORT == 1
>  	case SPI_CONTROLLER_BUSPIRATE:
>  #endif
>  #if DEDIPROG_SUPPORT == 1
>  	case SPI_CONTROLLER_DEDIPROG:
>  #endif
>  		return probe_spi_rdid_generic(flash, 4);
>  	default:
> -		printf_debug("4b ID not supported on this SPI controller\n");
> +		msg_cdbg("4b ID not supported on this SPI controller\n");

Hm. Maybe upgrade to cinfo?

>  	}
>  
>  	return 0;
>  }
>  
> @@ -452,54 +452,54 @@ int spi_chip_erase_c7(struct flashchip *flash)
>  		.writecnt	= JEDEC_CE_C7_OUTSIZE,
>  		.writearr	= (const unsigned char[]){ JEDEC_CE_C7 },
>  		.readcnt	= 0,
>  		.readarr	= NULL,
>  	}, {
>  		.writecnt	= 0,
>  		.writearr	= NULL,
>  		.readcnt	= 0,
>  		.readarr	= NULL,
>  	}};
>  
>  	result = spi_disable_blockprotect();
>  	if (result) {
> -		fprintf(stderr, "spi_disable_blockprotect failed\n");
> +		msg_cerr("spi_disable_blockprotect failed\n");
>  		return result;
>  	}
>  
>  	result = spi_send_multicommand(cmds);
>  	if (result) {
> -		fprintf(stderr, "%s failed during command execution\n", __func__);
> +		msg_cerr("%s failed during command execution\n", __func__);
>  		return result;
>  	}
>  	/* Wait until the Write-In-Progress bit is cleared.
>  	 * This usually takes 1-85 s, so wait in 1 s steps.
>  	 */
>  	/* FIXME: We assume spi_read_status_register will never fail. */
>  	while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
>  		programmer_delay(1000 * 1000);
>  	if (check_erased_range(flash, 0, flash->total_size * 1024)) {
> -		fprintf(stderr, "ERASE FAILED!\n");
> +		msg_cerr("ERASE FAILED!\n");
>  		return -1;
>  	}
>  	return 0;
>  }
>  
>  int spi_chip_erase_60_c7(struct flashchip *flash)

To be honest, this function should be killed completely. It is an unused
relic from a time where we had no block erasers.

>  {
>  	int result;
>  	result = spi_chip_erase_60(flash);
>  	if (result) {
> -		printf_debug("spi_chip_erase_60 failed, trying c7\n");
> +		msg_cdbg("spi_chip_erase_60 failed, trying c7\n");
>  		result = spi_chip_erase_c7(flash);
>  	}
>  	return result;
>  }
>  
> --- a/sst_fwhub.c
> +++ b/sst_fwhub.c
> @@ -23,59 +23,59 @@
>  /* Adapted from the Intel FW hub stuff for 82802ax parts. */
>  
>  #include <stdlib.h>
>  #include <string.h>
>  #include "flash.h"
>  #include "chipdrivers.h"
>  
>  int check_sst_fwhub_block_lock(struct flashchip *flash, int offset)
>  {
>  	chipaddr registers = flash->virtual_registers;
>  	uint8_t blockstatus;
>  
>  	blockstatus = chip_readb(registers + offset + 2);
> -	printf_debug("Lock status for 0x%06x (size 0x%06x) is %02x, ",
> +	msg_cdbg("Lock status for 0x%06x (size 0x%06x) is %02x, ",
>  		     offset, flash->page_size, blockstatus);
>  	switch (blockstatus & 0x3) {
>  	case 0x0:
> -		printf_debug("full access\n");
> +		msg_cdbg("full access\n");
>  		break;
>  	case 0x1:
> -		printf_debug("write locked\n");
> +		msg_cdbg("write locked\n");
>  		break;
>  	case 0x2:
> -		printf_debug("locked open\n");
> +		msg_cdbg("locked open\n");
>  		break;
>  	case 0x3:
> -		printf_debug("write locked down\n");
> +		msg_cdbg("write locked down\n");

cinfo or even cerr?

>  		break;
>  	}
>  	/* Return content of the write_locked bit */
>  	return blockstatus & 0x1;
>  }
>  
>  int clear_sst_fwhub_block_lock(struct flashchip *flash, int offset)
>  {
>  	chipaddr registers = flash->virtual_registers;
>  	uint8_t blockstatus;
>  
>  	blockstatus = check_sst_fwhub_block_lock(flash, offset);
>  
>  	if (blockstatus) {
> -		printf_debug("Trying to clear lock for 0x%06x... ", offset)
> +		msg_cdbg("Trying to clear lock for 0x%06x... ", offset);
>  		chip_writeb(0, registers + offset + 2);
>  
>  		blockstatus = check_sst_fwhub_block_lock(flash, offset);
> -		printf_debug("%s\n", (blockstatus) ? "failed" : "OK");
> +		msg_cdbg("%s\n", (blockstatus) ? "failed" : "OK");

cerr if failed

>  	}
>  
>  	return blockstatus;
>  }
> --- a/stm50flw0x0x.c
> +++ b/stm50flw0x0x.c
> @@ -51,100 +51,99 @@ int unlock_block_stm50flw0x0x(struct flashchip *flash, int offset)
>  	 *
>  	 * Sometimes, the BIOS does this for you; so you propably
>  	 * don't need to worry about that.
>  	 */
>  
>  	/* Check, if it's is a top/bottom-block with 4k-sectors. */
>  	/* TODO: What about the other types? */
>  	if ((offset == 0) ||
>  	    (offset == (flash->model_id == ST_M50FLW080A ? 0xE0000 : 0x10000))
>  	    || (offset == 0xF0000)) {
>  
>  		// unlock each 4k-sector
>  		for (j = 0; j < 0x10000; j += 0x1000) {
> -			printf_debug("unlocking at 0x%x\n", offset + j);
> +			msg_cdbg("unlocking at 0x%x\n", offset + j);
>  			chip_writeb(unlock_sector, wrprotect + offset + j);
>  			if (chip_readb(wrprotect + offset + j) != unlock_sector) {
> -				printf("Cannot unlock sector @ 0x%x\n",
> +				msg_cinfo("Cannot unlock sector @ 0x%x\n",

cerr?

>  				       offset + j);
>  				return -1;
>  			}
>  		}
>  	} else {
> -		printf_debug("unlocking at 0x%x\n", offset);
> +		msg_cdbg("unlocking at 0x%x\n", offset);
>  		chip_writeb(unlock_sector, wrprotect + offset);
>  		if (chip_readb(wrprotect + offset) != unlock_sector) {
> -			printf("Cannot unlock sector @ 0x%x\n", offset);
> +			msg_cinfo("Cannot unlock sector @ 0x%x\n", offset);

cerr?

>  			return -1;
>  		}
>  	}
>  
>  	return 0;
>  }
>  
> --- a/w39v040c.c
> +++ b/w39v040c.c
> @@ -32,17 +32,17 @@ int printlock_w39v040c(struct flashchip *flash)
>  	programmer_delay(10);
>  	chip_writeb(0x90, bios + 0x5555);
>  	programmer_delay(10);
>  
>  	lock = chip_readb(bios + 0xfff2);
>  
>  	chip_writeb(0xAA, bios + 0x5555);
>  	programmer_delay(10);
>  	chip_writeb(0x55, bios + 0x2AAA);
>  	programmer_delay(10);
>  	chip_writeb(0xF0, bios + 0x5555);
>  	programmer_delay(40);
>  
> -	printf("%s: Boot block #TBL is %slocked, rest of chip #WP is %slocked.\n",
> +	msg_cinfo("%s: Boot block #TBL is %slocked, rest of chip #WP is %slocked.\n",

cdbg for consistency with other chips.

>  		__func__, lock & 0x4 ? "" : "un", lock & 0x8 ? "" : "un");
>  	return 0;
>  }


I'd really appreciate it if someone else could take a look at the
generic/programmer patch.
For the programmer stuff there's a simple rule: Anything which is called
often (per access or per chip) should probably be pspew.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the flashrom mailing list