[coreboot] [PATCH] flashrom: Add external programmer delay functions

[Uwe Hermann]

On Fri, Jun 05, 2009 at 06:28:41PM +0200, Carl-Daniel Hailfinger wrote:
> Add external programmer delay functions so external programmers can
> handle the delay on their own if needed.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Acked-by: Uwe Hermann <uwe at hermann-uwe.de>

...but see below.

 
> Index: flashrom-programmer_delay/flash.h
> ===================================================================
> --- flashrom-programmer_delay/flash.h	(Revision 576)
> +++ flashrom-programmer_delay/flash.h	(Arbeitskopie)
> @@ -103,6 +103,8 @@
>  	uint8_t (*chip_readb) (const chipaddr addr);
>  	uint16_t (*chip_readw) (const chipaddr addr);
>  	uint32_t (*chip_readl) (const chipaddr addr);
> +
> +	void (*delay) (int usecs);
>  };
>  
>  extern const struct programmer_entry programmer_table[];
> @@ -118,6 +120,7 @@
>  uint8_t chip_readb(const chipaddr addr);
>  uint16_t chip_readw(const chipaddr addr);
>  uint32_t chip_readl(const chipaddr addr);
> +void programmer_delay(uint32_t usecs);

Why uint32_t here and int above?

  
> Index: flashrom-programmer_delay/spi.c
> ===================================================================
> --- flashrom-programmer_delay/spi.c	(Revision 576)
> +++ flashrom-programmer_delay/spi.c	(Arbeitskopie)
> @@ -425,7 +425,7 @@
>  	 */
>  	/* FIXME: We assume spi_read_status_register will never fail. */
>  	while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> -		sleep(1);
> +		programmer_delay(1000000);

I'd also make this 100 * 1000 as below, easier on the eyes.


>  	return 0;
>  }
>  
> @@ -453,7 +453,7 @@
>  	 */
>  	/* FIXME: We assume spi_read_status_register will never fail. */
>  	while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> -		sleep(1);
> +		programmer_delay(1000000);

Ditto.


>  	return 0;
>  }
>  
> @@ -485,7 +485,7 @@
>  	 * This usually takes 100-4000 ms, so wait in 100 ms steps.
>  	 */
>  	while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> -		usleep(100 * 1000);
> +		programmer_delay(100 * 1000);
>  	return 0;
>  }
  
> Index: flashrom-programmer_delay/am29f040b.c
> ===================================================================
> --- flashrom-programmer_delay/am29f040b.c	(Revision 576)
> +++ flashrom-programmer_delay/am29f040b.c	(Arbeitskopie)
> @@ -29,7 +29,7 @@
>  	chip_writeb(0x55, bios + 0x2AA);
>  	chip_writeb(0x30, bios + address);
>  
> -	sleep(2);
> +	programmer_delay(1000000 * 2);

Maybe 2 * 1000 * 1000, same format as above.


> Index: flashrom-programmer_delay/internal.c
> ===================================================================
> --- flashrom-programmer_delay/internal.c	(Revision 576)
> +++ flashrom-programmer_delay/internal.c	(Arbeitskopie)
> @@ -195,6 +195,18 @@
>  	return *(volatile uint32_t *) addr;
>  }
>  
> +void internal_delay(int usecs)
> +{
> +	/* If the delay is >1 s, use usleep because timing does not need to
> +	 * be so precise.

Hm, but why not use myusec_delay() always? Does it have drawbacks?

> +	 */
> +	if (usecs > 1000000) {
> +		usleep(usecs);
> +	} else {
> +		myusec_delay(usecs);
> +	}
> +}


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org