Created read and write cycle functions, along with a block read function. I think this will look better...

I also changed the port module so it doesn't needlessly update the data direction register over SPI if it's being told to set the same value it had before.

external_mem.c

@@ -105,8 +105,13 @@ void ExternalMem_DeassertOE(void)
 
 void ExternalMem_Read(uint32_t startAddress, uint32_t *buf, uint32_t len)
 {
+	// This is just a time saver if we know we will
+	// be reading a complete block -- doesn't bother
+	// playing with the control lines between each byte
+	ExternalMem_DeassertWE();
 	ExternalMem_AssertCS();
 	ExternalMem_AssertOE();
+	ExternalMem_SetDataAsInput();
 
 	while (len--)
 	{
@@ -128,22 +133,26 @@ void ExternalMem_Read(uint32_t startAddress, uint32_t *buf, uint32_t len)
 
 void ExternalMem_WriteCycle(uint32_t address, uint32_t data)
 {
+	ExternalMem_AssertCS();
+	ExternalMem_DeassertOE();
 	ExternalMem_SetAddressAndData(address, data);
 	ExternalMem_AssertWE();
-
 	_delay_us(1); // Give it a small amount of time needed? Could I do this with some NOP instructions instead of waiting 1us?
 	ExternalMem_DeassertWE();
 }
 
+uint32_t ExternalMem_ReadCycle(uint32_t address)
+{
+	ExternalMem_DeassertWE();
+	ExternalMem_AssertCS();
+	ExternalMem_AssertOE();
+	ExternalMem_SetDataAsInput();
+	ExternalMem_SetAddress(address);
+	return ExternalMem_ReadData();
+}
+
 void ExternalMem_UnlockAllChips(void)
 {
-	// Disable the chips completely and wait for a short time...
-	ExternalMem_DeassertCS();
-	ExternalMem_DeassertOE();
-	ExternalMem_DeassertWE();
-	_delay_us(1);
-	ExternalMem_AssertCS();
-
 	// First part of unlock sequence:
 	// Write 0x55555555 to the address bus and 0xAA to the data bus
 	// (Some datasheets may only say 0x555 or 0x5555, but they ignore
@@ -163,19 +172,14 @@ void ExternalMem_IdentifyChips(struct ChipID *chips)
 	ExternalMem_WriteCycle(0x55555555UL, 0x90909090UL);
 
 	// Now we can read the vendor and product ID
-	ExternalMem_SetAddress(0);
-	ExternalMem_SetDataAsInput();
-	ExternalMem_AssertOE();
-
-	uint32_t result = ExternalMem_ReadData();
+	uint32_t result = ExternalMem_ReadCycle(0);
 
 	chips[3].manufacturerID = (uint8_t)result;
 	chips[2].manufacturerID = (uint8_t)(result >> 8);
 	chips[1].manufacturerID = (uint8_t)(result >> 16);
 	chips[0].manufacturerID = (uint8_t)(result >> 24);
 
-	ExternalMem_SetAddress(1);
-	result = ExternalMem_ReadData();
+	result = ExternalMem_ReadCycle(1);
 
 	chips[3].deviceID = (uint8_t)result;
 	chips[2].deviceID = (uint8_t)(result >> 8);
@@ -183,6 +187,5 @@ void ExternalMem_IdentifyChips(struct ChipID *chips)
 	chips[0].deviceID = (uint8_t)(result >> 24);
 
 	// Exit software ID mode
-	ExternalMem_DeassertOE();
 	ExternalMem_WriteCycle(0, 0xF0F0F0F0UL);
 }

external_mem.h

@@ -51,9 +51,12 @@ void ExternalMem_DeassertOE(void);
 // Reads a set of data...
 void ExternalMem_Read(uint32_t startAddress, uint32_t *buf, uint32_t len);
 
-// Performs a write cycle
+// Performs a single write cycle
 void ExternalMem_WriteCycle(uint32_t address, uint32_t data);
 
+// Performs a single read cycle
+uint32_t ExternalMem_ReadCycle(uint32_t address);
+
 // Does an unlock sequence on the chips
 void ExternalMem_UnlockAllChips(void);
 

ports.c

@@ -14,10 +14,16 @@
 #define SIMM_OE		(1 << 5)
 #define SIMM_CS		(1 << 4)
 
+// Save some time by not changing the register
+// unless the value has changed [SPI = relatively slow]
+static uint32_t savedDataDDR = 0;
+
 void Ports_Init(void)
 {
 	// This module depends on the MPC23S17
 	MCP23S17_Init();
+	savedDataDDR = 0xFFFFFFFFUL;
+	Ports_SetDataDDR(0);
 }
 
 void Ports_SetAddressOut(uint32_t data)
@@ -164,13 +170,19 @@ void Ports_AddressDDR_RMW(uint32_t ddr, uint32_t modifyMask)
 
 void Ports_SetDataDDR(uint32_t ddr)
 {
-	MCP23S17_SetDDR(ddr & 0xFFFF); // D0-D15
-	DDRE = ((ddr >> 16) & 0xFF); // D16-D23
-	DDRF = ((ddr >> 24) & 0xFF); // D24-D31
+	if (savedDataDDR != ddr)
+	{
+		MCP23S17_SetDDR(ddr & 0xFFFF); // D0-D15
+		DDRE = ((ddr >> 16) & 0xFF); // D16-D23
+		DDRF = ((ddr >> 24) & 0xFF); // D24-D31
+
+		savedDataDDR = ddr;
+	}
 }
 
 void Ports_DataDDR_RMW(uint32_t ddr, uint32_t modifyMask)
 {
+	uint32_t newSavedDataDDR;
 	uint32_t modifiedDataOn = ddr & modifyMask;
 	uint32_t modifiedDataOff = ddr | ~modifyMask;
 
@@ -178,6 +190,9 @@ void Ports_DataDDR_RMW(uint32_t ddr, uint32_t modifyMask)
 	if ((modifyMask & 0xFFFF) == 0xFFFF)
 	{
 		MCP23S17_SetDDR(modifiedDataOn & 0xFFFF);
+
+		// Remember what the new DDR will be
+		newSavedDataDDR = modifiedDataOn & 0xFFFF;
 	}
 	else // Otherwise, we have to read what's in it first...(unless I decide to keep a local cached copy)
 	{
@@ -185,6 +200,9 @@ void Ports_DataDDR_RMW(uint32_t ddr, uint32_t modifyMask)
 		outputLatches |= (modifiedDataOn) & 0xFFFF;
 		outputLatches &= modifiedDataOff & 0xFFFF;
 		MCP23S17_SetDDR(outputLatches);
+
+		// Remember what the new DDR will be
+		newSavedDataDDR = outputLatches;
 	}
 
 	// Turn on/off requested bits in the DDR register.
@@ -192,6 +210,13 @@ void Ports_DataDDR_RMW(uint32_t ddr, uint32_t modifyMask)
 	DDRE &= ((modifiedDataOff >> 16) & 0xFF);
 	DDRF |= ((modifiedDataOn >> 24) & 0xFF);
 	DDRF &= ((modifiedDataOff >> 24) & 0xFF);
+
+	// Remember what the new DDR will be
+	newSavedDataDDR |= ((uint32_t)DDRE) << 16;
+	newSavedDataDDR |= ((uint32_t)DDRF) << 24;
+
+	// Save the new DDR
+	savedDataDDR = newSavedDataDDR;
 }
 
 void Ports_SetCSDDR(bool ddr)

usb_serial/usb_serial.c

@@ -84,17 +84,21 @@ void USBSerial_Check(void)
 			struct ChipID chips[4];
 			ExternalMem_IdentifyChips(chips);
 			char tmp[20];
+			uint32_t data = ExternalMem_ReadCycle(0);
 
 			int x;
 			for (x = 0; x < 4; x++)
 			{
-				sprintf(tmp, "IC%d: M%02X, D%02X\r\n", x, chips[x].manufacturerID, chips[x].deviceID);
+				sprintf(tmp, "IC%d: M%02X, D%02X\r\n", x+1, chips[x].manufacturerID, chips[x].deviceID);
 				CDC_Device_SendString(&VirtualSerial_CDC_Interface, tmp);
 			}
+
+			sprintf(tmp, "%08lX\r\n", data);
+			CDC_Device_SendString(&VirtualSerial_CDC_Interface, tmp);
 		}
 	}*/
 
-	if (USB_DeviceState == DEVICE_STATE_Configured)
+	/*if (USB_DeviceState == DEVICE_STATE_Configured)
 	{
 		// Check for commands, etc...
 		int16_t recvByte = CDC_Device_ReceiveByte(&VirtualSerial_CDC_Interface);
@@ -108,7 +112,7 @@ void USBSerial_Check(void)
 				break;
 			}
 		}
-	}
+	}*/
 
 	CDC_Device_USBTask(&VirtualSerial_CDC_Interface);
 	USB_USBTask();