[LinuxBIOS] [PATCH] flashrom: SPI serial flash: The sector/block erase pitfalls

Uwe Hermann uwe at hermann-uwe.de
Fri Oct 19 23:01:20 CEST 2007


On Thu, Oct 18, 2007 at 10:32:01PM +0200, Carl-Daniel Hailfinger wrote:
> Index: util/flashrom/spi.c
> ===================================================================
> --- util/flashrom/spi.c	(Revision 2876)
> +++ util/flashrom/spi.c	(Arbeitskopie)
> @@ -45,17 +45,31 @@
>  #define JEDEC_WRDI_OUTSIZE	0x01
>  #define JEDEC_WRDI_INSIZE	0x00
>  
> -/* Both Chip Erase commands below should work */
> -/* Chip Erase 0x60 */
> +/* Chip Erase 0x60 is supported by Macronix/SST chips */

Missing full stop at end of sentence. Also in other lines below.


>  #define JEDEC_CE_1	{0x60};
>  #define JEDEC_CE_1_OUTSIZE	0x01
>  #define JEDEC_CE_1_INSIZE	0x00
>  
> -/* Chip Erase 0xc7 */
> +/* Chip Erase 0xc7 is supported by EON/Macronix chips */
>  #define JEDEC_CE_2	{0xc7};
>  #define JEDEC_CE_2_OUTSIZE	0x01
>  #define JEDEC_CE_2_INSIZE	0x00
>  
> +/* Block Erase 0x52 is supported by SST chips */
> +#define JEDEC_BE_1	{0x52};
> +#define JEDEC_BE_1_OUTSIZE	0x04
> +#define JEDEC_BE_1_INSIZE	0x00
> +
> +/* Block Erase 0xd8 is supported by EON/Macronix chips */
> +#define JEDEC_BE_2	{0xd8};
> +#define JEDEC_BE_2_OUTSIZE	0x04
> +#define JEDEC_BE_2_INSIZE	0x00
> +
> +/* Sector Erase 0x20 is supported by Macronix/SST chips*/

Missing space and missing full stop.


> +#define JEDEC_SE	{0x20};
> +#define JEDEC_SE_OUTSIZE	0x04
> +#define JEDEC_SE_INSIZE	0x00
> +
>  /* Read Status Register */
>  #define JEDEC_RDSR	{0x05};
>  #define JEDEC_RDSR_OUTSIZE	0x01
> @@ -269,13 +283,52 @@
>  
>  	generic_spi_write_enable();
>  	/* Send CE (Chip Erase) */
> -	generic_spi_command(1, 0, cmd, NULL);
> +	generic_spi_command(JEDEC_CE_2_OUTSIZE, JEDEC_CE_2_INSIZE, cmd, NULL);

Yep, nice.


>  	/* Wait until the Write-In-Progress bit is cleared */
>  	while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
>  		sleep(1);
>  	return 0;
>  }
>  
> +/* Block size is usually
> + * 64k for Macronix
> + * 32k for SST
> + * 4-32k non-uniform for EON
> + */

Please make it a Doxygen comment and describe what the function does.


> +int generic_spi_block_erase(struct flashchip *flash, unsigned long addr)

"flash" can be const?


> +{
> +	unsigned char cmd[JEDEC_BE_2_OUTSIZE] = JEDEC_BE_2;

uint8_t


> +
> +	cmd[1] = (addr & 0x00ff0000) >> 16;
> +	cmd[2] = (addr & 0x0000ff00) >> 8;
> +	cmd[3] = (addr & 0x000000ff);
> +	generic_spi_write_enable();
> +	/* Send BE (Block Erase) */
> +	generic_spi_command(JEDEC_BE_2_OUTSIZE, JEDEC_BE_2_INSIZE, cmd, NULL);
> +	/* Wait until the Write-In-Progress bit is cleared */

Missing full stop.


> +	while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> +		usleep(100 * 1000);

Is the length of this delay documented in the datasheet or is it just an
estimate we hope will work?


> +	return 0;
> +}
> +
> +/* Sector size is usually 4k, though Macronix eliteflash has 64k */
> +int generic_spi_sector_erase(struct flashchip *flash, unsigned long addr)

constify "flash"?


> +{
> +	unsigned char cmd[JEDEC_SE_OUTSIZE] = JEDEC_SE;

uint8_t


> +	cmd[1] = (addr & 0x00ff0000) >> 16;
> +	cmd[2] = (addr & 0x0000ff00) >> 8;
> +	cmd[3] = (addr & 0x000000ff);
> +
> +	generic_spi_write_enable();
> +	/* Send SE (Sector Erase) */
> +	generic_spi_command(JEDEC_SE_OUTSIZE, JEDEC_SE_INSIZE, cmd, NULL);
> +	/* Wait until the Write-In-Progress bit is cleared */

Missing full stop.


> +	while (generic_spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> +		usleep(10 * 1000);

Lower delay than above. By design? What does the datasheet say?


> +	return 0;
> +}
> +
> +/* Page size is usually 256 bytes */
>  void it8716f_spi_page_program(int block, uint8_t *buf, uint8_t *bios) {
>  	int i;


Looks good IMO. With the above fixes:

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


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20071019/cd3a5c68/attachment.sig>


More information about the coreboot mailing list