Fix: SFDP conformance checks

src/actions.cxx

@@ -188,7 +188,7 @@ namespace bmpflash
 			return false;
 
 		// Ask for the SFDP data and display it then clean up
-		sfdp::readAndDisplay(*probe);
+		sfdp::readAndDisplay(*probe, sfdpArguments["display-raw"sv] != nullptr);
 		return probe->end();
 	}
 

src/include/options.hxx

@@ -59,6 +59,19 @@ namespace bmpflash
 		)
 	};
 
+	constexpr static auto sfdpOptions
+	{
+		options
+		(
+			deviceOptions,
+			option_t
+			{
+				optionFlagPair_t{"-r"sv, "--display-raw"sv},
+				"Dispaly the raw SFDP data read from the device as it's read"sv
+			}
+		)
+	};
+
 	constexpr static auto provisioningOptions{options(serialOption, fileOption)};
 	constexpr static auto generalFlashOptions{options(deviceOptions, fileOption)};
 
@@ -74,7 +87,7 @@ namespace bmpflash
 			{
 				"sfdp"sv,
 				"Display the SFDP (Serial Flash Discoverable Parameters) information for a Flash chip"sv,
-				deviceOptions,
+				sfdpOptions,
 			},
 			{
 				"provision"sv,

src/include/sfdp.hxx

@@ -10,7 +10,7 @@ namespace bmpflash::sfdp
 {
 	using bmpflash::spiFlash::spiFlash_t;
 
-	bool readAndDisplay(const bmp_t &probe);
+	bool readAndDisplay(const bmp_t &probe, bool displayRaw);
 	std::optional<spiFlash_t> read(const bmp_t &probe);
 } // namespace bmpflash::sfdp
 

src/include/sfdpInternal.hxx

@@ -50,6 +50,10 @@
 
 	struct parameterTableHeader_t
 	{
+	private:
+		[[nodiscard]] size_t lengthForVersion() const noexcept;
+
+	public:
 		uint8_t jedecParameterIDLow{};
 		uint8_t versionMinor{};
 		uint8_t versionMajor{};
@@ -59,7 +63,53 @@
 
 		[[nodiscard]] uint16_t jedecParameterID() const noexcept
 			{ return static_cast<uint16_t>((jedecParameterIDHigh << 8U) | jedecParameterIDLow); }
-		[[nodiscard]] size_t tableLength() const noexcept { return static_cast<size_t>(tableLengthInU32s) * 4U; }
+		[[nodiscard]] size_t tableLength() const noexcept { return static_cast<size_t>(tableLengthInU32s * 4U); }
+		void validate() noexcept;
+	};
+
+	struct writeAndEraseGranularity_t
+	{
+	private:
+		uint8_t data{};
+
+	public:
+		[[nodiscard]] std::optional<uint8_t> volatileStatusWriteEnable() const noexcept
+		{
+			// Check if the status register requires any write enables
+			if (!(data & 0x08U))
+				return std::nullopt;
+			// Otherwise check if the device requires 0x06 as write enable
+			if (data & 0x10U)
+				return 0x06U;
+			// If not, then it's 0x50 as write enable
+			return 0x50U;
+		}
+
+		[[nodiscard]] bool supports4KiBErase() const noexcept
+			{ return (data & 0x03U) == 0x01U; }
+	};
+
+	struct fastReadAndAddressing_t
+	{
+	private:
+		uint8_t data{};
+
+	public:
+		[[nodiscard]] bool supportsFastRead_1_1_2() const noexcept { return data & 0x01U; }
+		[[nodiscard]] bool supportsFastRead_1_1_4() const noexcept { return data & 0x40U; }
+		[[nodiscard]] bool supportsFastRead_1_2_2() const noexcept { return data & 0x10U; }
+		[[nodiscard]] bool supportsFastRead_1_4_4() const noexcept { return data & 0x20U; }
+
+		[[nodiscard]] uint8_t addressBytes() const
+		{
+			const auto bytes{static_cast<uint8_t>(data & 0x06U)};
+			if (bytes == 0x00U)
+				return 3U;
+			if (bytes == 0x04U)
+				return 4U;
+			// 0x02U is 3-or-4-bytes mode but we can't necessarily determine which the chip is in
+			throw std::range_error{"Address bytes value invalid or indeterminate"};
+		}
 	};
 
 	struct memoryDensity_t
@@ -109,10 +159,15 @@
 		uint8_t programmingTimingRatioAndPageSize{};
 		std::array<uint8_t, 3> eraseTimings;
 
-		[[nodiscard]] uint64_t pageSize() const noexcept
+		[[nodiscard]] uint32_t pageSize() const noexcept
 		{
+			// Extract the exponent, which by definition must be a value between 0 and 15
 			const auto pageSizeExponent{static_cast<uint8_t>(programmingTimingRatioAndPageSize >> 4U)};
-			return UINT64_C(1) << pageSizeExponent;
+			// If for some reason this is 0, return the default page size (256) instead
+			if (!pageSizeExponent)
+				return 256U;
+			// Exponentiate to get the real page size (0-64KiB)
+			return 1U << pageSizeExponent;
 		}
 	};
 
@@ -135,9 +190,9 @@
 
 	struct basicParameterTable_t
 	{
-		uint8_t value1{};
+		writeAndEraseGranularity_t writeAndEraseGranularity{};
 		uint8_t sectorEraseOpcode{};
-		uint8_t value2{};
+		fastReadAndAddressing_t fastReadAndAddressing{};
 		uint8_t reserved1{};
 		memoryDensity_t flashMemoryDensity{};
 		timingsAndOpcode_t fastQuadIO{};

src/include/spiFlash.hxx

@@ -81,6 +81,10 @@
 		pageRead = command(opcodeMode_t::with3BAddress, dataMode_t::dataIn, 0U, opcode_t::pageRead),
 	};
 
+	// NB: This technically invokes UB, however there's not really a better way to do this, so.
+	constexpr inline command_t operator |(const command_t &cmd, const uint8_t &opcode) noexcept
+		{ return static_cast<command_t>(uint16_t(cmd) | opcode); }
+
 	constexpr inline uint8_t spiStatusBusy{1};
 	constexpr inline uint8_t spiStatusWriteEnabled{2};
 

src/sfdp.cxx

@@ -3,12 +3,18 @@
 // SPDX-FileContributor: Written by Rachel Mant <[email protected]>
 #include <cstddef>
 #include <cstdint>
+#include <cinttypes>
 #include <string_view>
+#include <array>
 #include <tuple>
+#include <optional>
 #include <substrate/console>
 #include <substrate/index_sequence>
 #include <substrate/indexed_iterator>
 #include <substrate/units>
+#include <substrate/span>
+#include "bmp.hxx"
+#include "spiFlash.hxx"
 #include "sfdp.hxx"
 #include "sfdpInternal.hxx"
 #include "units.hxx"
@@ -30,11 +36,38 @@
 	constexpr static uint16_t basicSPIParameterTable{0xFF00U};
 
 	[[nodiscard]] bool sfdpRead(const bmp_t &probe, const uint32_t address, void *const data,
-			const size_t dataLength)
-		{ return probe.read(spiFlashCommand_t::readSFDP, address, data, dataLength); }
+			const size_t dataLength, const bool displayRaw)
+	{
+		const auto result{probe.read(spiFlashCommand_t::readSFDP, address, data, dataLength)};
+		// Check if we should display the raw data
+		if (displayRaw)
+		{
+			console.info("Raw data for read:"sv);
+			// Turn the buffer into a span so we can iterate through the raw bytes
+			substrate::span sfdpData{reinterpret_cast<const uint8_t *>(data), dataLength};
+			// Write the data out in blocks of 8 bytes per line
+			for (const auto &[index, value] : indexedIterator_t{sfdpData})
+			{
+				// If we should start a new line, output a start of line marker
+				if ((index & 7U) == 0)
+				{
+					// Output a new line to terminate the previous
+					if (index)
+						console.writeln();
+					// The trailing nullptr suppresses the automatic new line
+					console.info(asHex_t<6, '0'>{address + index}, ": "sv, nullptr);
+				}
+				console.writeln(asHex_t<2, '0'>{value}, ' ', nullptr);
+			}
+			// Complete the display by completing the last line with a new line
+			console.writeln();
+		}
+		return result;
+	}
 
-	template<typename T> [[nodiscard]] bool sfdpRead(const bmp_t &probe, const uintptr_t address, T &buffer)
-			{ return sfdpRead(probe, static_cast<uint32_t>(address), &buffer, sizeof(T)); }
+	template<typename T> [[nodiscard]] bool sfdpRead(const bmp_t &probe, const uintptr_t address, T &buffer,
+		const bool displayRaw)
+			{ return sfdpRead(probe, static_cast<uint32_t>(address), &buffer, sizeof(T), displayRaw); }
 
 	void displayHeader(const sfdpHeader_t &header)
 	{
@@ -54,11 +87,12 @@
 		console.info("-> table SFDP address: "sv, uint32_t{header.tableAddress});
 	}
 
-	[[nodiscard]] bool displayBasicParameterTable(const bmp_t &probe, const parameterTableHeader_t &header)
+	[[nodiscard]] bool displayBasicParameterTable(const bmp_t &probe, const parameterTableHeader_t &header,
+		const bool displayRaw)
 	{
 		basicParameterTable_t parameterTable{};
 		if (!sfdpRead(probe, header.tableAddress, &parameterTable,
-			std::min(sizeof(basicParameterTable_t), header.tableLength())))
+			std::min(sizeof(basicParameterTable_t), header.tableLength()), displayRaw))
 			return false;
 
 		console.info("Basic parameter table:");
@@ -88,11 +122,11 @@
 		return true;
 	}
 
-	bool readAndDisplay(const bmp_t &probe)
+	bool readAndDisplay(const bmp_t &probe, const bool displayRaw)
 	{
 		console.info("Reading SFDP data for device"sv);
 		sfdpHeader_t header{};
-		if (!sfdpRead(probe, sfdpHeaderAddress, header))
+		if (!sfdpRead(probe, sfdpHeaderAddress, header, displayRaw))
 			return false;
 		if (header.magic != sfdpMagic)
 		{
@@ -106,12 +140,13 @@
 		for (const auto idx : indexSequence_t{header.parameterHeadersCount()})
 		{
 			parameterTableHeader_t tableHeader{};
-			if (!sfdpRead(probe, tableHeaderAddress + (sizeof(parameterTableHeader_t) * idx), tableHeader))
+			if (!sfdpRead(probe, tableHeaderAddress + (sizeof(parameterTableHeader_t) * idx), tableHeader, displayRaw))
 				return false;
 			displayTableHeader(tableHeader, idx + 1U);
 			if (tableHeader.jedecParameterID() == basicSPIParameterTable)
 			{
-				if (!displayBasicParameterTable(probe, tableHeader))
+				tableHeader.validate();
+				if (!displayBasicParameterTable(probe, tableHeader, displayRaw))
 					return false;
 			}
 		}
@@ -136,7 +171,7 @@
 	{
 		basicParameterTable_t parameterTable{};
 		if (!sfdpRead(probe, header.tableAddress, &parameterTable,
-			std::min(sizeof(basicParameterTable_t), header.tableLength())))
+			std::min(sizeof(basicParameterTable_t), header.tableLength()), false))
 			return {};
 
 		const auto [sectorSize, sectorEraseOpcode]
@@ -158,8 +193,7 @@
 			{
 				if (header.versionMajor > 1U || (header.versionMajor == 1U && header.versionMinor >= 5U))
 					return parameterTable.programmingAndChipEraseTiming.pageSize();
-				else
-					return 256U;
+				return 256U;
 			}()
 		};
 		const auto capacity{parameterTable.flashMemoryDensity.capacity()};
@@ -170,7 +204,7 @@
 	{
 		console.info("Reading SFDP data for device"sv);
 		sfdpHeader_t header{};
-		if (!sfdpRead(probe, sfdpHeaderAddress, header))
+		if (!sfdpRead(probe, sfdpHeaderAddress, header, false))
 			return std::nullopt;
 		if (header.magic != sfdpMagic)
 		{
@@ -181,12 +215,102 @@
 		for (const auto idx : indexSequence_t{header.parameterHeadersCount()})
 		{
 			parameterTableHeader_t tableHeader{};
-			if (!sfdpRead(probe, tableHeaderAddress + (sizeof(parameterTableHeader_t) * idx), tableHeader))
+			if (!sfdpRead(probe, tableHeaderAddress + (sizeof(parameterTableHeader_t) * idx), tableHeader, false))
 				return std::nullopt;
 
 			if (tableHeader.jedecParameterID() == basicSPIParameterTable)
+			{
+				tableHeader.validate();
 				return readBasicParameterTable(probe, tableHeader);
+			}
 		}
 		return std::nullopt;
 	}
+
+	size_t parameterTableHeader_t::lengthForVersion() const noexcept
+	{
+		if (versionMajor < 1U)
+		{
+			console.warn("SFDP basic parameters table header version incorrect, got v"sv, versionMajor, '.',
+				versionMinor, " which is less than minimum allowable version of v1.0"sv);
+			// If the version number is impossible, just return the table length - there's nothing else we can do.
+			return tableLength();
+		}
+		// Turn the version number into a uint16_t with the upper byte being the major and the lower being the minor
+		const auto version{static_cast<uint16_t>((versionMajor << 8U) | versionMinor)};
+		// Now switch on the valid ones we know about
+		switch (version)
+		{
+			// v1.0 through v1.4 from the original JESD216
+			case 0x0100U:
+			case 0x0101U:
+			case 0x0102U:
+			case 0x0103U:
+			case 0x0104U:
+				return 36U; // 9 uint32_t's
+			// v1.5 (JESD216A), v1.6 (JESD216B)
+			case 0x0105U:
+			case 0x0106U:
+				return 64U; // 16 uint32_t's
+			// v1.7 (JESD216C, JESD216D, JESD216E)
+			case 0x0107U:
+				return 84U; // 21 uint32_t's
+			// v1.8 (JESD216F)
+			case 0x0108U:
+				return 96U; // 24 uint32_t's
+			default:
+				console.warn("Unknown SFDP version v"sv, versionMajor, '.', versionMinor, ", assuming valid size"sv);
+				return tableLength();
+		}
+	}
+
+	void parameterTableHeader_t::validate() noexcept
+	{
+		const auto expectedLength{lengthForVersion()};
+		const auto actualLength{tableLength()};
+		// If the table is the proper length for the version, we're done
+		if (actualLength == expectedLength)
+			return;
+
+		console.warn("Found mismatched basic parameters table length, got "sv, actualLength, ", expected "sv,
+			expectedLength);
+		// If the table is longer than it should be for the stated version, truncate it
+		if (actualLength > expectedLength)
+		{
+			console.warn("Adjusting table length to correct"sv);
+			tableLengthInU32s = static_cast<uint8_t>(expectedLength / 4U);
+		}
+		// Otherwise fix the version number to match the one for the actual length
+		else
+		{
+			console.warn("Adjusting table version number to correct"sv);
+			// 24 uint32_t's -> v1.8
+			if (actualLength == 96U)
+			{
+				versionMajor = 1U;
+				versionMinor = 8U;
+			}
+			// 21 uint32_t's -> v1.7
+			else if (actualLength == 84U)
+			{
+				versionMajor = 1U;
+				versionMinor = 7U;
+			}
+			// 16 uint32_t's -> v1.6 (assume the newer standard)
+			else if (actualLength == 64U)
+			{
+				versionMajor = 1U;
+				versionMinor = 6U;
+			}
+			// 9 uint32_t's -> v1.4 (assume the newer standard)
+			else if (actualLength == 36U)
+			{
+				versionMajor = 1U;
+				versionMinor = 4U;
+			}
+			else
+				console.error("This should not be possible, please check sfdp.cxx for sanity"sv);
+			console.info("Adjusted version is "sv, versionMajor, '.', versionMinor);
+		}
+	}
 } // namespace bmpflash::sfdp