commit ba38d9ce7668046d23a5e57c2e807ab157b7f8d8
author Roshan Pius <rpius@google.com> Fri Dec 08 07:32:08 2017 -0800
committer Roshan Pius <rpius@google.com> Mon Dec 11 23:47:43 2017 +0000
tree 526624788297aa4e03b02b0e30354f8cb15e1f43
parent 30944c7a73160c7607c2edac59b5d60f03742301

wifi(implementation): Allow chip reconfiguration

This was previously disallowed because WifiLegacyHal.stop() was non blocking.
WifiLegacyHal.stop() was made blocking using ag/2715673, so reconfiguration
should be supportable now.
Note: This is a partial revert of ag/2125543.

Bug: 65671875
Bug: 70411703
Test: Will send for regression tests.
Test: Reverted the VTS CL: ag/2139830 and ensured that the test works
now. Can't check-in that revert because it would not be backward
compatible.
Change-Id: Ia5f9ccaf5403d171b0def7740d2adece7eb3c157

wifi/1.2/default/wifi_chip.cpp

diff --git a/wifi/1.2/default/wifi_chip.cpp b/wifi/1.2/default/wifi_chip.cpp
index 79fdfdc..4e2191d 100644
--- a/wifi/1.2/default/wifi_chip.cpp
+++ b/wifi/1.2/default/wifi_chip.cpp
@@ -71,6 +71,7 @@
 namespace V1_2 {
 namespace implementation {
 using hidl_return_util::validateAndCall;
+using hidl_return_util::validateAndCallWithLock;
 
 WifiChip::WifiChip(
     ChipId chip_id, const std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal,
@@ -121,9 +122,9 @@
 
 Return<void> WifiChip::configureChip(ChipModeId mode_id,
                                      configureChip_cb hidl_status_cb) {
-    return validateAndCall(this, WifiStatusCode::ERROR_WIFI_CHIP_INVALID,
-                           &WifiChip::configureChipInternal, hidl_status_cb,
-                           mode_id);
+    return validateAndCallWithLock(
+        this, WifiStatusCode::ERROR_WIFI_CHIP_INVALID,
+        &WifiChip::configureChipInternal, hidl_status_cb, mode_id);
 }
 
 Return<void> WifiChip::getMode(getMode_cb hidl_status_cb) {
@@ -397,7 +398,9 @@
             {sta_chip_mode, ap_chip_mode}};
 }
 
-WifiStatus WifiChip::configureChipInternal(ChipModeId mode_id) {
+WifiStatus WifiChip::configureChipInternal(
+    /* NONNULL */ std::unique_lock<std::recursive_mutex>* lock,
+    ChipModeId mode_id) {
     if (mode_id != kStaChipModeId && mode_id != kApChipModeId) {
         return createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS);
     }
@@ -405,7 +408,7 @@
         LOG(DEBUG) << "Already in the specified mode " << mode_id;
         return createWifiStatus(WifiStatusCode::SUCCESS);
     }
-    WifiStatus status = handleChipConfiguration(mode_id);
+    WifiStatus status = handleChipConfiguration(lock, mode_id);
     if (status.code != WifiStatusCode::SUCCESS) {
         for (const auto& callback : event_cb_handler_.getCallbacks()) {
             if (!callback->onChipReconfigureFailure(status).isOk()) {
@@ -421,6 +424,7 @@
         }
     }
     current_mode_id_ = mode_id;
+    LOG(INFO) << "Configured chip in mode " << mode_id;
     return status;
 }
 
@@ -792,15 +796,22 @@
     return createWifiStatusFromLegacyError(legacy_status);
 }
 
-WifiStatus WifiChip::handleChipConfiguration(ChipModeId mode_id) {
+WifiStatus WifiChip::handleChipConfiguration(
+    /* NONNULL */ std::unique_lock<std::recursive_mutex>* lock,
+    ChipModeId mode_id) {
     // If the chip is already configured in a different mode, stop
     // the legacy HAL and then start it after firmware mode change.
-    // Currently the underlying implementation has a deadlock issue.
-    // We should return ERROR_NOT_SUPPORTED if chip is already configured in
-    // a different mode.
     if (current_mode_id_ != kInvalidModeId) {
-        // TODO(b/37446050): Fix the deadlock.
-        return createWifiStatus(WifiStatusCode::ERROR_NOT_SUPPORTED);
+        LOG(INFO) << "Reconfiguring chip from mode " << current_mode_id_
+                  << " to mode " << mode_id;
+        invalidateAndRemoveAllIfaces();
+        legacy_hal::wifi_error legacy_status =
+            legacy_hal_.lock()->stop(lock, []() {});
+        if (legacy_status != legacy_hal::WIFI_SUCCESS) {
+            LOG(ERROR) << "Failed to stop legacy HAL: "
+                       << legacyErrorToString(legacy_status);
+            return createWifiStatusFromLegacyError(legacy_status);
+        }
     }
     bool success;
     if (mode_id == kStaChipModeId) {

wifi/1.2/default/wifi_chip.h

diff --git a/wifi/1.2/default/wifi_chip.h b/wifi/1.2/default/wifi_chip.h
index 8cb15bb..ac59d59 100644
--- a/wifi/1.2/default/wifi_chip.h
+++ b/wifi/1.2/default/wifi_chip.h
@@ -141,7 +141,8 @@
         const sp<IWifiChipEventCallback>& event_callback);
     std::pair<WifiStatus, uint32_t> getCapabilitiesInternal();
     std::pair<WifiStatus, std::vector<ChipMode>> getAvailableModesInternal();
-    WifiStatus configureChipInternal(ChipModeId mode_id);
+    WifiStatus configureChipInternal(
+        std::unique_lock<std::recursive_mutex>* lock, ChipModeId mode_id);
     std::pair<WifiStatus, uint32_t> getModeInternal();
     std::pair<WifiStatus, IWifiChip::ChipDebugInfo>
     requestChipDebugInfoInternal();
@@ -185,7 +186,8 @@
     WifiStatus selectTxPowerScenarioInternal(TxPowerScenario scenario);
     WifiStatus resetTxPowerScenarioInternal();
 
-    WifiStatus handleChipConfiguration(ChipModeId mode_id);
+    WifiStatus handleChipConfiguration(
+        std::unique_lock<std::recursive_mutex>* lock, ChipModeId mode_id);
     WifiStatus registerDebugRingBufferCallback();
 
     ChipId chip_id_;