[nrfconnect] Fixed DFU over SMP by external flash handling refactor (#18796)

* [nrfconnect] Fixed DFU over SMP by external flash handling refactor

Currently DFU over SMP doesn't work properly due to problems
with external flash interaction that is put asleep by the OTA
Matter module.

* Added using BLE connect/disconnect callbacks to control
the QSPI NOR power mode.
* Refactored ExternalFlashHandler to be generic FlashHandler
that can be also implemented by other backends than NOR QSPI

* Fixed missing includes.

Author: kkasperczyk-no

examples/platform/nrfconnect/util/DFUOverSMP.cpp

@@ -30,6 +30,8 @@
 
 #include <lib/support/logging/CHIPLogging.h>
 
+#include "OTAUtil.h"
+
 using namespace ::chip::DeviceLayer;
 
 constexpr uint16_t kAdvertisingIntervalMinMs = 400;
@@ -44,10 +46,14 @@ void DFUOverSMP::Init(DFUOverSMPRestartAdvertisingHandler startAdvertisingCb)
     img_mgmt_set_upload_cb(UploadConfirmHandler, NULL);
 
     memset(&mBleConnCallbacks, 0, sizeof(mBleConnCallbacks));
+    mBleConnCallbacks.connected    = OnBleConnect;
     mBleConnCallbacks.disconnected = OnBleDisconnect;
 
     bt_conn_cb_register(&mBleConnCallbacks);
 
+    k_work_init(&mFlashSleepWork, [](k_work *) { GetFlashHandler().DoAction(FlashHandler::Action::SLEEP); });
+    k_work_init(&mFlashWakeUpWork, [](k_work *) { GetFlashHandler().DoAction(FlashHandler::Action::WAKE_UP); });
+
     restartAdvertisingCallback = startAdvertisingCb;
 
     PlatformMgr().AddEventHandler(ChipEventHandler, 0);
@@ -130,8 +136,21 @@ void DFUOverSMP::StartBLEAdvertising()
     }
 }
 
+void DFUOverSMP::OnBleConnect(bt_conn * conn, uint8_t err)
+{
+    if (GetDFUOverSMP().IsEnabled())
+    {
+        (void) k_work_submit(&sDFUOverSMP.mFlashWakeUpWork);
+    }
+}
+
 void DFUOverSMP::OnBleDisconnect(struct bt_conn * conId, uint8_t reason)
 {
+    if (GetDFUOverSMP().IsEnabled())
+    {
+        (void) k_work_submit(&sDFUOverSMP.mFlashSleepWork);
+    }
+
     PlatformMgr().LockChipStack();
 
     // After BLE disconnect SMP advertising needs to be restarted. Before making it ensure that BLE disconnect was not triggered
@@ -159,7 +178,7 @@ void DFUOverSMP::ChipEventHandler(const ChipDeviceEvent * event, intptr_t /* arg
                 sDFUOverSMP.restartAdvertisingCallback();
         }
         break;
-    case DeviceEventType::kCommissioningComplete:
+    case DeviceEventType::kCHIPoBLEConnectionClosed:
         // Check if after closing CHIPoBLE connection advertising is working, if no start SMP advertising.
         if (!ConnectivityMgr().IsBLEAdvertisingEnabled())
         {

examples/platform/nrfconnect/util/OTAUtil.cpp

@@ -31,15 +31,19 @@ DefaultOTARequestorStorage sOTARequestorStorage;
 DefaultOTARequestorDriver sOTARequestorDriver;
 chip::BDXDownloader sBDXDownloader;
 chip::DefaultOTARequestor sOTARequestor;
-
 } // namespace
 
+FlashHandler & GetFlashHandler()
+{
+    static FlashHandler sFlashHandler;
+    return sFlashHandler;
+}
+
 // compile-time factory method
 OTAImageProcessorImpl & GetOTAImageProcessor()
 {
 #if CONFIG_PM_DEVICE && CONFIG_NORDIC_QSPI_NOR
-    static ExtFlashHandler sQSPIHandler;
-    static OTAImageProcessorImplPMDevice sOTAImageProcessor{ sQSPIHandler };
+    static OTAImageProcessorImpl sOTAImageProcessor(&GetFlashHandler());
 #else
     static OTAImageProcessorImpl sOTAImageProcessor;
 #endif
@@ -55,4 +59,5 @@ void InitBasicOTARequestor()
     sOTARequestor.Init(Server::GetInstance(), sOTARequestorStorage, sOTARequestorDriver, sBDXDownloader);
     chip::SetRequestorInstance(&sOTARequestor);
     sOTARequestorDriver.Init(&sOTARequestor, &imageProcessor);
+    imageProcessor.TriggerFlashAction(FlashHandler::Action::SLEEP);
 }

examples/platform/nrfconnect/util/include/DFUOverSMP.h

@@ -42,12 +42,15 @@ class DFUOverSMP
     friend DFUOverSMP & GetDFUOverSMP(void);
 
     static int UploadConfirmHandler(uint32_t offset, uint32_t size, void * arg);
+    static void OnBleConnect(bt_conn * conn, uint8_t err);
     static void OnBleDisconnect(bt_conn * conn, uint8_t reason);
     static void ChipEventHandler(const chip::DeviceLayer::ChipDeviceEvent * event, intptr_t arg);
 
     bool mIsEnabled;
     bool mIsAdvertisingEnabled;
     bt_conn_cb mBleConnCallbacks;
+    k_work mFlashSleepWork;
+    k_work mFlashWakeUpWork;
     DFUOverSMPRestartAdvertisingHandler restartAdvertisingCallback;
 
     static DFUOverSMP sDFUOverSMP;

examples/platform/nrfconnect/util/include/OTAUtil.h

@@ -17,12 +17,22 @@
 
 #pragma once
 
+#include <platform/nrfconnect/OTAImageProcessorImpl.h>
+
 namespace chip {
 namespace DeviceLayer {
 class OTAImageProcessorImpl;
 } // namespace DeviceLayer
 } // namespace chip
 
+/**
+ * Get FlashHandler static instance.
+ *
+ * Returned object can be used to control the QSPI external flash,
+ * which can be introduced into sleep mode and woken up on demand.
+ */
+chip::DeviceLayer::FlashHandler & GetFlashHandler();
+
 /**
  * Select recommended OTA image processor implementation.
  *

src/platform/nrfconnect/OTAImageProcessorImpl.cpp

@@ -37,6 +37,8 @@ CHIP_ERROR OTAImageProcessorImpl::PrepareDownload()
 {
     VerifyOrReturnError(mDownloader != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
+    TriggerFlashAction(FlashHandler::Action::WAKE_UP);
+
     return DeviceLayer::SystemLayer().ScheduleLambda([this] { mDownloader->OnPreparedForDownload(PrepareDownloadImpl()); });
 }
 
@@ -56,7 +58,10 @@ CHIP_ERROR OTAImageProcessorImpl::Finalize()
 
 CHIP_ERROR OTAImageProcessorImpl::Abort()
 {
-    return System::MapErrorZephyr(dfu_target_reset());
+    CHIP_ERROR error = System::MapErrorZephyr(dfu_target_reset());
+
+    TriggerFlashAction(FlashHandler::Action::SLEEP);
+    return error;
 }
 
 CHIP_ERROR OTAImageProcessorImpl::Apply()
@@ -68,6 +73,8 @@ CHIP_ERROR OTAImageProcessorImpl::Apply()
         err = dfu_target_schedule_update(-1);
     }
 
+    TriggerFlashAction(FlashHandler::Action::SLEEP);
+
 #ifdef CONFIG_CHIP_OTA_REQUESTOR_REBOOT_ON_APPLY
     if (!err)
     {
@@ -214,8 +221,16 @@ CHIP_ERROR OTAImageProcessorImpl::ProcessHeader(ByteSpan & aBlock)
     return CHIP_NO_ERROR;
 }
 
+void OTAImageProcessorImpl::TriggerFlashAction(FlashHandler::Action action)
+{
+    if (mFlashHandler)
+    {
+        mFlashHandler->DoAction(action);
+    }
+}
+
 // external flash power consumption optimization
-void ExtFlashHandler::DoAction(Action aAction)
+void FlashHandler::DoAction(Action aAction)
 {
 #if CONFIG_PM_DEVICE && CONFIG_NORDIC_QSPI_NOR && !CONFIG_SOC_NRF52840 // nRF52 is optimized per default
     // utilize the QSPI driver sleep power mode
@@ -228,30 +243,5 @@ void ExtFlashHandler::DoAction(Action aAction)
 #endif
 }
 
-OTAImageProcessorImplPMDevice::OTAImageProcessorImplPMDevice(ExtFlashHandler & aHandler) : mHandler(aHandler)
-{
-    mHandler.DoAction(ExtFlashHandler::Action::SLEEP);
-}
-
-CHIP_ERROR OTAImageProcessorImplPMDevice::PrepareDownload()
-{
-    mHandler.DoAction(ExtFlashHandler::Action::WAKE_UP);
-    return OTAImageProcessorImpl::PrepareDownload();
-}
-
-CHIP_ERROR OTAImageProcessorImplPMDevice::Abort()
-{
-    auto status = OTAImageProcessorImpl::Abort();
-    mHandler.DoAction(ExtFlashHandler::Action::SLEEP);
-    return status;
-}
-
-CHIP_ERROR OTAImageProcessorImplPMDevice::Apply()
-{
-    auto status = OTAImageProcessorImpl::Apply();
-    mHandler.DoAction(ExtFlashHandler::Action::SLEEP);
-    return status;
-}
-
 } // namespace DeviceLayer
 } // namespace chip

src/platform/nrfconnect/OTAImageProcessorImpl.h

@@ -27,11 +27,25 @@ class OTADownloader;
 
 namespace DeviceLayer {
 
+class FlashHandler
+{
+public:
+    enum class Action : uint8_t
+    {
+        WAKE_UP,
+        SLEEP
+    };
+    virtual ~FlashHandler() {}
+    virtual void DoAction(Action aAction);
+};
+
 class OTAImageProcessorImpl : public OTAImageProcessorInterface
 {
 public:
     static constexpr size_t kBufferSize = CONFIG_CHIP_OTA_REQUESTOR_BUFFER_SIZE;
 
+    OTAImageProcessorImpl(FlashHandler * flashHandler = nullptr) : mFlashHandler(flashHandler){};
+
     enum class ImageType : uint8_t
     {
         kAppImage = 0,
@@ -54,6 +68,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
     CHIP_ERROR ProcessBlock(ByteSpan & aBlock) override;
     bool IsFirstImageRun() override;
     CHIP_ERROR ConfirmCurrentImage() override;
+    void TriggerFlashAction(FlashHandler::Action action);
 
 private:
     CHIP_ERROR PrepareDownloadImpl();
@@ -66,30 +81,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
     uint8_t mBuffer[kBufferSize];
     OTAImageContentHeader mContentHeader;
     OTAImage mCurrentImage;
-};
-
-class ExtFlashHandler
-{
-public:
-    enum class Action : uint8_t
-    {
-        WAKE_UP,
-        SLEEP
-    };
-    virtual ~ExtFlashHandler() {}
-    virtual void DoAction(Action aAction);
-};
-
-class OTAImageProcessorImplPMDevice : public OTAImageProcessorImpl
-{
-public:
-    explicit OTAImageProcessorImplPMDevice(ExtFlashHandler & aHandler);
-    CHIP_ERROR PrepareDownload() override;
-    CHIP_ERROR Abort() override;
-    CHIP_ERROR Apply() override;
-
-private:
-    ExtFlashHandler & mHandler;
+    FlashHandler * mFlashHandler;
 };
 
 } // namespace DeviceLayer