Revert "[power] Move domain creation to core driver"
This reverts commit dc6c2823091ee9eab28f45b2c0d667f1cc400a5b.
Reason for revert: device-enumeration-test failure
Original change's description:
> [power] Move domain creation to core driver
>
> Power domains were created in the board driver using composite nodes.
> There are a couple of issues with that mechanism: the logic will have to
> be repeated in every board driver, this deviates from how gpio/i2c
> children are created by the core drivers, this will be harder to create
> when nested domain structure need to be created.
>
> This CL updates the metadata passed to the power impl driver to include
> domain information, which is forwarded to the power core. The power core
> would internally create all the necessary domain nodes for relevant
> power domain clients to bind to.
> The motivation to make this change right now is it make the metadata and
> node creation more streamlined for addition of devicetree support.
>
> Bug: 333925701
> Test: device enumeration tests, power-test
> Change-Id: I2261e242f0f8124c8407e8a9055977090c80c3d5
> Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1027106
> Reviewed-by: Gurjant Kalsi <gkalsi@google.com>
> Reviewed-by: Suraj Malhotra <surajmalhotra@google.com>
> Reviewed-by: Christopher Anderson <cja@google.com>
> API-Review: Christopher Anderson <cja@google.com>
> Fuchsia-Auto-Submit: Puneetha Ramachandra <puneetha@google.com>
> Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
Bug: 333925701
Change-Id: I272f014cba4212c50c621c1b1963b267fc083ca6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1037433
Commit-Queue: Puneetha Ramachandra <puneetha@google.com>
Reviewed-by: RubberStamper 🤖 <android-build-ayeaye@system.gserviceaccount.com>
diff --git a/sdk/fidl/fuchsia.hardware.power/BUILD.gn b/sdk/fidl/fuchsia.hardware.power/BUILD.gn
index c2bd057..794645f 100644
--- a/sdk/fidl/fuchsia.hardware.power/BUILD.gn
+++ b/sdk/fidl/fuchsia.hardware.power/BUILD.gn
@@ -7,7 +7,6 @@
fidl("fuchsia.hardware.power") {
sources = [
"config.fidl",
- "metadata.fidl",
"overview.fidl",
"power.fidl",
]
diff --git a/sdk/fidl/fuchsia.hardware.power/fuchsia.hardware.power.api b/sdk/fidl/fuchsia.hardware.power/fuchsia.hardware.power.api
index 5b99df6..3acc1ae 100644
--- a/sdk/fidl/fuchsia.hardware.power/fuchsia.hardware.power.api
+++ b/sdk/fidl/fuchsia.hardware.power/fuchsia.hardware.power.api
@@ -1,3 +1,3 @@
{
- "fidl/fuchsia.hardware.power": "123bf2daba2016012c8fd577476d0a41"
+ "fidl/fuchsia.hardware.power": "bbf1cef0cefb74f7fd0954212b84205f"
}
\ No newline at end of file
diff --git a/sdk/fidl/fuchsia.hardware.power/metadata.fidl b/sdk/fidl/fuchsia.hardware.power/metadata.fidl
deleted file mode 100644
index ebea251..0000000
--- a/sdk/fidl/fuchsia.hardware.power/metadata.fidl
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright 2024 The Fuchsia Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-library fuchsia.hardware.power;
-
-/// Information related to a power domain.
-type Domain = table {
- /// Domain ID should be unique across all power domains in the same level.
- /// Used to associate power consumers that belong to the same power domain.
- 1: id uint32;
-};
-
-/// Passed to the power core driver in metadata as DEVICE_METADATA_POWER_DOMAINS.
-type DomainMetadata = table {
- /// List of power domains to be managed by this power driver.
- 1: domains vector<Domain>:MAX;
-};
diff --git a/src/devices/board/drivers/astro/astro-power.cc b/src/devices/board/drivers/astro/astro-power.cc
index af14fe5..a7d6fea 100644
--- a/src/devices/board/drivers/astro/astro-power.cc
+++ b/src/devices/board/drivers/astro/astro-power.cc
@@ -18,6 +18,7 @@
#include <bind/fuchsia/hardware/pwm/cpp/bind.h>
#include <bind/fuchsia/platform/cpp/bind.h>
#include <bind/fuchsia/power/cpp/bind.h>
+#include <ddk/metadata/power.h>
#include <soc/aml-common/aml-power.h>
#include <soc/aml-s905d2/s905d2-pwm.h>
@@ -44,19 +45,13 @@
constexpr voltage_pwm_period_ns_t kS905d2PwmPeriodNs = 1250;
+constexpr power_domain_t domains[] = {
+ {bind_fuchsia_amlogic_platform::POWER_DOMAIN_ARM_CORE_LITTLE},
+};
+
} // namespace
zx_status_t AddPowerImpl(fdf::WireSyncClient<fuchsia_hardware_platform_bus::PlatformBus>& pbus) {
- fuchsia_hardware_power::DomainMetadata domain_metadata = {
- {.domains = {{{{.id = {bind_fuchsia_amlogic_platform::POWER_DOMAIN_ARM_CORE_LITTLE}}}}}}};
-
- const fit::result encoded_metadata = fidl::Persist(domain_metadata);
- if (!encoded_metadata.is_ok()) {
- zxlogf(ERROR, "Failed to encode power domain metadata: %s",
- encoded_metadata.error_value().FormatDescription().c_str());
- return encoded_metadata.error_value().status();
- }
-
fpbus::Node dev;
dev.name() = "aml-power-impl-composite";
dev.vid() = bind_fuchsia_google_platform::BIND_PLATFORM_DEV_VID_GOOGLE;
@@ -75,10 +70,6 @@
reinterpret_cast<const uint8_t*>(&kS905d2PwmPeriodNs),
reinterpret_cast<const uint8_t*>(&kS905d2PwmPeriodNs) + sizeof(kS905d2PwmPeriodNs)),
}},
- {{
- .type = DEVICE_METADATA_POWER_DOMAINS,
- .data = encoded_metadata.value(),
- }},
};
const std::vector<fuchsia_driver_framework::BindRule> kPwmRules = {
@@ -111,6 +102,54 @@
return ZX_OK;
}
+zx_status_t AddPdArmcore(fdf::WireSyncClient<fuchsia_hardware_platform_bus::PlatformBus>& pbus) {
+ fpbus::Node dev;
+ dev.name() = "composite-pd-armcore";
+ dev.vid() = bind_fuchsia_platform::BIND_PLATFORM_DEV_VID_GENERIC;
+ dev.pid() = bind_fuchsia_platform::BIND_PLATFORM_DEV_PID_GENERIC;
+ dev.did() = bind_fuchsia_platform::BIND_PLATFORM_DEV_DID_POWER_CORE;
+ dev.metadata() = std::vector<fpbus::Metadata>{
+ {{
+ .type = DEVICE_METADATA_POWER_DOMAINS,
+ .data =
+ std::vector<uint8_t>(reinterpret_cast<const uint8_t*>(&domains),
+ reinterpret_cast<const uint8_t*>(&domains) + sizeof(domains)),
+ }},
+ };
+
+ const std::vector<fdf::BindRule> kPowerArmcoreRules = std::vector{
+ fdf::MakeAcceptBindRule(bind_fuchsia::PROTOCOL, bind_fuchsia_power::BIND_PROTOCOL_IMPL),
+ };
+
+ const std::vector<fdf::NodeProperty> kPowerArmcoreProperties = std::vector{
+ fdf::MakeProperty(bind_fuchsia::PROTOCOL, bind_fuchsia_power::BIND_PROTOCOL_IMPL),
+ };
+
+ const std::vector<fdf::ParentSpec> kParents = {
+ {
+ kPowerArmcoreRules,
+ kPowerArmcoreProperties,
+ },
+ };
+
+ fidl::Arena<> fidl_arena;
+ fdf::Arena arena('PDAC');
+ auto result = pbus.buffer(arena)->AddCompositeNodeSpec(
+ fidl::ToWire(fidl_arena, dev),
+ fidl::ToWire(fidl_arena,
+ fdf::CompositeNodeSpec{{.name = "pd_armcore", .parents = kParents}}));
+ if (!result.ok()) {
+ zxlogf(ERROR, "Failed to send AddCompositeNodeSpec request: %s", result.status_string());
+ return result.status();
+ }
+ if (result->is_error()) {
+ zxlogf(ERROR, "Failed to add composite: %s", zx_status_get_string(result->error_value()));
+ return result->error_value();
+ }
+
+ return ZX_OK;
+}
+
zx_status_t Astro::PowerInit() {
zx_status_t status = AddPowerImpl(pbus_);
if (status != ZX_OK) {
@@ -118,6 +157,12 @@
return status;
}
+ status = AddPdArmcore(pbus_);
+ if (status != ZX_OK) {
+ zxlogf(ERROR, "Failed to add pd-armcore composite device: %s", zx_status_get_string(status));
+ return status;
+ }
+
return ZX_OK;
}
diff --git a/src/devices/board/drivers/vim3/vim3-power.cc b/src/devices/board/drivers/vim3/vim3-power.cc
index 71c715f..139208c 100644
--- a/src/devices/board/drivers/vim3/vim3-power.cc
+++ b/src/devices/board/drivers/vim3/vim3-power.cc
@@ -4,7 +4,6 @@
#include <fidl/fuchsia.hardware.platform.bus/cpp/driver/fidl.h>
#include <fidl/fuchsia.hardware.platform.bus/cpp/fidl.h>
-#include <fidl/fuchsia.hardware.power/cpp/fidl.h>
#include <lib/ddk/binding.h>
#include <lib/ddk/debug.h>
#include <lib/ddk/device.h>
@@ -27,6 +26,7 @@
#include <bind/fuchsia/platform/cpp/bind.h>
#include <bind/fuchsia/power/cpp/bind.h>
#include <bind/fuchsia/pwm/cpp/bind.h>
+#include <ddk/metadata/power.h>
#include <ddktl/device.h>
#include <soc/aml-a311d/a311d-power.h>
#include <soc/aml-a311d/a311d-pwm.h>
@@ -72,7 +72,7 @@
fdf::MakeProperty(bind_fuchsia_amlogic_platform::PWM_ID,
bind_fuchsia_amlogic_platform::PWM_ID_A)};
-static fpbus::Node power_dev = []() {
+static const fpbus::Node power_dev = []() {
fpbus::Node dev = {};
dev.name() = "aml-power-impl-composite";
dev.vid() = bind_fuchsia_amlogic_platform::BIND_PLATFORM_DEV_VID_AMLOGIC;
@@ -81,6 +81,14 @@
return dev;
}();
+constexpr power_domain_t big_domain[] = {
+ {bind_fuchsia_amlogic_platform::POWER_DOMAIN_ARM_CORE_BIG},
+};
+
+constexpr power_domain_t little_domain[] = {
+ {bind_fuchsia_amlogic_platform::POWER_DOMAIN_ARM_CORE_LITTLE},
+};
+
const ddk::BindRule kI2cRules[] = {
ddk::MakeAcceptBindRule(bind_fuchsia_hardware_i2c::SERVICE,
bind_fuchsia_hardware_i2c::SERVICE_ZIRCONTRANSPORT),
@@ -102,6 +110,87 @@
bind_fuchsia_hardware_gpio::SERVICE_ZIRCONTRANSPORT),
ddk::MakeProperty(bind_fuchsia_gpio::FUNCTION, bind_fuchsia_gpio::FUNCTION_USB_POWER_DELIVERY)};
+const std::vector<fdf::BindRule> kPowerArmcoreRules = std::vector{
+ fdf::MakeAcceptBindRule(bind_fuchsia::PROTOCOL, bind_fuchsia_power::BIND_PROTOCOL_IMPL),
+};
+
+const std::vector<fdf::NodeProperty> kPowerArmcoreProperties = std::vector{
+ fdf::MakeProperty(bind_fuchsia::PROTOCOL, bind_fuchsia_power::BIND_PROTOCOL_IMPL),
+};
+
+const std::vector<fdf::ParentSpec> kPowerArmcoreParents = {
+ {
+ kPowerArmcoreRules,
+ kPowerArmcoreProperties,
+ },
+};
+
+zx_status_t AddBigCore(fdf::WireSyncClient<fuchsia_hardware_platform_bus::PlatformBus>& pbus) {
+ fpbus::Node big_core_dev;
+ big_core_dev.name() = "pd-big-core";
+ big_core_dev.vid() = bind_fuchsia_platform::BIND_PLATFORM_DEV_VID_GENERIC;
+ big_core_dev.pid() = bind_fuchsia_platform::BIND_PLATFORM_DEV_PID_GENERIC;
+ big_core_dev.did() = bind_fuchsia_platform::BIND_PLATFORM_DEV_DID_POWER_CORE;
+ big_core_dev.instance_id() = 0;
+ big_core_dev.metadata() = std::vector<fpbus::Metadata>{
+ {{
+ .type = DEVICE_METADATA_POWER_DOMAINS,
+ .data = std::vector<uint8_t>(
+ reinterpret_cast<const uint8_t*>(&big_domain),
+ reinterpret_cast<const uint8_t*>(&big_domain) + sizeof(big_domain)),
+ }},
+ };
+
+ fidl::Arena<> fidl_arena;
+ fdf::Arena big_core_arena('BIGC');
+ fdf::WireUnownedResult big_core_result =
+ pbus.buffer(big_core_arena)
+ ->AddCompositeNodeSpec(
+ fidl::ToWire(fidl_arena, big_core_dev),
+ fidl::ToWire(fidl_arena, fdf::CompositeNodeSpec{{.name = "pd_big_core",
+ .parents = kPowerArmcoreParents}}));
+ if (!big_core_result.ok() || big_core_result.value().is_error()) {
+ zxlogf(ERROR, "AddCompositeNodeSpec for big core failed, error = %s",
+ big_core_result.FormatDescription().c_str());
+ return ZX_ERR_INTERNAL;
+ }
+
+ return ZX_OK;
+}
+
+zx_status_t AddLittleCore(fdf::WireSyncClient<fuchsia_hardware_platform_bus::PlatformBus>& pbus) {
+ fpbus::Node little_core_dev;
+ little_core_dev.name() = "pd-little-core";
+ little_core_dev.vid() = bind_fuchsia_platform::BIND_PLATFORM_DEV_VID_GENERIC;
+ little_core_dev.pid() = bind_fuchsia_platform::BIND_PLATFORM_DEV_PID_GENERIC;
+ little_core_dev.did() = bind_fuchsia_platform::BIND_PLATFORM_DEV_DID_POWER_CORE;
+ little_core_dev.instance_id() = 1;
+ little_core_dev.metadata() = std::vector<fpbus::Metadata>{
+ {{
+ .type = DEVICE_METADATA_POWER_DOMAINS,
+ .data = std::vector<uint8_t>(
+ reinterpret_cast<const uint8_t*>(&little_domain),
+ reinterpret_cast<const uint8_t*>(&little_domain) + sizeof(little_domain)),
+ }},
+ };
+
+ fidl::Arena<> fidl_arena;
+ fdf::Arena little_core_arena('LITC');
+ fdf::WireUnownedResult little_core_result =
+ pbus.buffer(little_core_arena)
+ ->AddCompositeNodeSpec(
+ fidl::ToWire(fidl_arena, little_core_dev),
+ fidl::ToWire(fidl_arena, fdf::CompositeNodeSpec{{.name = "pd_little_core",
+ .parents = kPowerArmcoreParents}}));
+ if (!little_core_result.ok() || little_core_result.value().is_error()) {
+ zxlogf(ERROR, "AddCompositeNodeSpec for little core failed, error = %s",
+ little_core_result.FormatDescription().c_str());
+ return ZX_ERR_INTERNAL;
+ }
+
+ return ZX_OK;
+}
+
zx_status_t AddVreg(uint32_t pwm_id,
fdf::WireSyncClient<fuchsia_hardware_platform_bus::PlatformBus>& pbus) {
auto gpio_init_node = fuchsia_driver_framework::ParentSpec{{
@@ -196,30 +285,6 @@
return status;
}
- {
- fuchsia_hardware_power::DomainMetadata metadata = {
- {.domains = {{
- {{.id = {bind_fuchsia_amlogic_platform::POWER_DOMAIN_ARM_CORE_LITTLE}}},
- {{.id = {bind_fuchsia_amlogic_platform::POWER_DOMAIN_ARM_CORE_BIG}}},
- }}}};
-
- const fit::result encoded_metadata = fidl::Persist(metadata);
- if (!encoded_metadata.is_ok()) {
- zxlogf(ERROR, "Failed to encode power domain metadata: %s",
- encoded_metadata.error_value().FormatDescription().c_str());
- return encoded_metadata.error_value().status();
- }
-
- const std::vector<fpbus::Metadata> power_metadata{
- {{
- .type = DEVICE_METADATA_POWER_DOMAINS,
- .data = encoded_metadata.value(),
- }},
- };
-
- power_dev.metadata() = power_metadata;
- }
-
fidl::Arena<> fidl_arena;
fdf::Arena arena('PWR_');
const std::vector<fdf::ParentSpec> kAmlPowerImplComposite = {
@@ -240,6 +305,20 @@
return result->error_value();
}
+ status = AddBigCore(pbus_);
+ if (status != ZX_OK) {
+ zxlogf(ERROR, "%s: CompositeDeviceAdd for power domain Big Arm Core failed, status = %d",
+ __FUNCTION__, status);
+ return status;
+ }
+
+ status = AddLittleCore(pbus_);
+ if (status != ZX_OK) {
+ zxlogf(ERROR, "%s: CompositeDeviceAdd for power domain Little Arm Core failed, status = %d",
+ __FUNCTION__, status);
+ return status;
+ }
+
// Add USB power delivery unit
status = DdkAddCompositeNodeSpec(
"fusb302",
diff --git a/src/devices/power/drivers/aml-meson-power/aml-power.cc b/src/devices/power/drivers/aml-meson-power/aml-power.cc
index 4079c06..d52a1a4 100644
--- a/src/devices/power/drivers/aml-meson-power/aml-power.cc
+++ b/src/devices/power/drivers/aml-meson-power/aml-power.cc
@@ -452,8 +452,7 @@
std::unique_ptr<AmlPower> power_impl_device =
std::make_unique<AmlPower>(parent, std::move(domain_info));
- st = power_impl_device->DdkAdd(
- ddk::DeviceAddArgs("power-impl").forward_metadata(parent, DEVICE_METADATA_POWER_DOMAINS));
+ st = power_impl_device->DdkAdd("power-impl", DEVICE_ADD_ALLOW_MULTI_COMPOSITE);
if (st != ZX_OK) {
zxlogf(ERROR, "%s: DdkAdd failed, st = %d", __func__, st);
}
diff --git a/src/devices/power/drivers/power/BUILD.gn b/src/devices/power/drivers/power/BUILD.gn
index 2c2c7ab..915a955 100644
--- a/src/devices/power/drivers/power/BUILD.gn
+++ b/src/devices/power/drivers/power/BUILD.gn
@@ -41,6 +41,7 @@
"//src/devices/bind/fuchsia.power:fuchsia.power_cpp",
"//src/devices/lib/driver",
"//src/lib/ddk",
+ "//src/lib/ddk:ddk-metadata-headers",
"//src/lib/ddktl",
"//zircon/system/ulib/fbl",
"//zircon/system/ulib/zx",
diff --git a/src/devices/power/drivers/power/bind-tests.json b/src/devices/power/drivers/power/bind-tests.json
index 55520c1..d21a2c7 100644
--- a/src/devices/power/drivers/power/bind-tests.json
+++ b/src/devices/power/drivers/power/bind-tests.json
@@ -1,16 +1,30 @@
[
{
- "device": {
- "fuchsia.BIND_PROTOCOL": "fuchsia.gpio.BIND_PROTOCOL.IMPL"
- },
- "expected": "abort",
- "name": "Incorrect_Protocol"
+ "node": "power-impl",
+ "tests": [
+ {
+ "name": "Match",
+ "expected": "match",
+ "device": {
+ "fuchsia.BIND_PROTOCOL": "fuchsia.power.BIND_PROTOCOL.IMPL"
+ }
+ }
+ ]
},
{
- "device": {
- "fuchsia.BIND_PROTOCOL": "fuchsia.power.BIND_PROTOCOL.IMPL"
- },
- "expected": "match",
- "name": "Correct_Protocol"
+ "node": "pdev",
+ "tests": [
+ {
+ "name": "Match",
+ "expected": "match",
+ "device": {
+ "fuchsia.BIND_PROTOCOL": "fuchsia.platform.BIND_PROTOCOL.DEVICE",
+ "fuchsia.BIND_PLATFORM_DEV_PID": "fuchsia.platform.BIND_PLATFORM_DEV_PID.GENERIC",
+ "fuchsia.BIND_PLATFORM_DEV_VID": "fuchsia.platform.BIND_PLATFORM_DEV_VID.GENERIC",
+ "fuchsia.BIND_PLATFORM_DEV_DID": "fuchsia.platform.BIND_PLATFORM_DEV_DID.POWER_CORE",
+ "fuchsia.BIND_PLATFORM_DEV_INSTANCE_ID": "0"
+ }
+ }
+ ]
}
-]
+]
\ No newline at end of file
diff --git a/src/devices/power/drivers/power/power-test.cc b/src/devices/power/drivers/power/power-test.cc
index caf9474..1ac6eea 100644
--- a/src/devices/power/drivers/power/power-test.cc
+++ b/src/devices/power/drivers/power/power-test.cc
@@ -134,7 +134,7 @@
zx::result parent_power_client = parent_power_.SyncCall(&FakePower::BindServer);
ASSERT_OK(parent_power_client);
- dut_ = std::make_unique<PowerDomain>(fake_parent_.get(), 0, power_impl_->GetClient(),
+ dut_ = std::make_unique<PowerDevice>(fake_parent_.get(), 0, power_impl_->GetClient(),
std::move(parent_power_client.value()), 10, 1000, false);
}
@@ -157,7 +157,7 @@
std::shared_ptr<MockDevice> fake_parent_ = MockDevice::FakeRootParent();
fdf::UnownedSynchronizedDispatcher env_dispatcher_ =
fdf_testing::DriverRuntime::GetInstance()->StartBackgroundDispatcher();
- std::unique_ptr<PowerDomain> dut_;
+ std::unique_ptr<PowerDevice> dut_;
async_patterns::TestDispatcherBound<FakePower> parent_power_{env_dispatcher_->async_dispatcher(),
std::in_place};
std::unique_ptr<FakePowerImpl> power_impl_;
@@ -341,7 +341,7 @@
ASSERT_OK(parent_power_client);
auto dut_fixed =
- std::make_unique<PowerDomain>(fake_parent_.get(), 1, power_impl_->GetClient(),
+ std::make_unique<PowerDevice>(fake_parent_.get(), 1, power_impl_->GetClient(),
std::move(parent_power_client.value()), 1000, 1000, true);
auto endpoints = fidl::Endpoints<fuchsia_hardware_power::Device>::Create();
dut_fixed->GetHandler()(std::move(endpoints.server));
diff --git a/src/devices/power/drivers/power/power.bind b/src/devices/power/drivers/power/power.bind
index b9ee07d..192f935 100644
--- a/src/devices/power/drivers/power/power.bind
+++ b/src/devices/power/drivers/power/power.bind
@@ -2,6 +2,18 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+composite power_domain_arm_core;
+
+using fuchsia.platform;
using fuchsia.power;
-fuchsia.BIND_PROTOCOL == fuchsia.power.BIND_PROTOCOL.IMPL;
+primary node "power-impl" {
+ fuchsia.BIND_PROTOCOL == fuchsia.power.BIND_PROTOCOL.IMPL;
+}
+
+node "pdev" {
+ fuchsia.BIND_PROTOCOL == fuchsia.platform.BIND_PROTOCOL.DEVICE;
+ fuchsia.BIND_PLATFORM_DEV_VID == fuchsia.platform.BIND_PLATFORM_DEV_VID.GENERIC;
+ fuchsia.BIND_PLATFORM_DEV_PID == fuchsia.platform.BIND_PLATFORM_DEV_PID.GENERIC;
+ fuchsia.BIND_PLATFORM_DEV_DID == fuchsia.platform.BIND_PLATFORM_DEV_DID.POWER_CORE;
+}
\ No newline at end of file
diff --git a/src/devices/power/drivers/power/power.cc b/src/devices/power/drivers/power/power.cc
index 60d8ccf..f17e3fb 100644
--- a/src/devices/power/drivers/power/power.cc
+++ b/src/devices/power/drivers/power/power.cc
@@ -10,7 +10,6 @@
#include <lib/ddk/device.h>
#include <lib/ddk/metadata.h>
#include <lib/ddk/platform-defs.h>
-#include <lib/fit/defer.h>
#include <zircon/assert.h>
#include <zircon/errors.h>
#include <zircon/types.h>
@@ -18,6 +17,7 @@
#include <memory>
#include <bind/fuchsia/power/cpp/bind.h>
+#include <ddk/metadata/power.h>
#include <fbl/alloc_checker.h>
#include <fbl/auto_lock.h>
@@ -28,7 +28,7 @@
namespace power {
-void PowerDomainFragmentChild::RegisterPowerDomain(RegisterPowerDomainRequestView request,
+void PowerDeviceFragmentChild::RegisterPowerDomain(RegisterPowerDomainRequestView request,
RegisterPowerDomainCompleter::Sync& completer) {
zx_status_t status = power_device_->RegisterPowerDomain(
fragment_device_id_, request->min_needed_voltage, request->max_supported_voltage);
@@ -39,7 +39,7 @@
}
}
-void PowerDomainFragmentChild::UnregisterPowerDomain(
+void PowerDeviceFragmentChild::UnregisterPowerDomain(
UnregisterPowerDomainCompleter::Sync& completer) {
zx_status_t status = power_device_->UnregisterPowerDomain(fragment_device_id_);
if (status == ZX_OK) {
@@ -49,7 +49,7 @@
}
}
-void PowerDomainFragmentChild::GetPowerDomainStatus(
+void PowerDeviceFragmentChild::GetPowerDomainStatus(
GetPowerDomainStatusCompleter::Sync& completer) {
power_domain_status_t out_status;
zx_status_t status = power_device_->GetPowerDomainStatus(fragment_device_id_, &out_status);
@@ -60,7 +60,7 @@
}
}
-void PowerDomainFragmentChild::GetSupportedVoltageRange(
+void PowerDeviceFragmentChild::GetSupportedVoltageRange(
GetSupportedVoltageRangeCompleter::Sync& completer) {
uint32_t out_min, out_max;
zx_status_t status =
@@ -72,7 +72,7 @@
}
}
-void PowerDomainFragmentChild::RequestVoltage(RequestVoltageRequestView request,
+void PowerDeviceFragmentChild::RequestVoltage(RequestVoltageRequestView request,
RequestVoltageCompleter::Sync& completer) {
uint32_t out_actual_voltage;
zx_status_t status =
@@ -84,7 +84,7 @@
}
}
-void PowerDomainFragmentChild::GetCurrentVoltage(GetCurrentVoltageRequestView request,
+void PowerDeviceFragmentChild::GetCurrentVoltage(GetCurrentVoltageRequestView request,
GetCurrentVoltageCompleter::Sync& completer) {
uint32_t out_current_voltage;
zx_status_t status =
@@ -96,7 +96,7 @@
}
}
-void PowerDomainFragmentChild::WritePmicCtrlReg(WritePmicCtrlRegRequestView request,
+void PowerDeviceFragmentChild::WritePmicCtrlReg(WritePmicCtrlRegRequestView request,
WritePmicCtrlRegCompleter::Sync& completer) {
zx_status_t status =
power_device_->WritePmicCtrlReg(fragment_device_id_, request->reg_addr, request->value);
@@ -107,7 +107,7 @@
}
}
-void PowerDomainFragmentChild::ReadPmicCtrlReg(ReadPmicCtrlRegRequestView request,
+void PowerDeviceFragmentChild::ReadPmicCtrlReg(ReadPmicCtrlRegRequestView request,
ReadPmicCtrlRegCompleter::Sync& completer) {
uint32_t out_value;
zx_status_t status =
@@ -119,7 +119,7 @@
}
}
-PowerDomainFragmentChild* PowerDomain::GetFragmentChildLocked(uint64_t fragment_device_id) {
+PowerDeviceFragmentChild* PowerDevice::GetFragmentChildLocked(uint64_t fragment_device_id) {
for (auto& child : children_) {
if (child->fragment_device_id() == fragment_device_id) {
return child.get();
@@ -128,12 +128,12 @@
return nullptr;
}
-uint32_t PowerDomain::GetDependentCount() {
+uint32_t PowerDevice::GetDependentCount() {
fbl::AutoLock al(&power_device_lock_);
return GetDependentCountLocked();
}
-uint32_t PowerDomain::GetDependentCountLocked() {
+uint32_t PowerDevice::GetDependentCountLocked() {
uint32_t count = 0;
for (const auto& child : children_) {
if (child->registered()) {
@@ -143,7 +143,7 @@
return count;
}
-zx_status_t PowerDomain::GetSuitableVoltageLocked(uint32_t voltage, uint32_t* suitable_voltage) {
+zx_status_t PowerDevice::GetSuitableVoltageLocked(uint32_t voltage, uint32_t* suitable_voltage) {
uint32_t min_voltage_all_children = min_voltage_uV_;
uint32_t max_voltage_all_children = max_voltage_uV_;
for (auto& child : children_) {
@@ -167,12 +167,12 @@
return ZX_OK;
}
-zx_status_t PowerDomain::RegisterPowerDomain(uint64_t fragment_device_id,
+zx_status_t PowerDevice::RegisterPowerDomain(uint64_t fragment_device_id,
uint32_t min_needed_voltage_uV,
uint32_t max_supported_voltage_uV) {
zx_status_t status = ZX_OK;
fbl::AutoLock al(&power_device_lock_);
- PowerDomainFragmentChild* child = GetFragmentChildLocked(fragment_device_id);
+ PowerDeviceFragmentChild* child = GetFragmentChildLocked(fragment_device_id);
ZX_DEBUG_ASSERT(child != nullptr);
child->set_min_needed_voltage_uV(std::max(min_needed_voltage_uV, min_voltage_uV_));
child->set_max_supported_voltage_uV(std::min(max_supported_voltage_uV, max_voltage_uV_));
@@ -207,10 +207,10 @@
return ZX_OK;
}
-zx_status_t PowerDomain::UnregisterPowerDomain(uint64_t fragment_device_id) {
+zx_status_t PowerDevice::UnregisterPowerDomain(uint64_t fragment_device_id) {
zx_status_t status = ZX_OK;
fbl::AutoLock al(&power_device_lock_);
- PowerDomainFragmentChild* child = GetFragmentChildLocked(fragment_device_id);
+ PowerDeviceFragmentChild* child = GetFragmentChildLocked(fragment_device_id);
ZX_DEBUG_ASSERT(child != nullptr);
if (!child->registered()) {
return ZX_ERR_UNAVAILABLE;
@@ -241,12 +241,12 @@
return ZX_OK;
}
-zx_status_t PowerDomain::GetPowerDomainStatus(uint64_t fragment_device_id,
+zx_status_t PowerDevice::GetPowerDomainStatus(uint64_t fragment_device_id,
power_domain_status_t* out_status) {
return power_impl_.GetPowerDomainStatus(index_, out_status);
}
-zx_status_t PowerDomain::GetSupportedVoltageRange(uint64_t fragment_device_id,
+zx_status_t PowerDevice::GetSupportedVoltageRange(uint64_t fragment_device_id,
uint32_t* min_voltage, uint32_t* max_voltage) {
if (fixed_) {
return ZX_ERR_NOT_SUPPORTED;
@@ -256,7 +256,7 @@
return ZX_OK;
}
-zx_status_t PowerDomain::RequestVoltage(uint64_t fragment_device_id, uint32_t voltage,
+zx_status_t PowerDevice::RequestVoltage(uint64_t fragment_device_id, uint32_t voltage,
uint32_t* actual_voltage) {
fbl::AutoLock al(&power_device_lock_);
if (fixed_) {
@@ -266,7 +266,7 @@
zxlogf(ERROR, "The voltage is not within supported voltage range of the power domain");
return ZX_ERR_INVALID_ARGS;
}
- PowerDomainFragmentChild* child = GetFragmentChildLocked(fragment_device_id);
+ PowerDeviceFragmentChild* child = GetFragmentChildLocked(fragment_device_id);
ZX_DEBUG_ASSERT(child != nullptr);
if (!child->registered()) {
zxlogf(ERROR, "The device is not registered for the power domain");
@@ -283,39 +283,39 @@
return power_impl_.RequestVoltage(index_, suitable_voltage, actual_voltage);
}
-zx_status_t PowerDomain::GetCurrentVoltage(uint64_t fragment_device_id, uint32_t index,
+zx_status_t PowerDevice::GetCurrentVoltage(uint64_t fragment_device_id, uint32_t index,
uint32_t* current_voltage) {
fbl::AutoLock al(&power_device_lock_);
return power_impl_.GetCurrentVoltage(index_, current_voltage);
}
-zx_status_t PowerDomain::WritePmicCtrlReg(uint64_t fragment_device_id, uint32_t reg_addr,
+zx_status_t PowerDevice::WritePmicCtrlReg(uint64_t fragment_device_id, uint32_t reg_addr,
uint32_t value) {
fbl::AutoLock al(&power_device_lock_);
return power_impl_.WritePmicCtrlReg(index_, reg_addr, value);
}
-zx_status_t PowerDomain::ReadPmicCtrlReg(uint64_t fragment_device_id, uint32_t reg_addr,
+zx_status_t PowerDevice::ReadPmicCtrlReg(uint64_t fragment_device_id, uint32_t reg_addr,
uint32_t* out_value) {
fbl::AutoLock al(&power_device_lock_);
return power_impl_.ReadPmicCtrlReg(index_, reg_addr, out_value);
}
-void PowerDomain::DdkRelease() { delete this; }
+void PowerDevice::DdkRelease() { delete this; }
-fit::function<void(fidl::ServerEnd<fuchsia_hardware_power::Device>)> PowerDomain::GetHandler() {
+fit::function<void(fidl::ServerEnd<fuchsia_hardware_power::Device>)> PowerDevice::GetHandler() {
return [this](fidl::ServerEnd<fuchsia_hardware_power::Device> server_end) {
fbl::AutoLock al(&power_device_lock_);
fbl::AllocChecker ac;
uint64_t id = 0;
GetUniqueId(&id);
- std::unique_ptr<PowerDomainFragmentChild> child(new (&ac) PowerDomainFragmentChild(id, this));
+ std::unique_ptr<PowerDeviceFragmentChild> child(new (&ac) PowerDeviceFragmentChild(id, this));
if (!ac.check()) {
zxlogf(ERROR, "Failed to allocate PowerDeviceFragmentChild.");
return;
}
children_.push_back(std::move(child));
- PowerDomainFragmentChild* child_ptr = children_.back().get();
+ PowerDeviceFragmentChild* child_ptr = children_.back().get();
auto close_handler = [this, id](fidl::UnbindInfo info) {
fbl::AutoLock al(&power_device_lock_);
for (auto iter = children_.begin(); iter != children_.end(); iter++) {
@@ -331,7 +331,7 @@
};
}
-zx_status_t PowerDomain::Serve(fidl::ServerEnd<fuchsia_io::Directory> server_end) {
+zx_status_t PowerDevice::Serve(fidl::ServerEnd<fuchsia_io::Directory> server_end) {
fuchsia_hardware_power::Service::InstanceHandler handler({
.device = GetHandler(),
});
@@ -351,12 +351,21 @@
return ZX_OK;
}
-zx_status_t PowerDomain::Create(void* ctx, zx_device_t* parent,
- const ddk::PowerImplProtocolClient& power_impl,
- fuchsia_hardware_power::wire::Domain domain_info) {
- auto index = domain_info.id();
+zx_status_t PowerDevice::Create(void* ctx, zx_device_t* parent) {
+ auto power_domain = ddk::GetMetadata<power_domain_t>(parent, DEVICE_METADATA_POWER_DOMAINS);
+ if (!power_domain.is_ok()) {
+ zxlogf(ERROR, "Failed to get metadata: %s", power_domain.status_string());
+ return power_domain.error_value();
+ }
+
+ auto index = power_domain->index;
char name[20];
snprintf(name, sizeof(name), "power-%u", index);
+ ddk::PowerImplProtocolClient power_impl(parent, "power-impl");
+ if (!power_impl.is_valid()) {
+ zxlogf(ERROR, "%s: ZX_PROTOCOL_POWER_IMPL not available", __func__);
+ return ZX_ERR_NO_RESOURCES;
+ }
// This is optional.
fidl::ClientEnd<fuchsia_hardware_power::Device> parent_power;
@@ -378,7 +387,7 @@
fixed = true;
}
fbl::AllocChecker ac;
- std::unique_ptr<PowerDomain> dev(new (&ac) PowerDomain(
+ std::unique_ptr<PowerDevice> dev(new (&ac) PowerDevice(
parent, index, power_impl, std::move(parent_power), min_voltage, max_voltage, fixed));
if (!ac.check()) {
return ZX_ERR_NO_MEMORY;
@@ -416,45 +425,10 @@
return ZX_OK;
}
-zx_status_t Power::Create(void* ctx, zx_device_t* parent) {
- auto power_domains = ddk::GetEncodedMetadata<fuchsia_hardware_power::wire::DomainMetadata>(
- parent, DEVICE_METADATA_POWER_DOMAINS);
- if (!power_domains.is_ok()) {
- zxlogf(ERROR, "Failed to get metadata: %s", power_domains.status_string());
- return power_domains.error_value();
- }
-
- std::unique_ptr<Power> root = std::make_unique<Power>(parent);
- auto status = root->DdkAdd(ddk::DeviceAddArgs("power-core").set_flags(DEVICE_ADD_NON_BINDABLE));
- if (status != ZX_OK) {
- zxlogf(ERROR, "Failed to add power core: %s", zx_status_get_string(status));
- return status;
- }
-
- auto root_release = fit::defer([&root]() { [[maybe_unused]] auto ptr = root.release(); });
-
- ddk::PowerImplProtocolClient power_impl(parent);
- if (!power_impl.is_valid()) {
- zxlogf(ERROR, "%s: ZX_PROTOCOL_POWER_IMPL not available", __func__);
- return ZX_ERR_NO_RESOURCES;
- }
-
- for (const auto& domain : power_domains->domains()) {
- status = PowerDomain::Create(ctx, root->zxdev(), power_impl, domain);
- if (status != ZX_OK) {
- zxlogf(ERROR, "Failed to add power domain - %d: %s", domain.id(),
- zx_status_get_string(status));
- return status;
- }
- }
-
- return ZX_OK;
-}
-
static constexpr zx_driver_ops_t driver_ops = []() {
zx_driver_ops_t ops = {};
ops.version = DRIVER_OPS_VERSION;
- ops.bind = Power::Create;
+ ops.bind = PowerDevice::Create;
return ops;
}();
diff --git a/src/devices/power/drivers/power/power.h b/src/devices/power/drivers/power/power.h
index fe2045b..d69d891 100644
--- a/src/devices/power/drivers/power/power.h
+++ b/src/devices/power/drivers/power/power.h
@@ -19,36 +19,20 @@
#include <fbl/mutex.h>
namespace power {
-class PowerDomainFragmentChild;
+class PowerDeviceFragmentChild;
+class PowerDevice;
+using PowerDeviceType = ddk::Device<PowerDevice>;
-class Power;
-using PowerType = ddk::Device<Power>;
-
-// The core power device that binds to the power implementation and creates necessary power domain
-// devices.
-class Power : public PowerType {
+// Each power domain is modelled to be a power device and the power device class talks to
+// a driver that implements ZX_PROTOCOL_POWER_IMPL, passing in the index of this power domain.
+// For each dependent composite device of a PowerDevice(power domain), a PowerDeviceFragmentChild
+// is created.
+class PowerDevice : public PowerDeviceType {
public:
- explicit Power(zx_device_t* parent) : PowerType(parent) {}
-
- static zx_status_t Create(void* ctx, zx_device_t* parent);
-
- void DdkRelease() {}
-
- private:
-};
-
-class PowerDomain;
-using PowerDomainType = ddk::Device<PowerDomain>;
-
-// Each power domain is modelled to be a device and this device class talks to a driver that
-// implements ZX_PROTOCOL_POWER_IMPL, passing in the index of this power domain. For each dependent
-// composite device of a PowerDomain, a PowerDomainFragmentChild is created.
-class PowerDomain : public PowerDomainType {
- public:
- PowerDomain(zx_device_t* parent, uint32_t index, const ddk::PowerImplProtocolClient& power_impl,
+ PowerDevice(zx_device_t* parent, uint32_t index, const ddk::PowerImplProtocolClient& power_impl,
fidl::ClientEnd<fuchsia_hardware_power::Device> parent_power, uint32_t min_voltage,
uint32_t max_voltage, bool fixed)
- : PowerDomainType(parent),
+ : PowerDeviceType(parent),
index_(index),
power_impl_(power_impl),
parent_power_(std::move(parent_power)),
@@ -56,9 +40,7 @@
max_voltage_uV_(max_voltage),
fixed_(fixed) {}
- static zx_status_t Create(void* ctx, zx_device_t* parent,
- const ddk::PowerImplProtocolClient& power_impl,
- fuchsia_hardware_power::wire::Domain domain_info);
+ static zx_status_t Create(void* ctx, zx_device_t* parent);
void DdkRelease();
zx_status_t RegisterPowerDomain(uint64_t fragment_device_id, uint32_t min_needed_voltage_uV,
@@ -79,7 +61,7 @@
zx_status_t Serve(fidl::ServerEnd<fuchsia_io::Directory> server_end);
private:
- PowerDomainFragmentChild* GetFragmentChildLocked(uint64_t fragment_device_id)
+ PowerDeviceFragmentChild* GetFragmentChildLocked(uint64_t fragment_device_id)
__TA_REQUIRES(power_device_lock_);
zx_status_t GetSuitableVoltageLocked(uint32_t voltage, uint32_t* suitable_voltage)
__TA_REQUIRES(power_device_lock_);
@@ -91,7 +73,7 @@
fidl::ClientEnd<fuchsia_hardware_power::Device> parent_power_;
fbl::Mutex power_device_lock_;
- std::vector<std::unique_ptr<PowerDomainFragmentChild>> children_ __TA_GUARDED(power_device_lock_);
+ std::vector<std::unique_ptr<PowerDeviceFragmentChild>> children_ __TA_GUARDED(power_device_lock_);
// Min supported voltage of this domain
uint32_t min_voltage_uV_;
// Max supported voltage of this domain
@@ -108,9 +90,9 @@
// to the composite device. All the power protocol ops made by the composite device first
// arrive on this calss and are forwarded to the PowerDevice with the corresponding composite
// device context(fragment_device_id).
-class PowerDomainFragmentChild : public fidl::WireServer<fuchsia_hardware_power::Device> {
+class PowerDeviceFragmentChild : public fidl::WireServer<fuchsia_hardware_power::Device> {
public:
- explicit PowerDomainFragmentChild(uint64_t fragment_device_id, PowerDomain* parent)
+ explicit PowerDeviceFragmentChild(uint64_t fragment_device_id, PowerDevice* parent)
: fragment_device_id_(fragment_device_id), power_device_(parent) {}
void RegisterPowerDomain(RegisterPowerDomainRequestView request,
@@ -137,7 +119,7 @@
private:
uint64_t fragment_device_id_;
- PowerDomain* power_device_;
+ PowerDevice* power_device_;
uint32_t min_needed_voltage_uV_ = 0;
uint32_t max_supported_voltage_uV_ = 0;
bool registered_ = false;
diff --git a/src/lib/ddk/BUILD.gn b/src/lib/ddk/BUILD.gn
index a4a57cd..fce759f 100644
--- a/src/lib/ddk/BUILD.gn
+++ b/src/lib/ddk/BUILD.gn
@@ -415,6 +415,7 @@
"ddk/metadata/gpio.h",
"ddk/metadata/lights.h",
"ddk/metadata/nand.h",
+ "ddk/metadata/power.h",
"ddk/metadata/test.h",
]
diff --git a/src/lib/ddk/include/ddk/metadata/power.h b/src/lib/ddk/include/ddk/metadata/power.h
new file mode 100644
index 0000000..dfd321c
--- /dev/null
+++ b/src/lib/ddk/include/ddk/metadata/power.h
@@ -0,0 +1,12 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SRC_LIB_DDK_INCLUDE_DDK_METADATA_POWER_H_
+#define SRC_LIB_DDK_INCLUDE_DDK_METADATA_POWER_H_
+
+typedef struct {
+ uint32_t index;
+} power_domain_t;
+
+#endif // SRC_LIB_DDK_INCLUDE_DDK_METADATA_POWER_H_
diff --git a/src/lib/ddk/include/lib/ddk/metadata.h b/src/lib/ddk/include/lib/ddk/metadata.h
index f94a3b0..debb630 100644
--- a/src/lib/ddk/include/lib/ddk/metadata.h
+++ b/src/lib/ddk/include/lib/ddk/metadata.h
@@ -98,7 +98,7 @@
// type: FIDL fuchsia.hardware.clockimpl/InitMetadata
#define DEVICE_METADATA_CLOCK_INIT 0x494B4C43 // CLKI
-// type: FIDL fuchsia.hardware.power/DomainMetadata
+// type: array of power_domain_t
#define DEVICE_METADATA_POWER_DOMAINS 0x52574F50 // POWR
// type: clock_id_t
diff --git a/zircon/system/utest/device-enumeration/boards/astro.cc b/zircon/system/utest/device-enumeration/boards/astro.cc
index ada0a516..1e9a435 100644
--- a/zircon/system/utest/device-enumeration/boards/astro.cc
+++ b/zircon/system/utest/device-enumeration/boards/astro.cc
@@ -53,7 +53,7 @@
// CPU Device.
"sys/platform/03:03:6",
"class/cpu-ctrl/000",
- "sys/platform/03:03:26/aml-power-impl-composite/power-impl/power-core/power-0/aml_cpu/s905d2-arm-a53",
+ "sys/platform/03:03:26/aml-power-impl-composite/power-impl/pd_armcore/power-0/aml_cpu/s905d2-arm-a53",
// LED.
"sys/platform/05:00:1c/aml_light",
// RAM (DDR) control.
@@ -61,8 +61,8 @@
// Power Device.
"sys/platform/03:03:26/aml-power-impl-composite",
- "sys/platform/03:03:26/aml-power-impl-composite/power-impl/power-core",
- "sys/platform/03:03:26/aml-power-impl-composite/power-impl/power-core/power-0",
+ "sys/platform/03:03:26/aml-power-impl-composite/power-impl/pd_armcore",
+ "sys/platform/03:03:26/aml-power-impl-composite/power-impl/pd_armcore/power-0",
// Thermal
"sys/platform/05:03:a/thermal",
diff --git a/zircon/system/utest/device-enumeration/boards/vim3.cc b/zircon/system/utest/device-enumeration/boards/vim3.cc
index 89f17ef..3401f3d 100644
--- a/zircon/system/utest/device-enumeration/boards/vim3.cc
+++ b/zircon/system/utest/device-enumeration/boards/vim3.cc
@@ -45,8 +45,8 @@
"sys/platform/05:06:1d/aml-pwm-device/pwm-0/vreg-0/pwm-0-regulator",
"sys/platform/05:06:1d/aml-pwm-device/pwm-9/vreg-9/pwm-9-regulator",
"sys/platform/05:06:26/aml-power-impl-composite",
- "sys/platform/05:06:26/aml-power-impl-composite/power-impl/power-core/power-0",
- "sys/platform/05:06:26/aml-power-impl-composite/power-impl/power-core/power-1",
+ "sys/platform/05:06:26/aml-power-impl-composite/power-impl/pd_big_core",
+ "sys/platform/05:06:26/aml-power-impl-composite/power-impl/pd_little_core",
"sys/platform/05:06:26", // power
// EMMC
@@ -71,8 +71,8 @@
// CPU devices.
"sys/platform/05:06:1e",
- "sys/platform/05:06:26/aml-power-impl-composite/power-impl/power-core/power-0/aml_cpu/a311d-arm-a73",
- "sys/platform/05:06:26/aml-power-impl-composite/power-impl/power-core/power-0/aml_cpu/a311d-arm-a53",
+ "sys/platform/05:06:26/aml-power-impl-composite/power-impl/pd_little_core/power-0/aml_cpu/a311d-arm-a73",
+ "sys/platform/05:06:26/aml-power-impl-composite/power-impl/pd_little_core/power-0/aml_cpu/a311d-arm-a53",
"sys/platform/05:06:1/aml-gpio/gpio/gpio-93/fusb302",
