Merge pull request #12524 from VeijoPesonen/sfdp_split_4

Bugfix: Concurrent SFDP header address init fixed

Author: adbridge

components/storage/blockdevice/COMPONENT_QSPIF/QSPIFBlockDevice.cpp

@@ -179,6 +179,8 @@ QSPIFBlockDevice::QSPIFBlockDevice(PinName io0, PinName io1, PinName io2, PinNam
 
 int QSPIFBlockDevice::init()
 {
+    int status = QSPIF_BD_ERROR_OK;
+
     if (_unique_device_status == 0) {
         tr_debug("QSPIFBlockDevice csel: %d", (int)_csel);
     } else if (_unique_device_status == -1) {
@@ -189,12 +191,6 @@ int QSPIFBlockDevice::init()
         return QSPIF_BD_ERROR_DEVICE_MAX_EXCEED;
     }
 
-    int status = QSPIF_BD_ERROR_OK;
-    _sfdp_info.bptbl.addr = 0x0;
-    _sfdp_info.bptbl.size = 0;
-    _sfdp_info.smptbl.addr = 0x0;
-    _sfdp_info.smptbl.size = 0;
-
     _mutex.lock();
 
     // All commands other than Read and RSFDP use default 1-1-1 bus mode (Program/Erase are constrained by flash memory performance more than bus performance)
@@ -230,36 +226,33 @@ int QSPIFBlockDevice::init()
         goto exit_point;
     }
 
-    if (0 != _handle_vendor_quirks()) {
+    if (_handle_vendor_quirks() < 0) {
         tr_error("Init - Could not read vendor id");
         status = QSPIF_BD_ERROR_DEVICE_ERROR;
         goto exit_point;
     }
 
-    /**************************** Parse SFDP Header ***********************************/
-    if (sfdp_parse_headers(callback(this, &QSPIFBlockDevice::_qspi_send_read_sfdp_command), _sfdp_info) < 0) {
-        tr_error("Init - Parse SFDP Headers Failed");
-        status = QSPIF_BD_ERROR_PARSING_FAILED;
-        goto exit_point;
-    }
+    /**************************** Parse SFDP data ***********************************/
+    {
+        _sfdp_info.bptbl.addr = 0x0;
+        _sfdp_info.bptbl.size = 0;
+        _sfdp_info.smptbl.addr = 0x0;
+        _sfdp_info.smptbl.size = 0;
 
-    /**************************** Parse Basic Parameters Table ***********************************/
-    if (_sfdp_parse_basic_param_table(callback(this, &QSPIFBlockDevice::_qspi_send_read_sfdp_command),
-                                      _sfdp_info) < 0) {
-        tr_error("Init - Parse Basic Param Table Failed");
-        status = QSPIF_BD_ERROR_PARSING_FAILED;
-        goto exit_point;
-    }
+        if (sfdp_parse_headers(callback(this, &QSPIFBlockDevice::_qspi_send_read_sfdp_command), _sfdp_info) < 0) {
+            tr_error("Init - Parse SFDP Headers Failed");
+            status = QSPIF_BD_ERROR_PARSING_FAILED;
+            goto exit_point;
+        }
 
-    /**************************** Parse Sector Map Table ***********************************/
-    _sfdp_info.smptbl.region_size[0] = _sfdp_info.bptbl.device_size_bytes; // If there's no region map, we have a single region sized the entire device size
-    _sfdp_info.smptbl.region_high_boundary[0] = _sfdp_info.bptbl.device_size_bytes - 1;
+        if (_sfdp_parse_basic_param_table(callback(this, &QSPIFBlockDevice::_qspi_send_read_sfdp_command),
+                                          _sfdp_info) < 0) {
+            tr_error("Init - Parse Basic Param Table Failed");
+            status = QSPIF_BD_ERROR_PARSING_FAILED;
+            goto exit_point;
+        }
 
-    if ((_sfdp_info.smptbl.addr != 0) && (0 != _sfdp_info.smptbl.size)) {
-        tr_debug("Init - Parsing Sector Map Table - addr: 0x%lxh, Size: %d", _sfdp_info.smptbl.addr,
-                 _sfdp_info.smptbl.size);
-        if (sfdp_parse_sector_map_table(callback(this, &QSPIFBlockDevice::_qspi_send_read_sfdp_command),
-                                        _sfdp_info.smptbl) < 0) {
+        if (sfdp_parse_sector_map_table(callback(this, &QSPIFBlockDevice::_qspi_send_read_sfdp_command), _sfdp_info) < 0) {
             tr_error("Init - Parse Sector Map Table Failed");
             status = QSPIF_BD_ERROR_PARSING_FAILED;
             goto exit_point;
@@ -630,17 +623,15 @@ int QSPIFBlockDevice::_sfdp_parse_basic_param_table(Callback<int(bd_addr_t, void
     }
 
     // Check that density is not greater than 4 gigabits (i.e. that addressing beyond 4 bytes is not required)
-    if ((param_table[7] & 0x80) != 0) {
-        tr_error("Init - verify flash density failed");
+    if (sfdp_detect_addressability(param_table, _sfdp_info.bptbl) < 0) {
+        tr_error("Verify 4byte addressing failed");
         return -1;
     }
 
-    // Get device density (stored in bits - 1)
-    uint32_t density_bits = ((param_table[7] << 24) |
-                             (param_table[6] << 16) |
-                             (param_table[5] << 8)  |
-                             param_table[4]);
-    sfdp_info.bptbl.device_size_bytes = (density_bits + 1) / 8;
+    if (sfdp_detect_device_density(param_table, _sfdp_info.bptbl) < 0) {
+        tr_error("Detecting device density failed");
+        return -1;
+    }
 
     // Set Page Size (QSPI write must be done on Page limits)
     _page_size_bytes = sfdp_detect_page_size(param_table, sfdp_info.bptbl.size);

components/storage/blockdevice/COMPONENT_QSPIF/QSPIFBlockDevice.h

@@ -306,6 +306,9 @@ class QSPIFBlockDevice : public mbed::BlockDevice {
     // Enable Fast Mode - for flash chips with low power default
     int _enable_fast_mode();
 
+    // Query vendor ID and handle special behavior that isn't covered by SFDP data
+    int _handle_vendor_quirks();
+
     /****************************************/
     /* SFDP Detection and Parsing Functions */
     /****************************************/
@@ -329,23 +332,6 @@ class QSPIFBlockDevice : public mbed::BlockDevice {
     // Detect 4-byte addressing mode and enable it if supported
     int _sfdp_detect_and_enable_4byte_addressing(uint8_t *basic_param_table_ptr, int basic_param_table_size);
 
-    // Query vendor ID and handle special behavior that isn't covered by SFDP data
-    int _handle_vendor_quirks();
-
-    /***********************/
-    /* Utilities Functions */
-    /***********************/
-    // Find the region to which the given offset belong to
-    int _utils_find_addr_region(mbed::bd_size_t offset, mbed::sfdp_smptbl_info &smptbl);
-
-    // Iterate on all supported Erase Types of the Region to which the offset belong to.
-    // Iterates from highest type to lowest
-    int _utils_iterate_next_largest_erase_type(uint8_t &bitfield,
-                                               int size,
-                                               int offset,
-                                               int region,
-                                               mbed::sfdp_smptbl_info &smptbl);
-
 private:
     enum qspif_clear_protection_method_t {
         QSPIF_BP_ULBPR,    // Issue global protection unlock instruction

components/storage/blockdevice/COMPONENT_SPIF/SPIFBlockDevice.cpp

@@ -115,15 +115,7 @@ SPIFBlockDevice::SPIFBlockDevice(PinName mosi, PinName miso, PinName sclk, PinNa
 
 int SPIFBlockDevice::init()
 {
-    uint8_t vendor_device_ids[4];
-    size_t data_length = 3;
     int status = SPIF_BD_ERROR_OK;
-    spif_bd_error spi_status = SPIF_BD_ERROR_OK;
-
-    _sfdp_info.bptbl.addr = 0x0;
-    _sfdp_info.bptbl.size = 0;
-    _sfdp_info.smptbl.addr = 0x0;
-    _sfdp_info.smptbl.size = 0;
 
     _mutex->lock();
 
@@ -146,56 +138,39 @@ int SPIFBlockDevice::init()
         tr_debug("Initialize flash memory OK");
     }
 
-    /* Read Manufacturer ID (1byte), and Device ID (2bytes)*/
-    spi_status = _spi_send_general_command(SPIF_RDID, SPI_NO_ADDRESS_COMMAND, NULL, 0, (char *)vendor_device_ids,
-                                           data_length);
-    if (spi_status != SPIF_BD_ERROR_OK) {
-        tr_error("init - Read Vendor ID Failed");
+    if (_handle_vendor_quirks() < 0) {
+        tr_error("Init - Could not read vendor id");
         status = SPIF_BD_ERROR_DEVICE_ERROR;
         goto exit_point;
     }
 
-    switch (vendor_device_ids[0]) {
-        case 0xbf:
-            // SST devices come preset with block protection
-            // enabled for some regions, issue global protection unlock to clear
-            _set_write_enable();
-            _spi_send_general_command(SPIF_ULBPR, SPI_NO_ADDRESS_COMMAND, NULL, 0, NULL, 0);
-            break;
-    }
-
     //Synchronize Device
     if (false == _is_mem_ready()) {
         tr_error("init - _is_mem_ready Failed");
         status = SPIF_BD_ERROR_READY_FAILED;
         goto exit_point;
     }
 
-    /**************************** Parse SFDP Header ***********************************/
-    if (sfdp_parse_headers(callback(this, &SPIFBlockDevice::_spi_send_read_sfdp_command), _sfdp_info) < 0) {
-        tr_error("init - Parse SFDP Headers Failed");
-        status = SPIF_BD_ERROR_PARSING_FAILED;
-        goto exit_point;
-    }
-
+    /**************************** Parse SFDP headers and tables ***********************************/
+    {
+        _sfdp_info.bptbl.addr = 0x0;
+        _sfdp_info.bptbl.size = 0;
+        _sfdp_info.smptbl.addr = 0x0;
+        _sfdp_info.smptbl.size = 0;
 
-    /**************************** Parse Basic Parameters Table ***********************************/
-    if (_sfdp_parse_basic_param_table(callback(this, &SPIFBlockDevice::_spi_send_read_sfdp_command), _sfdp_info) < 0) {
-        tr_error("init - Parse Basic Param Table Failed");
-        status = SPIF_BD_ERROR_PARSING_FAILED;
-        goto exit_point;
-    }
+        if (sfdp_parse_headers(callback(this, &SPIFBlockDevice::_spi_send_read_sfdp_command), _sfdp_info) < 0) {
+            tr_error("init - Parse SFDP Headers Failed");
+            status = SPIF_BD_ERROR_PARSING_FAILED;
+            goto exit_point;
+        }
 
-    /**************************** Parse Sector Map Table ***********************************/
-    _sfdp_info.smptbl.region_size[0] = _sfdp_info.bptbl.device_size_bytes;
-    // If there's no region map, we have a single region sized the entire device size
-    _sfdp_info.smptbl.region_high_boundary[0] = _sfdp_info.bptbl.device_size_bytes - 1;
+        if (_sfdp_parse_basic_param_table(callback(this, &SPIFBlockDevice::_spi_send_read_sfdp_command), _sfdp_info) < 0) {
+            tr_error("init - Parse Basic Param Table Failed");
+            status = SPIF_BD_ERROR_PARSING_FAILED;
+            goto exit_point;
+        }
 
-    if ((_sfdp_info.smptbl.addr != 0) && (0 != _sfdp_info.smptbl.size)) {
-        tr_debug("init - Parsing Sector Map Table - addr: 0x%" PRIx32 "h, Size: %d", _sfdp_info.smptbl.addr,
-                 _sfdp_info.smptbl.size);
-        if (sfdp_parse_sector_map_table(callback(this, &SPIFBlockDevice::_spi_send_read_sfdp_command),
-                                        _sfdp_info.smptbl) < 0) {
+        if (sfdp_parse_sector_map_table(callback(this, &SPIFBlockDevice::_spi_send_read_sfdp_command), _sfdp_info) < 0) {
             tr_error("init - Parse Sector Map Table Failed");
             status = SPIF_BD_ERROR_PARSING_FAILED;
             goto exit_point;
@@ -629,19 +604,15 @@ int SPIFBlockDevice::_sfdp_parse_basic_param_table(Callback<int(bd_addr_t, void
     }
 
     // Check address size, currently only supports 3byte addresses
-    if ((param_table[2] & 0x4) != 0 || (param_table[7] & 0x80) != 0) {
-        tr_error("init - verify 3byte addressing Failed");
+    if (sfdp_detect_addressability(param_table, _sfdp_info.bptbl) < 0) {
+        tr_error("Verify 3byte addressing failed");
         return -1;
     }
 
-    // Get device density (stored in bits - 1)
-    uint32_t density_bits = (
-                                (param_table[7] << 24) |
-                                (param_table[6] << 16) |
-                                (param_table[5] << 8) |
-                                param_table[4]);
-    sfdp_info.bptbl.device_size_bytes = (density_bits + 1) / 8;
-    tr_debug("Density bits: %" PRIu32 " , device size: %llu bytes", density_bits, sfdp_info.bptbl.device_size_bytes);
+    if (sfdp_detect_device_density(param_table, _sfdp_info.bptbl) < 0) {
+        tr_error("Detecting device density failed");
+        return -1;
+    }
 
     // Set Default read/program/erase Instructions
     _read_instruction = SPIF_READ;
@@ -778,3 +749,32 @@ int SPIFBlockDevice::_set_write_enable()
     } while (false);
     return status;
 }
+
+int SPIFBlockDevice::_handle_vendor_quirks()
+{
+    uint8_t vendor_device_ids[4];
+    size_t data_length = 3;
+
+    /* Read Manufacturer ID (1byte), and Device ID (2bytes)*/
+    spif_bd_error spi_status = _spi_send_general_command(SPIF_RDID, SPI_NO_ADDRESS_COMMAND, NULL, 0,
+                                                         (char *)vendor_device_ids,
+                                                         data_length);
+
+    if (spi_status != SPIF_BD_ERROR_OK) {
+        tr_error("Read Vendor ID Failed");
+        return -1;
+    }
+
+    tr_debug("Vendor device ID = 0x%x 0x%x 0x%x", vendor_device_ids[0], vendor_device_ids[1], vendor_device_ids[2]);
+
+    switch (vendor_device_ids[0]) {
+        case 0xbf:
+            // SST devices come preset with block protection
+            // enabled for some regions, issue global protection unlock to clear
+            _set_write_enable();
+            _spi_send_general_command(SPIF_ULBPR, SPI_NO_ADDRESS_COMMAND, NULL, 0, NULL, 0);
+            break;
+    }
+
+    return 0;
+}

components/storage/blockdevice/COMPONENT_SPIF/SPIFBlockDevice.h

@@ -260,6 +260,9 @@ class SPIFBlockDevice : public mbed::BlockDevice {
     // Wait on status register until write not-in-progress
     bool _is_mem_ready();
 
+    // Query vendor ID and handle special behavior that isn't covered by SFDP data
+    int _handle_vendor_quirks();
+
 private:
     // Master side hardware
     mbed::SPI _spi;

drivers/internal/SFDP.h

@@ -90,11 +90,11 @@ int sfdp_parse_headers(Callback<int(bd_addr_t, void *, bd_size_t)> sfdp_reader,
  * Retrieves the table from a device and parses the information contained by the table
  *
  * @param      sfdp_reader Callback function used to read headers from within a device
- * @param[out] smtbl       Contains the results of parsing the JEDEC Sector Map Table
+ * @param[out] sfdp_info   Contains the results of parsing the JEDEC Sector Map Table
  *
  * @return MBED_SUCCESS on success, negative error code on failure
  */
-int sfdp_parse_sector_map_table(Callback<int(bd_addr_t, void *, bd_size_t)> sfdp_reader, sfdp_smptbl_info &smtbl);
+int sfdp_parse_sector_map_table(Callback<int(bd_addr_t, void *, bd_size_t)> sfdp_reader, sfdp_hdr_info &sfdp_info);
 
 /** Detect page size used for writing on flash
  *
@@ -131,7 +131,7 @@ int sfdp_find_addr_region(bd_size_t offset, const sfdp_hdr_info &sfdp_info);
  * @param size     Upper limit for region size
  * @param offset   Offset value
  * @param region   Region number
- * @param smtbl    Information about different erase types
+ * @param smptbl    Information about different erase types
  *
  * @return Largest erase type
  */
@@ -141,6 +141,24 @@ int sfdp_iterate_next_largest_erase_type(uint8_t &bitfield,
                                          int region,
                                          const sfdp_smptbl_info &smptbl);
 
+/** Detect device density
+ *
+ * @param bptbl_ptr Pointer to memory holding a Basic Parameter Table structure
+ * @param bptbl_info Basic Parameter Table information structure
+ *
+ * @return 0 on success, negative error code on failure
+ */
+int sfdp_detect_device_density(uint8_t *bptbl_ptr, sfdp_bptbl_info &bptbl_info);
+
+/** Detect is it possible to access the whole memory region
+ *
+ * @param bptbl_ptr Pointer to memory holding a Basic Parameter Table structure
+ * @param bptbl_info Basic Parameter Table information structure
+ *
+ * @return 0 on success, negative error code on failure
+ */
+int sfdp_detect_addressability(uint8_t *bptbl_ptr, sfdp_bptbl_info &bptbl_info);
+
 /** @}*/
 } /* namespace mbed */
 #endif