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 <[email protected]> > Tested-by: Max Varentsov <[email protected]> (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/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
