[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