[flashrom] [PATCH] Add support for the Macronix MX28F2000P chip (256KB parallel)

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Sep 12 19:08:09 CEST 2011


Hi Uwe,


Am 11.09.2011 15:15 schrieb Uwe Hermann:
> Add support for the Macronix MX28F2000P chip (256KB parallel).
>
> This 256KB chip has a similar, but not identical, command set to the
> SST SST28SF040A chip. It features a chip erase, block erase, byte write,
> chip ID, and reset command.
>
> In contrast to the SST28SF040A, the reset command is 0xff, 0xff (instead of
> only one 0xff), and the write command is 0x40 (instead of 0x10).
> No locking/unlocking (protect/unprotect) commands are documented, it seems.
>
> Finally, unlike the SST28SF040A, the MX28F2000P requires 12V for write and
> erase operations. Only read/ID is supported with the usual 5V. On the tested
> board the 12V are always supplied on the VPP pin (confirmed via multimeter)
> though, so that's not a problem there.
>
> All operations were successfully tested, thus mark the chip as OK.
>
> Write log:
> http://paste.flashrom.org/view.php?id=804

Thanks for your patch.


> Signed-off-by: Uwe Hermann <uwe at hermann-uwe.de>
> Tested-by: Max Varentsov <admin at phpcode.us> (Meteo0 on IRC)

The coding style is very different from what we use elsewhere. Please
fix that. See below.


> Index: flashchips.c
> ===================================================================
> --- flashchips.c	(Revision 1435)
> +++ flashchips.c	(Arbeitskopie)
> @@ -4458,6 +4458,38 @@
>  
>  	{
>  		.vendor		= "Macronix",
> +		.name		= "MX28F2000P",
> +		.bustype	= BUS_PARALLEL,
> +		.manufacture_id	= MACRONIX_ID,
> +		.model_id	= MACRONIX_MX28F2000P,
> +		.total_size	= 256,
> +		.page_size	= 0, /* unused */

Hm yes, reminds me that we should kill page_size completely.


> +		.feature_bits	= 0,
> +		.tested		= TEST_OK_PREW,
> +		.probe		= probe_mx28f2000p,
> +		.probe_timing	= TIMING_ZERO,
> +		.block_erasers	=
> +		{
> +			{
> +				.eraseblocks = {
> +					{4 * 1024, 4},
> +					{16 * 1024, 14},
> +					{4 * 1024, 4},
> +				},
> +				.block_erase = erase_sector_mx28f2000p,
> +			}, {
> +				.eraseblocks = { {256 * 1024, 1} },
> +				.block_erase = erase_chip_28sf040,
> +			}
> +		},
> +		.unlock		= NULL, /* No locking mechanism available. */
> +		.write		= write_mx28f2000p,
> +		.read		= read_memmapped,
> +		.voltage	= {4500, 5500}, /* Needs 12V for write/erase! */
> +	},
> +
> +	{
> +		.vendor		= "Macronix",
>  		.name		= "MX29F001B",
>  		.bustype	= BUS_PARALLEL,
>  		.manufacture_id	= MACRONIX_ID,
> Index: flashchips.h
> ===================================================================
> --- flashchips.h	(Revision 1435)
> +++ flashchips.h	(Arbeitskopie)
> @@ -368,6 +368,7 @@
>  #define MACRONIX_MX25L1635D	0x2415
>  #define MACRONIX_MX25L1635E	0x2515	/* MX25L1635{E} */
>  #define MACRONIX_MX25L3235D	0x5E16	/* MX25L3225D/MX25L3235D/MX25L3237D */
> +#define MACRONIX_MX28F2000P	0x2a
>  #define MACRONIX_MX29F001B	0x19
>  #define MACRONIX_MX29F001T	0x18
>  #define MACRONIX_MX29F002B	0x34	/* Same as MX29F002NB; N has reset pin n/c. */
> Index: sst28sf040.c
> ===================================================================
> --- sst28sf040.c	(Revision 1435)
> +++ sst28sf040.c	(Arbeitskopie)
> @@ -26,6 +26,7 @@
>  #define AUTO_PG_ERASE1		0x20
>  #define AUTO_PG_ERASE2		0xD0
>  #define AUTO_PGRM		0x10
> +#define AUTO_PGRM2		0x40 /* Used on MX28F2000P */
>  #define CHIP_ERASE		0x30
>  #define RESET			0xFF
>  #define READ_ID			0x90
> @@ -60,7 +61,8 @@
>  	return 0;
>  }
>  
> -int erase_sector_28sf040(struct flashchip *flash, unsigned int address, unsigned int sector_size)
> +static int erase_sector_28sf040_generic(struct flashchip *flash,
> +	unsigned int address, unsigned int sector_size, int toggle_delay)
>  {
>  	chipaddr bios = flash->virtual_memory;
>  
> @@ -68,38 +70,65 @@
>  	chip_writeb(AUTO_PG_ERASE1, bios);
>  	chip_writeb(AUTO_PG_ERASE2, bios + address);
>  
> -	/* wait for Toggle bit ready */
> +	programmer_delay(toggle_delay);
> +
> +	/* Wait for toggle bit ready. */
>  	toggle_ready_jedec(bios);

Can't you achieve the same effect with toggle_ready_jedec_slow?

Besides that, the whole function looks very similar to
erase_block_82802ab(). Can we just kill one of them?


>  
>  	/* FIXME: Check the status register for errors. */
>  	return 0;
>  }
>  
> +int erase_sector_28sf040(struct flashchip *flash, unsigned int address,
> +			 unsigned int sector_size)
> +{
> +	return erase_sector_28sf040_generic(flash, address, sector_size, 0);
> +}
> +
> +int erase_sector_mx28f2000p(struct flashchip *flash, unsigned int address,
> +			    unsigned int sector_size)
> +{
> +	/* Needs 200us delay before the toggle bit checking begins. */
> +	return erase_sector_28sf040_generic(flash, address, sector_size, 200);
> +}
> +
>  /* chunksize is 1 */
> -int write_28sf040(struct flashchip *flash, uint8_t *src, int start, int len)
> +static int write_28sf040_generic(struct flashchip *flash, uint8_t *src,
> +				 int start, int len, uint8_t auto_pgrm)

NACK.
We don't have any erase/write function in flashrom which takes a command
as parameter, and I'd like to keep it that way.
If you really insist on command-as-parameter, we have to rewrite every
single erase/write function in flashrom and while that may be fun for
some people (after all, you can generate a huge patch with a few lines
of sed/perl/...), it would increase our patch queue significantly.

 
>  {
>  	int i;
>  	chipaddr bios = flash->virtual_memory;
>  	chipaddr dst = flash->virtual_memory + start;
>  
>  	for (i = 0; i < len; i++) {
> -		/* transfer data from source to destination */
> +		/* Transfer data from source to destination. */
>  		if (*src == 0xFF) {
>  			dst++, src++;
> -			/* If the data is 0xFF, don't program it */
> +			/* If the data is 0xFF, don't program it. */
>  			continue;
>  		}
> -		/*issue AUTO PROGRAM command */
> -		chip_writeb(AUTO_PGRM, dst);
> +
> +		/* Issue auto program command. */
> +		chip_writeb(auto_pgrm, dst);
>  		chip_writeb(*src++, dst++);
>  
> -		/* wait for Toggle bit ready */
> +		/* Wait for Toggle bit ready. */
>  		toggle_ready_jedec(bios);
>  	}
>  
>  	return 0;
>  }
>  
> +int write_28sf040(struct flashchip *flash, uint8_t *src, int start, int len)
> +{
> +	return write_28sf040_generic(flash, src, start, len, AUTO_PGRM);
> +}
> +
> +int write_mx28f2000p(struct flashchip *flash, uint8_t *src, int start, int len)
> +{
> +	return write_28sf040_generic(flash, src, start, len, AUTO_PGRM2);
> +}
> +
>  static int erase_28sf040(struct flashchip *flash)
>  {
>  	chipaddr bios = flash->virtual_memory;
> @@ -123,3 +152,30 @@
>  	}
>  	return erase_28sf040(flash);
>  }
> +
> +int probe_mx28f2000p(struct flashchip *flash)
> +{
> +	chipaddr bios = flash->virtual_memory;
> +	uint8_t id1, id2;
> +
> +	/* Reset to get a clean state. */
> +	chip_writeb(0xff, bios);
> +	chip_writeb(0xff, bios);
> +
> +	/* Get the vendor and device ID. One 0x90 command per byte needed! */
> +	chip_writeb(0x90, bios);
> +	id1 = chip_readb(bios);
> +	chip_writeb(0x90, bios);
> +	id2 = chip_readb(bios + 0x01);

This needs the same "normal flash content" check as probe_jedec.


> +
> +	msg_cdbg("%s: id1 0x%02x, id2 0x%02x", __func__, id1, id2);
> +
> +	if (!oddparity(id1))
> +		msg_cdbg(", id1 parity violation");
> +
> +	msg_cdbg("\n");
> +	if (id1 != flash->manufacture_id || id2 != flash->model_id)
> +		return 0;
> +
> +	return 1;
> +}    

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the flashrom mailing list