[nrfconnect] Introduced several improvements for the factory data man…

…agement (#33195) * [nrfconnect] Added FactoryDataBase to facilitate dynamic assignments of FactoryDataProvider concrete types to to the generic pointer. Signed-off-by: Marcin Kajor <[email protected]> * [nrfconnect] Added non-PM based factory data support Added support for platforms not built using PM. This commit includes fixes only firmware implementation and does not include cmake changes for factory data generation. Signed-off-by: Kamil Kasperczyk <[email protected]> * [nrfconnect] Improved generation of the factory data Allow to merge the factory data .hex file with the firmware's .hex file when partition manager is not available. * [nrfconnect] Disable External FD if QSPI NOR is not available We need to have support for factory data when the partition manager is enabled but there is no support for QSPI external flash. --------- Signed-off-by: Marcin Kajor <[email protected]> Signed-off-by: Kamil Kasperczyk <[email protected]> Co-authored-by: Marcin Kajor <[email protected]> Co-authored-by: Arkadiusz Balys <[email protected]>
project-chip · May 6, 2024 · be19fc9 · be19fc9
1 parent 062e063
commit be19fc9
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 47 deletions.
diff --git a/config/nrfconnect/chip-module/Kconfig b/config/nrfconnect/chip-module/Kconfig
@@ -106,7 +106,6 @@ config CHIP_MALLOC_SYS_HEAP
 config CHIP_FACTORY_DATA
 	bool "Factory data provider"
 	select ZCBOR
-	imply FPROTECT
 	help
 	  Enables the default nRF Connect factory data provider implementation that
 	  supports reading the factory data encoded in the CBOR format from the
@@ -149,7 +148,7 @@ config CHIP_FACTORY_DATA_WRITE_PROTECT
 	bool "Enable Factory Data write protection"
 	select FPROTECT
 	depends on CHIP_FACTORY_DATA
-	default y if CHIP_FACTORY_DATA
+	default y
 	help
 		Enables the write protection of the Factory Data partition in the flash memory.
 		This is a recommended feature, but it requires the Settings partition size to be

diff --git a/config/nrfconnect/chip-module/generate_factory_data.cmake b/config/nrfconnect/chip-module/generate_factory_data.cmake
@@ -129,9 +129,21 @@ set(factory_data_output_path ${output_path}/${factory_data_target})
 string(APPEND script_args "-o \"${factory_data_output_path}\"\n")
 string(APPEND script_args "-s \"${schema_path}\"\n")
 
-# Add optional offset and size arguments to generate both .hex and .json files.
-string(APPEND script_args "--offset $<TARGET_PROPERTY:partition_manager,PM_FACTORY_DATA_ADDRESS>\n")
-string(APPEND script_args "--size $<TARGET_PROPERTY:partition_manager,PM_FACTORY_DATA_OFFSET>\n")
+# Add optional offset and size arguments to generate .hex file as well as .json.
+if(CONFIG_PARTITION_MANAGER_ENABLED)     
+    string(APPEND script_args "--offset $<TARGET_PROPERTY:partition_manager,PM_FACTORY_DATA_ADDRESS>\n")
+    string(APPEND script_args "--size $<TARGET_PROPERTY:partition_manager,PM_FACTORY_DATA_OFFSET>\n")
+else()
+    dt_alias(factory_data_alias PROPERTY "factory-data")
+    dt_node_exists(factory_data_exists PATH "${factory_data_alias}")
+    if(NOT ${factory_data_exists})
+        message(FATAL_ERROR "factory-data alias does not exist in DTS")
+    endif()
+    dt_reg_addr(factory_data_addr PATH ${factory_data_alias})
+    dt_reg_size(factory_data_size PATH ${factory_data_alias})
+    string(APPEND script_args "--offset ${factory_data_addr}\n")
+    string(APPEND script_args "--size ${factory_data_size}\n")     
+endif()
 
 # execute first script to create a JSON file
 separate_arguments(separated_script_args NATIVE_COMMAND ${script_args})
@@ -175,10 +187,15 @@ nrfconnect_create_factory_data(factory_data
                                ${FACTORY_DATA_SCHEMA_PATH} 
                                ${OUTPUT_FILE_PATH})
 
-if(CONFIG_CHIP_FACTORY_DATA_MERGE_WITH_FIRMWARE)                                   
-    # set custom target for merging factory_data hex file
-    set_property(GLOBAL PROPERTY factory_data_PM_HEX_FILE ${OUTPUT_FILE_PATH}/factory_data.hex)
-    set_property(GLOBAL PROPERTY factory_data_PM_TARGET factory_data)
+if(CONFIG_CHIP_FACTORY_DATA_MERGE_WITH_FIRMWARE)
+    if(CONFIG_PARTITION_MANAGER_ENABLED)                           
+        # set custom target for merging factory_data hex file
+        set_property(GLOBAL PROPERTY factory_data_PM_HEX_FILE ${OUTPUT_FILE_PATH}/factory_data.hex)
+        set_property(GLOBAL PROPERTY factory_data_PM_TARGET factory_data)
+    else()
+        set_property(GLOBAL APPEND PROPERTY HEX_FILES_TO_MERGE ${OUTPUT_FILE_PATH}/factory_data.hex ${OUTPUT_FILE_PATH}/zephyr.hex)
+        set_property(TARGET runners_yaml_props_target PROPERTY hex_file ${OUTPUT_FILE_PATH}/merged.hex)
+    endif()
 endif()
 
 

diff --git a/src/platform/nrfconnect/FactoryDataProvider.cpp b/src/platform/nrfconnect/FactoryDataProvider.cpp
@@ -432,7 +432,10 @@ CHIP_ERROR FactoryDataProvider<FlashFactoryData>::GetUserKey(const char * userKe
 
 // Fully instantiate the template class in whatever compilation unit includes this file.
 template class FactoryDataProvider<InternalFlashFactoryData>;
+#if defined(USE_PARTITION_MANAGER) && USE_PARTITION_MANAGER == 1 && (defined(CONFIG_CHIP_QSPI_NOR) || defined(CONFIG_CHIP_SPI_NOR))
 template class FactoryDataProvider<ExternalFlashFactoryData>;
+#endif // if defined(USE_PARTITION_MANAGER) && USE_PARTITION_MANAGER == 1 (defined(CONFIG_CHIP_QSPI_NOR) ||
+       // defined(CONFIG_CHIP_SPI_NOR))
 
 } // namespace DeviceLayer
 } // namespace chip
diff --git a/src/platform/nrfconnect/FactoryDataProvider.h b/src/platform/nrfconnect/FactoryDataProvider.h
@@ -25,8 +25,20 @@
 #include <crypto/CHIPCryptoPALPSA.h>
 #endif
 
+#ifdef CONFIG_FPROTECT
 #include <fprotect.h>
+#endif // if CONFIG_FPROTECT
+
+#if defined(USE_PARTITION_MANAGER) && USE_PARTITION_MANAGER == 1
 #include <pm_config.h>
+#define FACTORY_DATA_ADDRESS PM_FACTORY_DATA_ADDRESS
+#define FACTORY_DATA_SIZE PM_FACTORY_DATA_SIZE
+#else
+#include <zephyr/storage/flash_map.h>
+#define FACTORY_DATA_SIZE DT_REG_SIZE(DT_ALIAS(factory_data))
+#define FACTORY_DATA_ADDRESS DT_REG_ADDR(DT_ALIAS(factory_data))
+#endif // if defined(USE_PARTITION_MANAGER) && USE_PARTITION_MANAGER == 1
+
 #include <system/SystemError.h>
 #include <zephyr/drivers/flash.h>
 
@@ -39,8 +51,8 @@ struct InternalFlashFactoryData
 {
     CHIP_ERROR GetFactoryDataPartition(uint8_t *& data, size_t & dataSize)
     {
-        data     = reinterpret_cast<uint8_t *>(PM_FACTORY_DATA_ADDRESS);
-        dataSize = PM_FACTORY_DATA_SIZE;
+        data     = reinterpret_cast<uint8_t *>(FACTORY_DATA_ADDRESS);
+        dataSize = FACTORY_DATA_SIZE;
         return CHIP_NO_ERROR;
     }
 
@@ -54,16 +66,16 @@ struct InternalFlashFactoryData
     // the application code at runtime anyway.
     constexpr size_t FactoryDataBlockBegin()
     {
-        // calculate the nearest multiple of CONFIG_FPROTECT_BLOCK_SIZE smaller than PM_FACTORY_DATA_ADDRESS
-        return PM_FACTORY_DATA_ADDRESS & (-CONFIG_FPROTECT_BLOCK_SIZE);
+        // calculate the nearest multiple of CONFIG_FPROTECT_BLOCK_SIZE smaller than FACTORY_DATA_ADDRESS
+        return FACTORY_DATA_ADDRESS & (-CONFIG_FPROTECT_BLOCK_SIZE);
     }
 
     constexpr size_t FactoryDataBlockSize()
     {
         // calculate the factory data end address rounded up to the nearest multiple of CONFIG_FPROTECT_BLOCK_SIZE
         // and make sure we do not overlap with settings partition
         constexpr size_t kFactoryDataBlockEnd =
-            (PM_FACTORY_DATA_ADDRESS + PM_FACTORY_DATA_SIZE + CONFIG_FPROTECT_BLOCK_SIZE - 1) & (-CONFIG_FPROTECT_BLOCK_SIZE);
+            (FACTORY_DATA_ADDRESS + FACTORY_DATA_SIZE + CONFIG_FPROTECT_BLOCK_SIZE - 1) & (-CONFIG_FPROTECT_BLOCK_SIZE);
         static_assert(kFactoryDataBlockEnd <= PM_SETTINGS_STORAGE_ADDRESS,
                       "FPROTECT memory block, which contains factory data"
                       "partition overlaps with the settings partition."
@@ -75,44 +87,94 @@ struct InternalFlashFactoryData
 #undef TO_STR_IMPL
     CHIP_ERROR ProtectFactoryDataPartitionAgainstWrite()
     {
+#ifdef CONFIG_FPROTECT
         int ret = fprotect_area(FactoryDataBlockBegin(), FactoryDataBlockSize());
         return System::MapErrorZephyr(ret);
+#else
+        return CHIP_ERROR_NOT_IMPLEMENTED;
+#endif // if CONFIG_FPROTECT
     }
 #else
     CHIP_ERROR ProtectFactoryDataPartitionAgainstWrite() { return CHIP_ERROR_NOT_IMPLEMENTED; }
 #endif
 };
 
+#if defined(USE_PARTITION_MANAGER) && USE_PARTITION_MANAGER == 1 && (defined(CONFIG_CHIP_QSPI_NOR) || defined(CONFIG_CHIP_SPI_NOR))
 struct ExternalFlashFactoryData
 {
     CHIP_ERROR GetFactoryDataPartition(uint8_t *& data, size_t & dataSize)
     {
-        int ret = flash_read(mFlashDevice, PM_FACTORY_DATA_ADDRESS, mFactoryDataBuffer, PM_FACTORY_DATA_SIZE);
+        int ret = flash_read(mFlashDevice, FACTORY_DATA_ADDRESS, mFactoryDataBuffer, FACTORY_DATA_SIZE);
 
         if (ret != 0)
         {
             return CHIP_ERROR_READ_FAILED;
         }
 
         data     = mFactoryDataBuffer;
-        dataSize = PM_FACTORY_DATA_SIZE;
+        dataSize = FACTORY_DATA_SIZE;
 
         return CHIP_NO_ERROR;
     }
 
     CHIP_ERROR ProtectFactoryDataPartitionAgainstWrite() { return CHIP_ERROR_NOT_IMPLEMENTED; }
 
-    const struct device * mFlashDevice = DEVICE_DT_GET(DT_CHOSEN(zephyr_flash_controller));
-    uint8_t mFactoryDataBuffer[PM_FACTORY_DATA_SIZE];
+    const struct device * mFlashDevice = DEVICE_DT_GET(DT_CHOSEN(nordic_pm_ext_flash));
+    uint8_t mFactoryDataBuffer[FACTORY_DATA_SIZE];
+};
+#endif // if defined(USE_PARTITION_MANAGER) && USE_PARTITION_MANAGER == 1 && (defined(CONFIG_CHIP_QSPI_NOR) ||
+       // defined(CONFIG_CHIP_SPI_NOR))
+
+class FactoryDataProviderBase : public chip::Credentials::DeviceAttestationCredentialsProvider,
+                                public CommissionableDataProvider,
+                                public DeviceInstanceInfoProvider
+{
+public:
+    /**
+     * @brief Perform all operations needed to initialize factory data provider.
+     *
+     * @returns CHIP_NO_ERROR in case of a success, specific error code otherwise
+     */
+    virtual CHIP_ERROR Init() = 0;
+
+    /**
+     * @brief Get the EnableKey as MutableByteSpan
+     *
+     * @param enableKey MutableByteSpan object to obtain EnableKey
+     * @returns
+     * CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if factory data does not contain enable_key field, or the value cannot be read
+     * out. CHIP_ERROR_BUFFER_TOO_SMALL if provided MutableByteSpan is too small
+     */
+    virtual CHIP_ERROR GetEnableKey(MutableByteSpan & enableKey) = 0;
+
+    /**
+     * @brief Get the user data in CBOR format as MutableByteSpan
+     *
+     * @param userData MutableByteSpan object to obtain all user data in CBOR format
+     * @returns
+     * CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if factory data does not contain user field, or the value cannot be read out.
+     * CHIP_ERROR_BUFFER_TOO_SMALL if provided MutableByteSpan is too small
+     */
+    virtual CHIP_ERROR GetUserData(MutableByteSpan & userData) = 0;
+
+    /**
+     * @brief Try to find user data key and return its value
+     *
+     * @param userKey A key name to be found
+     * @param buf Buffer to store value of found key
+     * @param len Length of the buffer. This value will be updated to the actual value if the key is read.
+     * @returns
+     * CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if factory data does not contain user key field, or the value cannot be read
+     * out. CHIP_ERROR_BUFFER_TOO_SMALL if provided buffer length is too small
+     */
+    virtual CHIP_ERROR GetUserKey(const char * userKey, void * buf, size_t & len) = 0;
 };
 
 template <class FlashFactoryData>
-class FactoryDataProvider : public chip::Credentials::DeviceAttestationCredentialsProvider,
-                            public CommissionableDataProvider,
-                            public DeviceInstanceInfoProvider
+class FactoryDataProvider : public FactoryDataProviderBase
 {
 public:
-    CHIP_ERROR Init();
+    CHIP_ERROR Init() override;
 
     // ===== Members functions that implement the DeviceAttestationCredentialsProvider
     CHIP_ERROR GetCertificationDeclaration(MutableByteSpan & outBuffer) override;
@@ -147,33 +209,13 @@ class FactoryDataProvider : public chip::Credentials::DeviceAttestationCredentia
     CHIP_ERROR GetProductPrimaryColor(app::Clusters::BasicInformation::ColorEnum * primaryColor) override;
 
     // ===== Members functions that are platform-specific
-    CHIP_ERROR GetEnableKey(MutableByteSpan & enableKey);
-
-    /**
-     * @brief Get the user data in CBOR format as MutableByteSpan
-     *
-     * @param userData MutableByteSpan object to obtain all user data in CBOR format
-     * @returns
-     * CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if factory data does not contain user field, or the value cannot be read out.
-     * CHIP_ERROR_BUFFER_TOO_SMALL if provided MutableByteSpan is too small
-     */
-    CHIP_ERROR GetUserData(MutableByteSpan & userData);
-
-    /**
-     * @brief Try to find user data key and return its value
-     *
-     * @param userKey A key name to be found
-     * @param buf Buffer to store value of found key
-     * @param len Length of the buffer. This value will be updated to the actual value if the key is read.
-     * @returns
-     * CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if factory data does not contain user field, or the value cannot be read out.
-     * CHIP_ERROR_BUFFER_TOO_SMALL if provided buffer length is too small
-     */
-    CHIP_ERROR GetUserKey(const char * userKey, void * buf, size_t & len);
+    CHIP_ERROR GetEnableKey(MutableByteSpan & enableKey) override;
+    CHIP_ERROR GetUserData(MutableByteSpan & userData) override;
+    CHIP_ERROR GetUserKey(const char * userKey, void * buf, size_t & len) override;
 
 private:
-    static constexpr uint16_t kFactoryDataPartitionSize    = PM_FACTORY_DATA_SIZE;
-    static constexpr uint32_t kFactoryDataPartitionAddress = PM_FACTORY_DATA_ADDRESS;
+    static constexpr uint16_t kFactoryDataPartitionSize    = FACTORY_DATA_SIZE;
+    static constexpr uint32_t kFactoryDataPartitionAddress = FACTORY_DATA_ADDRESS;
     static constexpr uint8_t kDACPrivateKeyLength          = 32;
     static constexpr uint8_t kDACPublicKeyLength           = 65;