[aml-uart] Add lease management based on timer

Introduce a timer in the driver so that the driver can hold the wake
lease for a certain time period before dropping it. The driver resets
the timer each time when the wake vector interrupt is asserted and
drops the lease then the timer times out.

Bug: b/339233136
Test: New unittests and observation on begonia(The driver doesn't drop
the lease while packets coming in, and drops the lease a while after
the last packet).

Multiply: aml-uart-dfv2-test

Change-Id: Icc047c9609d39f9973b52385752e3e6cce0abe05
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1049252
Commit-Queue: Zhiyi Chen <zhiyichen@google.com>
Reviewed-by: Andres Oportus <andresoportus@google.com>

Author: Zhiyi Chen

src/devices/serial/drivers/aml-uart/BUILD.gn

@@ -195,6 +195,7 @@ test("aml-uart-dfv2-test-bin") {
   sources = [
     "tests/aml-uart-dfv2-test.cc",
     "tests/device_state.h",
+    "tests/fake_timer.cc",
   ]
   deps = [
     ":aml-uart-dfv2-source",

src/devices/serial/drivers/aml-uart/aml-uart-dfv2.cc

@@ -7,6 +7,7 @@
 #include <lib/ddk/metadata.h>
 #include <lib/driver/component/cpp/driver_export.h>
 #include <lib/driver/component/cpp/node_add_args.h>
+#include <lib/driver/power/cpp/element-description-builder.h>
 #include <lib/driver/power/cpp/power-support.h>
 
 namespace serial {
@@ -50,7 +51,8 @@ void AmlUartV2::PrepareStop(fdf::PrepareStopCompleter completer) {
     prepare_stop_completer_.emplace(std::move(completer));
     irq_dispatcher_->ShutdownAsync();
   } else {
-    // No irq_dispatcher_, just reply to the PrepareStopCompleter.
+    // No irq_dispatcher_, just reply to the PrepareStopCompleter, timer_dispatcher_ is created
+    // after irq_dispatcher_, so we assume that there's no timer_dispatcher_ either.
     completer(zx::ok());
   }
 }
@@ -60,10 +62,6 @@ AmlUart& AmlUartV2::aml_uart_for_testing() {
   return aml_uart_.value();
 }
 
-fidl::ClientEnd<fuchsia_power_broker::ElementControl>& AmlUartV2::element_control_for_testing() {
-  return element_control_client_end_;
-}
-
 void AmlUartV2::OnReceivedMetadata(
     fidl::WireUnownedResult<fuchsia_driver_compat::Device::GetMetadata>& metadata_result) {
   if (!metadata_result.ok()) {
@@ -163,20 +161,19 @@ zx_status_t AmlUartV2::GetPowerConfiguration(
     return ZX_ERR_INTERNAL;
   }
 
-  zx::result lessor_endpoints = fidl::CreateEndpoints<fuchsia_power_broker::Lessor>();
-
-  auto result_add_element =
-      fdf_power::AddElement(power_broker.value(), config, std::move(tokens.value()), {}, {}, {},
-                            std::move(lessor_endpoints->server));
+  fdf_power::ElementDesc description =
+      fdf_power::ElementDescBuilder(config, std::move(tokens.value())).Build();
+  auto result_add_element = fdf_power::AddElement(power_broker.value(), description);
   if (result_add_element.is_error()) {
     FDF_LOG(ERROR, "Failed to add power element: %u",
             static_cast<uint8_t>(result_add_element.error_value()));
     return ZX_ERR_INTERNAL;
   }
 
   element_control_client_end_ = std::move(result_add_element->element_control_channel());
-  lessor_client_end_ = std::move(lessor_endpoints->client);
-
+  lessor_client_end_ = std::move(*description.lessor_client_);
+  current_level_client_end_ = std::move(*description.current_level_client_);
+  required_level_client_end_ = std::move(*description.required_level_client_);
   return ZX_OK;
 }
 
@@ -216,6 +213,11 @@ void AmlUartV2::OnDeviceServerInitialized(zx::result<> device_server_init_result
 
   zx::result irq_dispatcher_result =
       fdf::SynchronizedDispatcher::Create({}, "aml_uart_irq", [this](fdf_dispatcher_t*) {
+        // Shutdown timer dispatcher if there is one, otherwise call prepare_stop_completer_.
+        if (timer_dispatcher_) {
+          timer_dispatcher_->ShutdownAsync();
+          return;
+        }
         if (prepare_stop_completer_.has_value()) {
           fdf::PrepareStopCompleter completer = std::move(prepare_stop_completer_.value());
           prepare_stop_completer_.reset();
@@ -229,8 +231,27 @@ void AmlUartV2::OnDeviceServerInitialized(zx::result<> device_server_init_result
   }
 
   irq_dispatcher_.emplace(std::move(irq_dispatcher_result.value()));
+
+  zx::result timer_dispatcher_result =
+      fdf::SynchronizedDispatcher::Create({}, "aml_uart_timer", [this](fdf_dispatcher_t*) {
+        if (prepare_stop_completer_.has_value()) {
+          fdf::PrepareStopCompleter completer = std::move(prepare_stop_completer_.value());
+          prepare_stop_completer_.reset();
+          completer(zx::ok());
+        }
+      });
+  if (timer_dispatcher_result.is_error()) {
+    FDF_LOG(ERROR, "Failed to create timer dispatcher: %s",
+            timer_dispatcher_result.status_string());
+    CompleteStart(timer_dispatcher_result.take_error());
+    return;
+  }
+
+  timer_dispatcher_.emplace(std::move(timer_dispatcher_result.value()));
   aml_uart_.emplace(std::move(pdev), serial_port_info_, std::move(mmio.value()),
-                    irq_dispatcher_->borrow(), std::move(lessor_client_end_));
+                    irq_dispatcher_->borrow(), timer_dispatcher_->borrow(),
+                    std::move(lessor_client_end_), std::move(current_level_client_end_),
+                    std::move(required_level_client_end_), std::move(element_control_client_end_));
 
   // Default configuration for the case that serial_impl_config is not called.
   constexpr uint32_t kDefaultBaudRate = 115200;

src/devices/serial/drivers/aml-uart/aml-uart-dfv2.h

@@ -26,8 +26,6 @@ class AmlUartV2 : public fdf::DriverBase {
   // Used by the unit test to access the device.
   AmlUart& aml_uart_for_testing();
 
-  fidl::ClientEnd<fuchsia_power_broker::ElementControl>& element_control_for_testing();
-
  private:
   zx_status_t GetPowerConfiguration(
       const fidl::WireSyncClient<fuchsia_hardware_platform_device::Device>& pdev);
@@ -47,6 +45,7 @@ class AmlUartV2 : public fdf::DriverBase {
   fidl::WireClient<fuchsia_driver_framework::Node> parent_node_client_;
   fuchsia_hardware_serial::wire::SerialPortInfo serial_port_info_;
   std::optional<fdf::SynchronizedDispatcher> irq_dispatcher_;
+  std::optional<fdf::SynchronizedDispatcher> timer_dispatcher_;
   std::optional<AmlUart> aml_uart_;
   std::optional<fdf::PrepareStopCompleter> prepare_stop_completer_;
   compat::AsyncInitializedDeviceServer device_server_;
@@ -55,7 +54,9 @@ class AmlUartV2 : public fdf::DriverBase {
   aml_uart_dfv2_config::Config driver_config_;
 
   // Client ends for talking to power broker.
-  fidl::ClientEnd<fuchsia_power_broker::ElementControl> element_control_client_end_;
+  std::optional<fidl::ClientEnd<fuchsia_power_broker::CurrentLevel>> current_level_client_end_;
+  std::optional<fidl::ClientEnd<fuchsia_power_broker::RequiredLevel>> required_level_client_end_;
+  std::optional<fidl::ClientEnd<fuchsia_power_broker::ElementControl>> element_control_client_end_;
   std::optional<fidl::ClientEnd<fuchsia_power_broker::Lessor>> lessor_client_end_;
 };
 

src/devices/serial/drivers/aml-uart/aml-uart.cc

@@ -10,6 +10,7 @@
 #include <lib/driver/compat/cpp/logging.h>  // nogncheck
 #endif
 
+#include <lib/zx/clock.h>
 #include <threads.h>
 #include <zircon/syscalls-next.h>
 #include <zircon/threads.h>
@@ -48,19 +49,88 @@ fit::closure DriverTransportWriteOperation::MakeCallback(zx_status_t status) {
 
 }  // namespace internal
 
-AmlUart::AmlUart(ddk::PDevFidl pdev,
-                 const fuchsia_hardware_serial::wire::SerialPortInfo& serial_port_info,
-                 fdf::MmioBuffer mmio, fdf::UnownedSynchronizedDispatcher irq_dispatcher,
-                 std::optional<fidl::ClientEnd<fuchsia_power_broker::Lessor>> lessor_client_end)
+AmlUart::AmlUart(
+    ddk::PDevFidl pdev, const fuchsia_hardware_serial::wire::SerialPortInfo& serial_port_info,
+    fdf::MmioBuffer mmio, fdf::UnownedSynchronizedDispatcher irq_dispatcher,
+    std::optional<fdf::UnownedSynchronizedDispatcher> timer_dispatcher,
+    std::optional<fidl::ClientEnd<fuchsia_power_broker::Lessor>> lessor_client_end,
+    std::optional<fidl::ClientEnd<fuchsia_power_broker::CurrentLevel>> current_level_client_end,
+    std::optional<fidl::ClientEnd<fuchsia_power_broker::RequiredLevel>> required_level_client_end,
+    std::optional<fidl::ClientEnd<fuchsia_power_broker::ElementControl>> element_control_client_end)
     : pdev_(std::move(pdev)),
       serial_port_info_(serial_port_info),
       mmio_(std::move(mmio)),
       irq_dispatcher_(std::move(irq_dispatcher)) {
+  if (timer_dispatcher != std::nullopt) {
+    timer_dispatcher_ = std::move(*timer_dispatcher);
+    // Use ZX_TIMER_SLACK_LATE so that the timer will never fire earlier than the deadline.
+    zx::timer::create(ZX_TIMER_SLACK_LATE, ZX_CLOCK_MONOTONIC, &lease_timer_);
+    timer_waiter_.set_object(lease_timer_.get());
+    timer_waiter_.set_trigger(ZX_TIMER_SIGNALED);
+  }
+
   if (lessor_client_end != std::nullopt) {
     lessor_client_.Bind(std::move(*lessor_client_end));
   } else {
     zxlogf(INFO, "No lessor client end gets passed into AmlUart during initialization.");
   }
+
+  if (required_level_client_end != std::nullopt) {
+    required_level_client_.Bind(std::move(*required_level_client_end),
+                                fdf::Dispatcher::GetCurrent()->async_dispatcher());
+  } else {
+    zxlogf(INFO, "No required level client end gets passed into AmlUart during initialization.");
+  }
+
+  if (current_level_client_end != std::nullopt) {
+    current_level_client_.Bind(std::move(*current_level_client_end));
+  } else {
+    zxlogf(INFO, "No current level client end gets passed into AmlUart during initialization.");
+  }
+
+  if (element_control_client_end != std::nullopt) {
+    element_control_client_end_ = std::move(element_control_client_end);
+    WatchRequiredLevel();
+  } else {
+    zxlogf(INFO, "No element control client end gets passed into AmlUart during initialization.");
+  }
+}
+
+// Keep watching the required level, since the driver doesn't toggle the hardware power state
+// according to power element levels, report the current level to power broker immediately with the
+// required level value received from power broker.
+void AmlUart::WatchRequiredLevel() {
+  if (!required_level_client_.is_valid()) {
+    zxlogf(ERROR, "RequiredLevel not valid, can't monitor it");
+    element_control_client_end_.reset();
+    return;
+  }
+  required_level_client_->Watch().Then(
+      [this](fidl::Result<fuchsia_power_broker::RequiredLevel::Watch>& result) {
+        if (result.is_error()) {
+          if (result.error_value().is_framework_error() &&
+              result.error_value().framework_error().status() != ZX_ERR_CANCELED) {
+            zxlogf(ERROR, "Power level required call failed: %s. Stop monitoring required level",
+                   result.error_value().FormatDescription().c_str());
+          }
+          element_control_client_end_.reset();
+          return;
+        }
+        zxlogf(DEBUG, "RequiredLevel : %u", static_cast<uint8_t>(result->required_level()));
+        if (!current_level_client_.is_valid()) {
+          zxlogf(ERROR, "CurrentLevel not valid. Stop monitoring required level");
+          element_control_client_end_.reset();
+          return;
+        }
+        auto update_result = current_level_client_->Update(result->required_level());
+        if (update_result.is_error()) {
+          zxlogf(ERROR, "CurrentLevel call failed: %s. Stop monitoring required level",
+                 update_result.error_value().FormatDescription().c_str());
+          element_control_client_end_.reset();
+          return;
+        }
+        WatchRequiredLevel();
+      });
 }
 
 constexpr auto kMinBaudRate = 2;
@@ -200,6 +270,10 @@ void AmlUart::EnableLocked(bool enable) {
   }
 }
 
+fidl::ClientEnd<fuchsia_power_broker::ElementControl>& AmlUart::element_control_for_testing() {
+  return *element_control_client_end_;
+}
+
 void AmlUart::HandleTXRaceForTest() {
   {
     fbl::AutoLock al(&enable_lock_);
@@ -220,6 +294,12 @@ void AmlUart::HandleRXRaceForTest() {
   HandleRX();
 }
 
+void AmlUart::InjectTimerForTest(zx_handle_t handle) {
+  lease_timer_.reset(handle);
+  timer_waiter_.set_object(lease_timer_.get());
+  timer_waiter_.set_trigger(ZX_TIMER_SIGNALED);
+}
+
 zx_status_t AmlUart::Enable(bool enable) {
   fbl::AutoLock al(&enable_lock_);
 
@@ -388,11 +468,60 @@ void AmlUart::handle_unknown_method(
   zxlogf(WARNING, "handle_unknown_method in fuchsia_hardware_serialimpl::Device server.");
 }
 
+zx::result<fidl::ClientEnd<fuchsia_power_broker::LeaseControl>> AmlUart::AcquireLease() {
+  auto lease_result = lessor_client_->Lease(kPowerLevelHandling);
+  if (lease_result.is_error()) {
+    zxlogf(ERROR, "Failed to aquire lease: %s",
+           lease_result.error_value().FormatDescription().c_str());
+    return zx::error(ZX_ERR_BAD_STATE);
+  }
+  if (!lease_result->lease_control().is_valid()) {
+    zxlogf(ERROR, "Lease returned invalid lease control client end.");
+    return zx::error(ZX_ERR_BAD_STATE);
+  }
+  // Wait until the lease is actually granted.
+  fidl::SyncClient<fuchsia_power_broker::LeaseControl> lease_control_client(
+      std::move(lease_result->lease_control()));
+  fuchsia_power_broker::LeaseStatus current_lease_status =
+      fuchsia_power_broker::LeaseStatus::kUnknown;
+  do {
+    auto watch_result = lease_control_client->WatchStatus(current_lease_status);
+    if (watch_result.is_error()) {
+      zxlogf(ERROR, "WatchStatus returned error: %s",
+             watch_result.error_value().FormatDescription().c_str());
+      return zx::error(ZX_ERR_BAD_STATE);
+    }
+    current_lease_status = watch_result->status();
+  } while (current_lease_status != fuchsia_power_broker::LeaseStatus::kSatisfied);
+  return zx::ok(lease_control_client.TakeClientEnd());
+}
+
 void AmlUart::HandleIrq(async_dispatcher_t* dispatcher, async::IrqBase* irq, zx_status_t status,
                         const zx_packet_interrupt_t* interrupt) {
   if (status != ZX_OK) {
     return;
   }
+  // If the element control client end is not available, it means that power management is not
+  // enabled for this driver, the driver will not take a wake lease and set timer in this case.
+  if (element_control_client_end_) {
+    fbl::AutoLock lock(&timer_lock_);
+
+    if (lease_control_client_end_.is_valid()) {
+      // Cancel the timer and set it again if the last one hasn't expired.
+      lease_timer_.cancel();
+    } else {
+      // Take the wake lease and keep the lease control client.
+      auto lease_control_result = AcquireLease();
+      if (lease_control_result.is_error()) {
+        return;
+      }
+      lease_control_client_end_ = std::move(*lease_control_result);
+    }
+
+    timeout_ = zx::deadline_after(zx::msec(kPowerLeaseTimeoutMs));
+    timer_waiter_.Begin(timer_dispatcher_->async_dispatcher());
+    lease_timer_.set(timeout_, zx::duration());
+  }
 
   auto uart_status = Status::Get().ReadFrom(&mmio_);
   if (!uart_status.rx_empty()) {
@@ -405,4 +534,19 @@ void AmlUart::HandleIrq(async_dispatcher_t* dispatcher, async::IrqBase* irq, zx_
   irq_.ack();
 }
 
+void AmlUart::HandleLeaseTimer(async_dispatcher_t* dispatcher, async::WaitBase* wait,
+                               zx_status_t status, const zx_packet_signal_t* signal) {
+  fbl::AutoLock lock(&timer_lock_);
+  if (status == ZX_ERR_CANCELED) {
+    // Do nothing if the handler is triggered by the destroy of the dispatcher.
+    return;
+  }
+  if (zx::clock::get_monotonic() < timeout_) {
+    // If the current time is earlier than timeout, it means that this handler is triggered after
+    // |HandleIrq| holds the lock, and when this handler get the lock, the timer has been reset, so
+    // don't drop the lease in this case.
+    return;
+  }
+  lease_control_client_end_.reset();
+}
 }  // namespace serial