Code cleanup and additional comments

Signed-off-by: Herbert Poetzl <herbert@13thfloor.at>

lib/CMakeLists.txt

@@ -45,17 +45,17 @@ if (PRJXRAY_BUILD_TESTING)
 		xilinx/xc7series/bitstream_reader_test.cc
 		xilinx/xc7series/bitstream_writer_test.cc
 		xilinx/xc7series/block_type_test.cc
-		xilinx/xc7series/frame_address_test.cc
 		xilinx/xc7series/configuration_bus_test.cc
 		xilinx/xc7series/configuration_column_test.cc
 		xilinx/xc7series/configuration_test.cc
 		xilinx/xc7series/configuration_packet_test.cc
 		xilinx/xc7series/configuration_packetizer_test.cc
+		xilinx/xc7series/crc_test.cc
+		xilinx/xc7series/ecc_test.cc
+		xilinx/xc7series/frame_address_test.cc
 		xilinx/xc7series/global_clock_region_test.cc
 		xilinx/xc7series/part_test.cc
-		xilinx/xc7series/row_test.cc
+		xilinx/xc7series/row_test.cc)
-		xilinx/xc7series/crc_test.cc
-		xilinx/xc7series/ecc_test.cc)
 	target_link_libraries(xilinx_xc7series_test libprjxray gtest_main)
 	add_test(NAME xilinx_xc7series_test
 		 COMMAND xilinx_xc7series_test

lib/include/prjxray/xilinx/xc7series/crc.h

@@ -3,19 +3,25 @@
 
 #include <cstdint>
 
+constexpr uint32_t kCrc32CastagnoliPolynomial = 0x82F63B78;
+
 namespace prjxray {
 namespace xilinx {
 namespace xc7series {
 
-// Extend the current CRC value with one address (5bit) and data (32bit) 
+// The CRC is calculated from each written data word and the current
-// pair and return the newly computed CRC value
+// register address the data is written to.
+
+// Extend the current CRC value with one register address (5bit) and
+// frame data (32bit) pair and return the newly computed CRC value.
 
 uint32_t icap_crc(uint32_t addr, uint32_t data, uint32_t prev) {
-	uint64_t val = ((uint64_t)addr << 32) | data;
+	uint64_t poly = static_cast<uint64_t>(kCrc32CastagnoliPolynomial) << 1;
+	uint64_t val = (static_cast<uint64_t>(addr) << 32) | data;
 	uint64_t crc = prev;
-	uint64_t poly = 0x82F63B78L << 1;	// CRC-32C (Castagnoli)
+	constexpr int kFivePlusThrityTwo = 37;
 
-	for (int i = 0; i < 37; i++) {
+	for (int i = 0; i < kFivePlusThrityTwo; i++) {
 		if ((val & 1) != (crc & 1))
 			crc ^= poly;
 

lib/include/prjxray/xilinx/xc7series/ecc.h

@@ -8,36 +8,35 @@ namespace xilinx {
 namespace xc7series {
 
 // Extend the current ECC code with one data word (32 bit) at a given
-// index in the configuration packet and return the new ECC code.
+// word index in the configuration frame and return the new ECC code.
 
 uint32_t icap_ecc(uint32_t idx, uint32_t data, uint32_t ecc) {
-	uint32_t off = idx * 32;
+	uint32_t val = idx * 32;  // bit offset
 
-	if (idx > 0x25)			// avoid 0x800
+	if (idx > 0x25)  // avoid 0x800
-		off += 0x1360;
+		val += 0x1360;
-	else if (idx > 0x6)		// avoid 0x400
+	else if (idx > 0x6)  // avoid 0x400
-		off += 0x1340;
+		val += 0x1340;
-	else				// avoid lower
+	else  // avoid lower
-		off += 0x1320;
+		val += 0x1320;
 
-
+	if (idx == 0x32)  // mask ECC
-	if (idx == 0x32)		// mask ECC
 		data &= 0xFFFFE000;
 
 	for (int i = 0; i < 32; i++) {
 		if (data & 1)
-			ecc ^= off + i;
+			ecc ^= val + i;
 
 		data >>= 1;
 	}
 
-	if (idx == 0x64) {		// last index
+	if (idx == 0x64) {  // last index
 		uint32_t v = ecc & 0xFFF;
 		v ^= v >> 8;
 		v ^= v >> 4;
 		v ^= v >> 2;
 		v ^= v >> 1;
-		ecc ^= (v & 1) << 12;	// parity
+		ecc ^= (v & 1) << 12;  // parity
 	}
 
 	return ecc;

lib/xilinx/xc7series/crc_test.cc

@@ -5,9 +5,12 @@
 namespace xc7series = prjxray::xilinx::xc7series;
 
 TEST(IcapCrcTest, SimpleTests) {
-	EXPECT_EQ(xc7series::icap_crc(0, 0, 0), (uint32_t)0x0);
+	// CRC for Zero Data
-	EXPECT_EQ(xc7series::icap_crc(~0, ~0, 0), 0xBF86D4DF);
+	EXPECT_EQ(xc7series::icap_crc(0, 0, 0), 0x0L);
-	EXPECT_EQ(xc7series::icap_crc(0, 0, ~0), 0xC631E365);
+	// Polynomial (single bit operation)
 	EXPECT_EQ(xc7series::icap_crc(1 << 4, 0, 0), 0x82F63B78);
+	// All Reg/Data bits
+	EXPECT_EQ(xc7series::icap_crc(~0, ~0, 0), 0xBF86D4DF);
+	// All CRC bits
+	EXPECT_EQ(xc7series::icap_crc(0, 0, ~0), 0xC631E365);
 }
-

lib/xilinx/xc7series/ecc_test.cc

@@ -5,11 +5,16 @@
 namespace xc7series = prjxray::xilinx::xc7series;
 
 TEST(IcapEccTest, SimpleTests) {
+	// ECC for Zero Data
 	EXPECT_EQ(xc7series::icap_ecc(0, 0, 0), (uint32_t)0x0);
+	// 0x1320 - 0x13FF (avoid lower)
 	EXPECT_EQ(xc7series::icap_ecc(0, 1, 0), (uint32_t)0x1320);
+	// 0x1420 - 0x17FF (avoid 0x400)
 	EXPECT_EQ(xc7series::icap_ecc(0x7, 1, 0), (uint32_t)0x1420);
+	// 0x1820 - 0x1FFF (avoid 0x800)
 	EXPECT_EQ(xc7series::icap_ecc(0x26, 1, 0), (uint32_t)0x1820);
+	// Masked ECC Value
 	EXPECT_EQ(xc7series::icap_ecc(0x32, ~0, 0), (uint32_t)0x000019AC);
+	// Final ECC Parity
 	EXPECT_EQ(xc7series::icap_ecc(0x64, 0, 1), (uint32_t)0x00001001);
 }
-