[LinuxBIOS] [PATCH] flashrom: add MX25L4005 erase/write support

Harald Gutmann harald.gutmann at gmx.net
Wed Oct 17 20:22:05 CEST 2007


Am Mittwoch, 17. Oktober 2007 14:14:17 schrieb Carl-Daniel Hailfinger:
> On 15.10.2007 01:28, Harald Gutmann wrote:
> > Am Montag, 15. Oktober 2007 01:06:00 schrieb Carl-Daniel Hailfinger:
> >> Great, thanks! The code I have also is not polished, so even if your
> >> code is ugly, it is better than no code. We can clean up that stuff
> >> together.
> >
> > yes, that's right!
> > here it is.
> >
> > the patch is done with:
> > Revision: 2850
>
> I have rearranged some code, so your patch does no longer apply, but it
> should be easy to fix.
fine!

> > diff -ubrN ../../flashrom.original/board_enable.c
> > flashrom.new/board_enable.c ---
> > ../../flashrom.original/board_enable.c	2007-10-15 01:14:29.000000000
> > +0200 +++ flashrom.new/board_enable.c	2007-10-14 20:28:41.000000000 +0200
>
> This is now in spi.c
>
> > @@ -37,7 +37,7 @@
> >  #define JEDEC_RDID_OUTSIZE	0x01
> >  #define JEDEC_RDID_INSIZE	0x03
> >
> > -static uint16_t it8716f_flashport = 0;
> > +uint16_t it8716f_flashport = 0;
> >
> >  /* Generic Super I/O helper functions */
> >  uint8_t regval(uint16_t port, uint8_t reg)
> > @@ -111,7 +111,7 @@
> >     whereas the IT8716F splits commands internally into address and
> > non-address commands with the address in inverse wire order. That's why
> > the register ordering in case 4 and 5 may seem strange. */
> > -static int it8716f_spi_command(uint16_t port, unsigned char writecnt,
> > unsigned char readcnt, const unsigned char *writearr, unsigned char
> > *readarr) +int it8716f_spi_command(uint16_t port, unsigned char writecnt,
> > unsigned char readcnt, const unsigned char *writearr, unsigned char
> > *readarr) {
> >  	uint8_t busy, writeenc;
> >  	do {
> > diff -ubrN ../../flashrom.original/board_enable.h
> > flashrom.new/board_enable.h ---
> > ../../flashrom.original/board_enable.h	1970-01-01 01:00:00.000000000
> > +0100 +++ flashrom.new/board_enable.h	2007-10-14 21:09:18.000000000 +0200
>
> Perhaps move to spi.h?
>
> > @@ -0,0 +1,7 @@
> > +#ifndef boardenableh
> > +#define boardenableh
> > +uint16_t it8716f_flashport;
> > +uint8_t regval(uint16_t port, uint8_t reg);
> > +void regwrite(uint16_t port, uint8_t reg, uint8_t val);
> > +int it8716f_spi_command(uint16_t port, unsigned char writecnt, unsigned
> > char readcnt, const unsigned char *writearr, unsigned char *readarr);
> > +#endif
> > diff -ubrN ../../flashrom.original/flashchips.c flashrom.new/flashchips.c
> > --- ../../flashrom.original/flashchips.c	2007-10-15 01:14:28.000000000
> > +0200 +++ flashrom.new/flashchips.c	2007-10-15 01:24:58.000000000 +0200
> > @@ -39,7 +39,7 @@
> >  	{"Mx29f002",	MX_ID,		MX_29F002,	256, 64 * 1024,
> >  	 probe_29f002,	erase_29f002, 	write_29f002},
> >  	{"MX25L4005",	MX_ID,		MX_25L4005,	512, 4 * 1024,
> > -	 probe_spi,	NULL,		NULL},
> > +	 probe_spi,	erase_25l4005,	write_25l4005},
> >  	{"SST29EE020A", SST_ID,		SST_29EE020A,	256, 128,
> >  	 probe_jedec,	erase_chip_jedec, write_jedec},
> >  	{"SST28SF040A", SST_ID,		SST_28SF040,	512, 256,
>
> OK
>
> > diff -ubrN ../../flashrom.original/flash.h flashrom.new/flash.h
> > --- ../../flashrom.original/flash.h	2007-10-15 01:14:28.000000000 +0200
> > +++ flashrom.new/flash.h	2007-10-14 19:13:52.000000000 +0200
> > @@ -294,4 +294,10 @@
> >  /* w49f002u.c */
> >  int write_49f002(struct flashchip *flash, uint8_t *buf);
> >
> > +/* mx25l4005.c */
> > +// probe
the "//probe" could  be deleted.

> > +int write_25l4005(struct flashchip *flash, uint8_t *buf);
> > +int erase_25l4005(struct flashchip *flash);
> > +int read_25l4005(struct flashchip *flash, uint8_t *buf);
> > +
> >  #endif				/* !__FLASH_H__ */
>
> OK
>
> > diff -ubrN ../../flashrom.original/Makefile flashrom.new/Makefile
> > --- ../../flashrom.original/Makefile	2007-10-15 01:14:29.000000000 +0200
> > +++ flashrom.new/Makefile	2007-10-15 01:20:16.000000000 +0200
> > @@ -24,7 +24,7 @@
> >  	am29f040b.o mx29f002.o sst39sf020.o m29f400bt.o w49f002u.o \
> >  	82802ab.o msys_doc.o pm49fl004.o sst49lf040.o sst49lfxxxc.o \
> >  	sst_fwhub.o layout.o lbtable.o flashchips.o flashrom.o \
> > -	sharplhf00l04.o w29ee011.o
> > +	sharplhf00l04.o w29ee011.o mx25l4005.o
> >
> >  all: pciutils dep $(PROGRAM)
> >
> > @@ -33,7 +33,7 @@
> >  	$(STRIP) $(STRIP_ARGS) $(PROGRAM)
> >
> >  clean:
> > -	rm -f *.o *~
> > +	rm -f *.o *~ flashrom
> >
> >  distclean: clean
> >  	rm -f $(PROGRAM) .dependencies
>
> Skip the make clean changes. We have make distclean.
like mentioned it's in distclean.

> > diff -ubrN ../../flashrom.original/mx25l4005.c flashrom.new/mx25l4005.c
> > --- ../../flashrom.original/mx25l4005.c	1970-01-01 01:00:00.000000000
> > +0100 +++ flashrom.new/mx25l4005.c	2007-10-15 01:17:56.000000000 +0200 @@
> > -0,0 +1,57 @@
> > +#include <stdint.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include "flash.h"
> > +#include "board_enable.h"
> > +
> > +static void check_n_write_enable() {
> > +	uint8_t result[3] = {0, 0, 0};
> > +	uint8_t command[5] = {0x06, 0, 0, 0, 0};
> > +	// Send WREN (Write Enable)
> > +	it8716f_spi_command(it8716f_flashport, 1, 0, command, result);
>
> generic_spi_command
>
> > +	uint8_t reg=regval(it8716f_flashport,0x24);
> > +	reg|=(1<<4);
> > +	regwrite(it8716f_flashport,0x24,reg);
>
> I have absolutely no idea what you are trying to do here. According to
> my data sheet, your code does the following:
> * Enable multiple byte mode, set clock to 33/2 MHz, read the last JEDEC
> command sent to the flash chip
> * Set bit 4 of the last JEDEC command
> * Enable multiple byte mode, set clock to 33/2 MHz, write the modified
> JEDEC command
sry, i also don't know exactly, a friend of mine hacked that (and i just have 
started to learn c.)
and like told the code was hacked in little time, but worked.

> Please note that none of these three lines send or receive anything
> from/to the chip.
>
> > +
> > +}
> > +
> > +static void write_disable() {
> > +	uint8_t result[3] = {0, 0, 0};
> > +	uint8_t command[5] = {0x04, 0, 0, 0, 0};
> > +	// Send WRDI (Write Disable)
> > +	it8716f_spi_command(it8716f_flashport, 1, 0, command, result);
>
> generic_spi_command
>
> > +	uint8_t reg=regval(it8716f_flashport,0x24);
> > +	reg&=~(1<<4);
> > +	regwrite(it8716f_flashport,0x24,reg);
>
> Same as above: What are you trying to do?
same as above: i don't know.

>
> > +}
> > +
> > +void write256b(int block, uint8_t *buf, uint8_t *bios ) {
> > +	check_n_write_enable();
> > +	outb(0x06,it8716f_flashport+1);
> > +	outb((3<<4),it8716f_flashport);
> > +	int i;
> > +	for (i=0;i<256;++i) {
> > +		bios[256*block+i]=buf[256*block+i];
> > +	}
this splitting is because of the superIO and not of the spi chip programmend.
look at the end of the message. (*)

> > +	outb(0,it8716f_flashport);
> > +	write_disable();
> > +	usleep (1000);
>
> Check the chip status register instead fo waiting.
we tried that before, but it didn't work for any reason,
but i don't know what the problem was, but the first working result was with 
the hardcoded sleep.

>
> > +}
>
> OK, but we probably need a generic_spi_command_prepare and
> generic_spi_command_stop
>
> > +
> > +int write_25l4005(struct flashchip *flash, uint8_t *buf) {
> > +	int total_size=1024*flash->total_size;
> > +	int i;
> > +	for (i=0;i<total_size/256;++i) {
> > +		write256b(i, buf, (uint8_t*)flash->virtual_memory);
> > +	}
> > +return 0;
> > +}
>
> OK
>
> > +
> > +int erase_25l4005(struct flashchip *flash) {
> > +check_n_write_enable();
> > +uint8_t result[3]={0, 0, 0};
> > +uint8_t command[5]={0xc7, 0, 0, 0, 0};
> > +it8716f_spi_command(it8716f_flashport, 1, 0, command, result);
>
> generic_spi_command
>
> > +write_disable();
> > +sleep (3);
> > +// the chip needs some time, until it responses again.
> > +return 0;
>
> Can you instead read the flash chip status register in a loop? That way,
> you don't have to hardcode the sleep duration.
would be better.

>
> > +}
>
> Overall, I like it.
>
> My version of the write patch follows. Please test.
tested and approved to work!

(*) but just another thing: it would be good not to fix that code just for the 
MX25L4005 chip, because there are much more other chips out which can be 
programmed the same way. for the chip it was originally written, it works and 
that's really fine! 

regards, harald


> Index: flash.h
> ===================================================================
> --- flash.h	(Revision 2864)
> +++ flash.h	(Arbeitskopie)
> @@ -296,4 +296,10 @@
>  /* w49f002u.c */
>  int write_49f002(struct flashchip *flash, uint8_t *buf);
>
> +/* mx25l4005.c */
> +// probe
> +int write_25l4005(struct flashchip *flash, uint8_t *buf);
> +int erase_25l4005(struct flashchip *flash);
> +int read_25l4005(struct flashchip *flash, uint8_t *buf);
> +
>  #endif				/* !__FLASH_H__ */
> Index: flashchips.c
> ===================================================================
> --- flashchips.c	(Revision 2864)
> +++ flashchips.c	(Arbeitskopie)
> @@ -39,7 +39,7 @@
>  	{"Mx29f002",	MX_ID,		MX_29F002,	256, 64 * 1024,
>  	 probe_29f002,	erase_29f002, 	write_29f002},
>  	{"MX25L4005",	MX_ID,		MX_25L4005,	512, 4 * 1024,
> -	 probe_spi,	NULL,		NULL},
> +	 probe_spi,	erase_25l4005,	write_25l4005},
>  	{"SST29EE020A", SST_ID,		SST_29EE020A,	256, 128,
>  	 probe_jedec,	erase_chip_jedec, write_jedec},
>  	{"SST28SF040A", SST_ID,		SST_28SF040,	512, 256,
> Index: spi.c
> ===================================================================
> --- spi.c	(Revision 2864)
> +++ spi.c	(Arbeitskopie)
> @@ -34,8 +34,16 @@
>  #define JEDEC_RDID_OUTSIZE	0x01
>  #define JEDEC_RDID_INSIZE	0x03
>
> -static uint16_t it8716f_flashport = 0;
> +#define JEDEC_WREN	{0x06}
> +#define JEDEC_WREN_OUTSIZE	0x01
> +#define JEDEC_WREN_INSIZE	0x00
>
> +#define JEDEC_WRDI	{0x04}
> +#define JEDEC_WRDI_OUTSIZE	0x01
> +#define JEDEC_WRDI_INSIZE	0x00
> +
> +uint16_t it8716f_flashport = 0;
> +
>  /* Generic Super I/O helper functions */
>  uint8_t regval(uint16_t port, uint8_t reg)
>  {
> @@ -170,7 +178,7 @@
>  	return 0;
>  }
>
> -static int generic_spi_command(unsigned char writecnt, unsigned char
> readcnt, const unsigned char *writearr, unsigned char *readarr) +int
> generic_spi_command(unsigned char writecnt, unsigned char readcnt, const
> unsigned char *writearr, unsigned char *readarr) {
>  	if (it8716f_flashport)
>  		return it8716f_spi_command(it8716f_flashport, writecnt, readcnt,
> writearr, readarr); @@ -188,6 +196,25 @@
>  	return 0;
>  }
>
> +void generic_spi_write_enable()
> +{
> +	const unsigned char cmd[] = JEDEC_WREN;
> +	unsigned char result[] = {0, 0, 0};
> +
> +	// Send WREN (Write Enable)
> +	generic_spi_command(JEDEC_WREN_OUTSIZE, JEDEC_WREN_INSIZE, cmd, result);
> +
> +}
> +
> +void generic_spi_write_disable()
> +{
> +	const unsigned char cmd[] = JEDEC_WRDI;
> +	unsigned char result[] = {0, 0, 0};
> +
> +	// Send WRDI (Write Disable)
> +	generic_spi_command(JEDEC_WRDI_OUTSIZE, JEDEC_WRDI_INSIZE, cmd, result);
> +}
> +
>  int probe_spi(struct flashchip *flash)
>  {
>  	unsigned char readarr[3];
> Index: Makefile
> ===================================================================
> --- Makefile	(Revision 2864)
> +++ Makefile	(Arbeitskopie)
> @@ -24,7 +24,7 @@
>  	am29f040b.o mx29f002.o sst39sf020.o m29f400bt.o w49f002u.o \
>  	82802ab.o msys_doc.o pm49fl004.o sst49lf040.o sst49lfxxxc.o \
>  	sst_fwhub.o layout.o lbtable.o flashchips.o flashrom.o \
> -	sharplhf00l04.o w29ee011.o spi.o
> +	sharplhf00l04.o w29ee011.o spi.o mx25l4005.o
>
>  all: pciutils dep $(PROGRAM)
>
> Index: spi.h
> ===================================================================
> --- spi.h	(Revision 0)
> +++ spi.h	(Revision 0)
> @@ -0,0 +1,9 @@
> +#ifndef __SPI_H__
> +#define __SPI_H__ 1
> +
> +uint16_t it8716f_flashport;
> +int generic_spi_command(unsigned char writecnt, unsigned char readcnt,
> const unsigned char *writearr, unsigned char *readarr); +void
> generic_spi_write_enable();
> +void generic_spi_write_disable();
> +
> +#endif
> Index: mx25l4005.c
> ===================================================================
> --- mx25l4005.c	(Revision 0)
> +++ mx25l4005.c	(Revision 0)
> @@ -0,0 +1,39 @@
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include "flash.h"
> +#include "spi.h"
> +
> +
> +void write256b(int block, uint8_t *buf, uint8_t *bios ) {
> +	generic_spi_write_enable();
> +	outb(0x06,it8716f_flashport+1);
> +	outb((3<<4),it8716f_flashport);
> +	int i;
> +	for (i=0;i<256;++i) {
> +		bios[256*block+i]=buf[256*block+i];
> +	}
> +	outb(0,it8716f_flashport);
> +	generic_spi_write_disable();
> +	usleep (1000);
> +}
> +
> +int write_25l4005(struct flashchip *flash, uint8_t *buf) {
> +	int total_size=1024*flash->total_size;
> +	int i;
> +	for (i=0;i<total_size/256;++i) {
> +		write256b(i, buf, (uint8_t*)flash->virtual_memory);
> +	}
> +return 0;
> +}
> +
> +int erase_25l4005(struct flashchip *flash) {
> +	generic_spi_write_enable();
> +	uint8_t result[3]={0, 0, 0};
> +	uint8_t command[5]={0xc7, 0, 0, 0, 0};
> +	generic_spi_command(1, 0, command, result);
> +	generic_spi_write_disable();
> +	// the chip needs some time, until it responses again.
> +	sleep (3);
> +	return 0;
> +}






More information about the coreboot mailing list