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 <[EMAIL PROTECTED]>
Uwe.
--
http://www.hermann-uwe.de | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
signature.asc
Description: Digital signature
-- linuxbios mailing list [email protected] http://www.linuxbios.org/mailman/listinfo/linuxbios
