Don't assert OE and CS simultaneously during electrical test

If you do, and a SIMM is installed in the socket, you will get an
erroneous short detected for any data pins that are currently outputting
a 0. Avoid this by testing OE and CS for 5V shorts separately.

drivers/parallel_flash.c

@@ -140,32 +140,112 @@ void ParallelFlash_EraseChips(uint8_t chipsMask)
  * @param address The start address to erase (must be aligned to a sector boundary)
  * @param length The number of bytes to erase (must be aligned to a sector boundary)
  * @param chipsMask The mask of which chips to erase
+ * @param numEraseSectorGroups The number of erase sector groups we know about
+ * @param eraseSectorGroups The erase sector groups
  * @return True on success, false on failure
  */
-bool ParallelFlash_EraseSectors(uint32_t address, uint32_t length, uint8_t chipsMask)
+bool ParallelFlash_EraseSectors(uint32_t address, uint32_t length, uint8_t chipsMask, uint8_t numEraseSectorGroups, ParallelFlashEraseSectorGroup const *eraseSectorGroups)
 {
-	bool result = false;
+	// Choose a default sector group if we don't have the info
+	static const ParallelFlashEraseSectorGroup defaultSST39SF040Sectors[] = {
+		{0xFFFFFFFFUL, SECTOR_SIZE_SST39SF040}
+	};
 
-	// Figure out our sector size
-	uint32_t sectorSize;
-	switch (curChipType)
+	static const ParallelFlashEraseSectorGroup defaultM29F160FBSectors[] = {
+		{1, 0x4000},
+		{2, 0x2000},
+		{1, 0x8000},
+		{0xFFFFFFFFUL, SECTOR_SIZE_M29F160FB5AN6E2_8}
+	};
+
+	// If we don't know the sector info (older programmer or unknown chips)
+	// then fall back to the previous hardcoded sector maps.
+	// Note that "chip type" isn't really accurate anymore; this is more about
+	// whether or not it has shifted unlock addresses. But these are the hardcoded
+	// defaults that seemed to work okay for people previously.
+	if (numEraseSectorGroups == 0)
 	{
-	case ParallelFlash_SST39SF040_x4:
-	default:
-		sectorSize = SECTOR_SIZE_SST39SF040;
-		break;
-	case ParallelFlash_M29F160FB5AN6E2_x4:
-		sectorSize = SECTOR_SIZE_M29F160FB5AN6E2_8;
-		break;
+		switch (curChipType)
+		{
+		case ParallelFlash_SST39SF040_x4:
+		default:
+			eraseSectorGroups = defaultSST39SF040Sectors;
+			numEraseSectorGroups = sizeof(defaultSST39SF040Sectors)/sizeof(defaultSST39SF040Sectors[0]);
+			break;
+		case ParallelFlash_M29F160FB5AN6E2_x4:
+			eraseSectorGroups = defaultM29F160FBSectors;
+			numEraseSectorGroups = sizeof(defaultM29F160FBSectors)/sizeof(defaultM29F160FBSectors[0]);
+			break;
+		}
 	}
 
-	// Make sure the area requested to be erased is on good boundaries
-	if ((address % sectorSize) ||
-		(length % sectorSize))
+	bool result = false;
+
+	// The first sector group and index in that group to erase
+	uint32_t firstSectorGroup = 0;
+	uint32_t firstSectorInGroup = 0;
+
+	// Temporary counters for matching up sector locations
+	uint32_t curSectorGroup = 0;
+	uint32_t curSectorInGroup = 0;
+
+	// Find the first sector we need to erase. Keep searching until we've
+	// 1) found it or gone past it, or
+	// 2) exhausted our list of erase sector groups
+	uint32_t curAddress = 0;
+	while (curAddress < address &&
+		   curSectorGroup < numEraseSectorGroups)
+	{
+		curAddress += eraseSectorGroups[curSectorGroup].size;
+		curSectorInGroup++;
+		if (curSectorInGroup >= eraseSectorGroups[curSectorGroup].count)
+		{
+			curSectorGroup++;
+			curSectorInGroup = 0;
+		}
+	}
+
+	// If the start address wasn't on a sector boundary, bail
+	if (curAddress != address)
 	{
 		return false;
 	}
 
+	// OK, we've found our first sector to erase.
+	firstSectorGroup = curSectorGroup;
+	firstSectorInGroup = curSectorInGroup;
+
+	// Now, locate our last sector to erase.
+	uint32_t curLength = 0;
+	while (curLength < length &&
+		   curSectorGroup < numEraseSectorGroups)
+	{
+		curLength += eraseSectorGroups[curSectorGroup].size;
+
+		// If we still haven't handled the entire requested space,
+		// go to the next sector
+		if (curLength < length)
+		{
+			curSectorInGroup++;
+			if (curSectorInGroup >= eraseSectorGroups[curSectorGroup].count)
+			{
+				curSectorGroup++;
+				curSectorInGroup = 0;
+			}
+		}
+	}
+
+	// If the length wasn't on a sector boundary, bail
+	if (curLength != length)
+	{
+		return false;
+	}
+
+	// We've now verified that everything is on a sector boundary, so we can
+	// go ahead with the erase operation!
+	curSectorGroup = firstSectorGroup;
+	curSectorInGroup = firstSectorInGroup;
+
 	// We're good to go. Let's do it. The process varies based on the chip type
 	if (curChipType == ParallelFlash_SST39SF040_x4)
 	{
@@ -183,8 +263,15 @@ bool ParallelFlash_EraseSectors(uint32_t address, uint32_t length, uint8_t chips
 			// unlock sequence has to be done again after this sector is done.
 			ParallelBus_WriteCycle(address, 0x30303030UL);
 
-			address += sectorSize;
-			length -= sectorSize;
+			// Move our counters in preparation for the next sector
+			address += eraseSectorGroups[curSectorGroup].size;
+			length -= eraseSectorGroups[curSectorGroup].size;
+			curSectorInGroup++;
+			if (curSectorInGroup >= eraseSectorGroups[curSectorGroup].count)
+			{
+				curSectorGroup++;
+				curSectorInGroup = 0;
+			}
 
 			// Wait for completion of this individual erase operation before
 			// we can start a new erase operation.
@@ -202,25 +289,19 @@ bool ParallelFlash_EraseSectors(uint32_t address, uint32_t length, uint8_t chips
 		ParallelBus_WriteCycle(ParallelFlash_UnlockAddress1(), 0x80808080UL);
 		ParallelFlash_UnlockChips(chipsMask);
 
-		// Now provide as many sector addresses as needed to erase.
-		// The first address is a bit of a special case because the boot sector
-		// actually has finer granularity for sector sizes.
-		if (address == 0)
-		{
-			ParallelBus_WriteCycle(0x00000000UL, 0x30303030UL);
-			ParallelBus_WriteCycle(0x00004000UL, 0x30303030UL);
-			ParallelBus_WriteCycle(0x00006000UL, 0x30303030UL);
-			ParallelBus_WriteCycle(0x00008000UL, 0x30303030UL);
-			address += sectorSize;
-			length -= sectorSize;
-		}
-
-		// The remaining sectors can use a more generic algorithm
 		while (length)
 		{
 			ParallelBus_WriteCycle(address, 0x30303030UL);
-			address += sectorSize;
-			length -= sectorSize;
+
+			// Move our counters in preparation for the next sector
+			address += eraseSectorGroups[curSectorGroup].size;
+			length -= eraseSectorGroups[curSectorGroup].size;
+			curSectorInGroup++;
+			if (curSectorInGroup >= eraseSectorGroups[curSectorGroup].count)
+			{
+				curSectorGroup++;
+				curSectorInGroup = 0;
+			}
 		}
 
 		// Wait for completion of the entire erase operation
@@ -230,7 +311,6 @@ bool ParallelFlash_EraseSectors(uint32_t address, uint32_t length, uint8_t chips
 	}
 
 	return result;
-
 }
 
 /** Writes a buffer of data to all 4 chips simultaneously

drivers/parallel_flash.h

@@ -54,6 +54,13 @@ typedef enum ParallelFlashChipType
 	ParallelFlash_M29F160FB5AN6E2_x4,
 } ParallelFlashChipType;
 
+/// Struct representing a group of identical erase sectors
+typedef struct ParallelFlashEraseSectorGroup
+{
+	uint32_t count;
+	uint32_t size;
+} ParallelFlashEraseSectorGroup;
+
 // Tells which type of flash chip we are communicating with
 void ParallelFlash_SetChipType(ParallelFlashChipType type);
 ParallelFlashChipType ParallelFlash_ChipType(void);
@@ -69,7 +76,7 @@ void ParallelFlash_IdentifyChips(ParallelFlashChipID *chips);
 
 // Erases the chips/sectors requested
 void ParallelFlash_EraseChips(uint8_t chipsMask);
-bool ParallelFlash_EraseSectors(uint32_t address, uint32_t length, uint8_t chipsMask);
+bool ParallelFlash_EraseSectors(uint32_t address, uint32_t length, uint8_t chipsMask, uint8_t numEraseSectorGroups, ParallelFlashEraseSectorGroup const *eraseSectorGroups);
 
 // Writes a buffer to all 4 chips simultaneously (each uint32_t contains an 8-bit portion for each chip).
 // Optimized variant of this function if we know we're writing to all 4 chips simultaneously.

programmer_protocol.h

@@ -46,7 +46,9 @@ typedef enum ProgrammerCommand
     ErasePortion,
     WriteChipsAt,
     ReadChipsAt,
-    SetChipsMask
+	SetChipsMask,
+	SetSectorLayout,
+	GetFirmwareVersion
 } ProgrammerCommand;
 
 // After a command is sent, the programmer will always respond with
@@ -184,4 +186,15 @@ typedef enum ProgrammerErasePortionOfChipReply
 	ProgrammerErasePortionFinished
 } ProgrammerErasePortionOfChipReply;
 
+// -------------------------  GET FIRMWARE VERSION PROTOCOL  -------------------------
+// If the command is GetFirmwareVersion, the programmer will reply CommandReplyOK.
+// Next, it will return 4 bytes: major version, minor version, revision, and a final
+// byte where 0 means it's a normal version and 1 means it's a prerelease version.
+// Other values are reserved.
+// Finally, it will finish the response with ProgrammerGetFWVersionDone.
+typedef enum ProgrammerGetFWVersionReply
+{
+	ProgrammerGetFWVersionDone
+} ProgrammerGetFWVersionReply;
+
 #endif /* PROGRAMMER_PROTOCOL_H_ */

simm_programmer.c

@@ -29,6 +29,7 @@
 #include "led.h"
 #include "hardware.h"
 #include <stdbool.h>
+#include <string.h>
 
 /// Maximum size of an individual chip on a SIMM we read
 #define MAX_CHIP_SIZE				(2UL * 1024UL * 1024UL)
@@ -38,8 +39,20 @@
 #if ((READ_WRITE_CHUNK_SIZE_BYTES % 4) != 0)
 #error Read/write chunk size should be a multiple of 4 bytes
 #endif
-/// The smallest granularity for sector erase that we support
-#define ERASE_SECTOR_SIZE_BYTES		(256UL * 1024UL)
+
+/// The maximum number of erase groups we deal with
+#define MAX_ERASE_SECTOR_GROUPS				10
+
+/// Version info to respond with
+#define VERSION_MAJOR						1
+#define VERSION_MINOR						5
+#define VERSION_REVISION					0
+
+/// The number of erase sector groups we know about currently.
+/// If it's zero, we don't know, so fall back to defaults.
+static uint8_t numEraseSectorGroups = 0;
+/// The erase sector groups that we will pass to the programmer
+static ParallelFlashEraseSectorGroup eraseSectorGroups[MAX_ERASE_SECTOR_GROUPS];
 
 /// Internal state so we know how to interpret the next-received byte
 typedef enum ProgrammerCommandState
@@ -52,6 +65,7 @@ typedef enum ProgrammerCommandState
 	ReadingChipsReadStartPos,    //!< Reading the start position for reading data from the SIMM
 	WritingChipsReadingStartPos, //!< Reading the start position for writing data to the SIMM
 	ReadingChipsMask,            //!< Reading the bitmask of which chips should be programmed
+	ReadingSectorLayout,         //!< Reading the erase sector layout
 } ProgrammerCommandState;
 static ProgrammerCommandState curCommandState = WaitingForCommand;
 
@@ -84,6 +98,7 @@ static void SIMMProgrammer_HandleErasePortionReadPosLengthByte(uint8_t byte);
 static void SIMMProgrammer_HandleReadingChipsReadStartPosByte(uint8_t byte);
 static void SIMMProgrammer_HandleWritingChipsReadingStartPosByte(uint8_t byte);
 static void SIMMProgrammer_HandleReadingChipsMaskByte(uint8_t byte);
+static void SIMMProgrammer_HandleReadingSectorLayoutByte(uint8_t byte);
 
 /** Initializes the SIMM programmer and prepares it for USB communication.
  *
@@ -132,6 +147,9 @@ void SIMMProgrammer_Check(void)
 		case ReadingChipsMask:
 			SIMMProgrammer_HandleReadingChipsMaskByte(recvByte);
 			break;
+		case ReadingSectorLayout:
+			SIMMProgrammer_HandleReadingSectorLayoutByte(recvByte);
+			break;
 		}
 	}
 
@@ -264,6 +282,18 @@ static void SIMMProgrammer_HandleWaitingForCommandByte(uint8_t byte)
 		curCommandState = ReadingChipsMask;
 		USBCDC_SendByte(CommandReplyOK);
 		break;
+	case SetSectorLayout:
+		curCommandState = ReadingSectorLayout;
+		USBCDC_SendByte(CommandReplyOK);
+		break;
+	case GetFirmwareVersion:
+		USBCDC_SendByte(CommandReplyOK);
+		USBCDC_SendByte(VERSION_MAJOR);
+		USBCDC_SendByte(VERSION_MINOR);
+		USBCDC_SendByte(VERSION_REVISION);
+		USBCDC_SendByte(0);
+		USBCDC_SendByte(ProgrammerGetFWVersionDone);
+		break;
 	// We don't know what this command is, so reply that it was invalid.
 	default:
 		USBCDC_SendByte(CommandReplyInvalid);
@@ -523,44 +553,28 @@ static void SIMMProgrammer_HandleErasePortionReadPosLengthByte(uint8_t byte)
 
 	if (++readLengthByteIndex >= 8)
 	{
-		ParallelFlashChipType chipType = ParallelFlash_ChipType();
 		bool eraseSuccess = false;
 
-		// Ensure they are both within limits of sector size erasure
-		if (((erasePosition % ERASE_SECTOR_SIZE_BYTES) == 0) &&
-			((eraseLength % ERASE_SECTOR_SIZE_BYTES) == 0))
+		// Ensure the position and length are a multiple of 4 so that the division by 4
+		// won't confuse anything.
+		if (((erasePosition % 4) == 0) &&
+			((eraseLength % 4) == 0))
 		{
 			uint32_t boundary = eraseLength + erasePosition;
 
-			// Ensure they are within the limits of the chip size too
-			if (chipType == ParallelFlash_SST39SF040_x4)
+			// Ensure they are within the limits of our addressable length too.
+			// We can't address more than 8 MB of data at a time.
+			if (boundary <= (8 * 1024UL * 1024UL))
 			{
-				if (boundary <= (8 * 1024UL * 1024UL))
+				// OK! We're erasing certain sectors of a SIMM.
+				USBCDC_SendByte(ProgrammerErasePortionOK);
+				// Send the response immediately, it could take a while.
+				USBCDC_Flush();
+				if (ParallelFlash_EraseSectors(erasePosition/PARALLEL_FLASH_NUM_CHIPS,
+						eraseLength/PARALLEL_FLASH_NUM_CHIPS, chipsMask,
+						numEraseSectorGroups, eraseSectorGroups))
 				{
-					// OK! We're erasing certain sectors of a SIMM.
-					USBCDC_SendByte(ProgrammerErasePortionOK);
-					// Send the response immediately, it could take a while.
-					USBCDC_Flush();
-					if (ParallelFlash_EraseSectors(erasePosition/PARALLEL_FLASH_NUM_CHIPS,
-							eraseLength/PARALLEL_FLASH_NUM_CHIPS, chipsMask))
-					{
-						eraseSuccess = true;
-					}
-				}
-			}
-			else if (chipType == ParallelFlash_M29F160FB5AN6E2_x4)
-			{
-				if (boundary <= (8 * 1024UL * 1024UL))
-				{
-					// OK! We're erasing certain sectors of a SIMM.
-					USBCDC_SendByte(ProgrammerErasePortionOK);
-					// Send the response immediately, it could take a while.
-					USBCDC_Flush();
-					if (ParallelFlash_EraseSectors(erasePosition/PARALLEL_FLASH_NUM_CHIPS,
-							eraseLength/PARALLEL_FLASH_NUM_CHIPS, chipsMask))
-					{
-						eraseSuccess = true;
-					}
+					eraseSuccess = true;
 				}
 			}
 		}
@@ -646,3 +660,57 @@ static void SIMMProgrammer_HandleReadingChipsMaskByte(uint8_t byte)
 	// Done either way; now we're waiting for a command to arrive
 	curCommandState = WaitingForCommand;
 }
+
+/** Handles a received byte when we are reading in the sector layout
+ *
+ * @param byte The received byte, which is the first sector layout byte
+ */
+static void SIMMProgrammer_HandleReadingSectorLayoutByte(uint8_t byte)
+{
+	numEraseSectorGroups = 0;
+
+	uint32_t sectorCount = byte;
+	uint32_t sectorSize = 0;
+	int byteIndex = 1;
+
+	while (1)
+	{
+		// Read in the sector size
+		for (int i = byteIndex; i < 4; i++)
+		{
+			uint32_t nextByte = (uint32_t)USBCDC_ReadByteBlocking();
+			sectorCount |= nextByte << (i * 8);
+		}
+		// From now on, we loop over 4 bytes, not 3
+		byteIndex = 0;
+
+		// If we read in a count of 0, we're done!
+		if (sectorCount == 0)
+		{
+			break;
+		}
+
+		// We have a nonzero count, so read in the size now
+		for (int i = 0; i < 4; i++)
+		{
+			uint32_t nextByte = (uint32_t)USBCDC_ReadByteBlocking();
+			sectorSize |= nextByte << (i * 8);
+		}
+
+		// If we have room to store it in the array, do it
+		if (numEraseSectorGroups < MAX_ERASE_SECTOR_GROUPS)
+		{
+			eraseSectorGroups[numEraseSectorGroups].count = sectorCount;
+			eraseSectorGroups[numEraseSectorGroups].size = sectorSize;
+			numEraseSectorGroups++;
+		}
+
+		// Now read in the next chunk of data
+		sectorCount = 0;
+		sectorSize = 0;
+	}
+
+	// We got the list. Done!
+	USBCDC_SendByte(CommandReplyOK);
+	curCommandState = WaitingForCommand;
+}