[devcoordinator] REVERT: Fix buggy composite device behavior
This reverts commit 75b35ceab71c50f43b11a9ee9bd4e034a9fc7447.
Reason for revert: This CL is a suspect for the currently red fuchsia-roller; it fails the /boot/test/ddk/platform-bus-test test.
Original change's description:
> [devcoordinator] Fix buggy composite device behavior
>
> If a component was a device with MUST_ISOLATE set on it, we would try to
> bind to the non-proxied side of it, which doesn't speak the device
> protocols.
>
> Change-Id: I64eaccec5d51997f4374d0277b040a952f681667
TBR=teisenbe@google.com,abarth@google.com,jocelyndang@google.com
Change-Id: Id9d48682336ea648599cfead60444c5074c4ae1e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
diff --git a/zircon/system/core/devmgr/devcoordinator/composite-device.cpp b/zircon/system/core/devmgr/devcoordinator/composite-device.cpp
index ba586e8..a9aad30 100644
--- a/zircon/system/core/devmgr/devcoordinator/composite-device.cpp
+++ b/zircon/system/core/devmgr/devcoordinator/composite-device.cpp
@@ -127,15 +127,9 @@
// Create all of the proxies for the component devices, in the same process
for (auto& component : bound_) {
const auto& component_dev = component.component_device();
- auto bound_dev = component.bound_device();
+ const auto& bound_dev = component.bound_device();
coordinator = component_dev->coordinator;
- // If the device we're bound to is proxied, we care about its proxy
- // rather than it, since that's the side that we communicate with.
- if (bound_dev->proxy()) {
- bound_dev = bound_dev->proxy();
- }
-
// Check if we need to use the proxy. If not, share a reference straight
// to the target device rather than the instance of the component device
// that bound to it.
diff --git a/zircon/system/core/devmgr/devcoordinator/coordinator.cpp b/zircon/system/core/devmgr/devcoordinator/coordinator.cpp
index dd31246..ef4b167 100644
--- a/zircon/system/core/devmgr/devcoordinator/coordinator.cpp
+++ b/zircon/system/core/devmgr/devcoordinator/coordinator.cpp
@@ -589,14 +589,10 @@
}
devices_.push_back(dev);
- // Note that |dev->parent()| may not match |parent| here, so we should always
- // use |dev->parent()|. This case can happen if |parent| refers to a device
- // proxy.
-
// If we're creating a device that's using the component driver, inform the
// component.
if (component_driver_ != nullptr && dev->libname() == component_driver_->libname) {
- CompositeDeviceComponent* component = dev->parent()->component();
+ CompositeDeviceComponent* component = parent->component();
component->set_component_device(dev);
status = component->composite()->TryAssemble();
if (status != ZX_OK && status != ZX_ERR_SHOULD_WAIT) {
diff --git a/zircon/system/dev/board/test/BUILD.gn b/zircon/system/dev/board/test/BUILD.gn
index eba0ce3..3fa7c5b7 100644
--- a/zircon/system/dev/board/test/BUILD.gn
+++ b/zircon/system/dev/board/test/BUILD.gn
@@ -7,7 +7,6 @@
simple_drivers = [
"child-1",
"child-2",
- "child-2.proxy",
"child-3",
"parent",
"composite",
diff --git a/zircon/system/dev/board/test/test-board.cpp b/zircon/system/dev/board/test/test-board.cpp
index 19fe9d6..b628f04 100644
--- a/zircon/system/dev/board/test/test-board.cpp
+++ b/zircon/system/dev/board/test/test-board.cpp
@@ -114,17 +114,6 @@
BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_POWER),
BI_MATCH_IF(EQ, BIND_POWER_DOMAIN, 3),
};
- const zx_bind_inst_t child2_match[] = {
- BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_PDEV),
- BI_ABORT_IF(NE, BIND_PLATFORM_DEV_VID, PDEV_VID_TEST),
- BI_ABORT_IF(NE, BIND_PLATFORM_DEV_PID, PDEV_PID_PBUS_TEST),
- BI_MATCH_IF(EQ, BIND_PLATFORM_DEV_DID, PDEV_DID_TEST_CHILD_2),
- };
- const zx_bind_inst_t child4_match[] = {
- BI_ABORT_IF(NE, BIND_PLATFORM_DEV_VID, PDEV_VID_TEST),
- BI_ABORT_IF(NE, BIND_PLATFORM_DEV_PID, PDEV_PID_PBUS_TEST),
- BI_MATCH_IF(EQ, BIND_PLATFORM_DEV_DID, PDEV_DID_TEST_CHILD_4),
- };
device_component_part_t gpio_component[] = {
{ fbl::count_of(root_match), root_match },
{ fbl::count_of(gpio_match), gpio_match },
@@ -141,18 +130,11 @@
{ fbl::count_of(root_match), root_match },
{ fbl::count_of(power_match), power_match },
};
- device_component_part_t child4_component[] = {
- { fbl::count_of(root_match), root_match },
- { fbl::count_of(child2_match), child2_match },
- { fbl::count_of(child4_match), child4_match },
- };
-
device_component_t composite[] = {
{ fbl::count_of(gpio_component), gpio_component },
{ fbl::count_of(clock_component), clock_component },
{ fbl::count_of(i2c_component), i2c_component },
{ fbl::count_of(power_component), power_component },
- { fbl::count_of(child4_component), child4_component },
};
const uint32_t test_metadata_value = 12345;
diff --git a/zircon/system/dev/board/test/test/child-1.c b/zircon/system/dev/board/test/test/child-1.c
index 0be1d1f..81d8ed9 100644
--- a/zircon/system/dev/board/test/test/child-1.c
+++ b/zircon/system/dev/board/test/test/child-1.c
@@ -72,7 +72,7 @@
device_add_args_t child_2_args = {
.version = DEVICE_ADD_ARGS_VERSION,
- .name = "child-2",
+ .name = "child-2-top",
.ctx = child_2,
.ops = &test_device_protocol,
.props = child_2_props,
diff --git a/zircon/system/dev/board/test/test/child-2.c b/zircon/system/dev/board/test/test/child-2.c
index 41b9eb2..e12d0bf 100644
--- a/zircon/system/dev/board/test/test/child-2.c
+++ b/zircon/system/dev/board/test/test/child-2.c
@@ -22,18 +22,8 @@
free(ctx);
}
-static zx_status_t test_rxrpc(void* ctx, zx_handle_t channel) {
- if (channel == ZX_HANDLE_INVALID) {
- return ZX_OK;
- }
- // This won't actually get called, since the other half doesn't send
- // messages at the moment
- __builtin_trap();
-};
-
static zx_protocol_device_t test_device_protocol = {
.version = DEVICE_OPS_VERSION,
- .rxrpc = test_rxrpc,
.release = test_release,
};
@@ -72,21 +62,11 @@
return ZX_ERR_NO_MEMORY;
}
- zx_device_prop_t child_props[] = {
- { BIND_PLATFORM_DEV_VID, 0, PDEV_VID_TEST},
- { BIND_PLATFORM_DEV_PID, 0, PDEV_PID_PBUS_TEST},
- { BIND_PLATFORM_DEV_DID, 0, PDEV_DID_TEST_CHILD_4 },
- };
-
device_add_args_t args = {
.version = DEVICE_ADD_ARGS_VERSION,
- .name = "child-4",
+ .name = "child-2",
.ctx = test,
.ops = &test_device_protocol,
- .props = child_props,
- .prop_count = countof(child_props),
- .flags = DEVICE_ADD_MUST_ISOLATE,
- .proxy_args = ",",
};
status = device_add(parent, &args, &test->zxdev);
diff --git a/zircon/system/dev/board/test/test/child-2.proxy.c b/zircon/system/dev/board/test/test/child-2.proxy.c
deleted file mode 100644
index dd7b4b3..0000000
--- a/zircon/system/dev/board/test/test/child-2.proxy.c
+++ /dev/null
@@ -1,86 +0,0 @@
-// Copyright 2018 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.
-
-#include <stdlib.h>
-#include <string.h>
-
-#include <ddk/binding.h>
-#include <ddk/debug.h>
-#include <ddk/device.h>
-#include <ddk/driver.h>
-#include <ddk/platform-defs.h>
-#include <ddk/protocol/gpio.h>
-#include <ddk/protocol/platform/device.h>
-
-#define DRIVER_NAME "test-child-4"
-
-typedef struct {
- zx_device_t* zxdev;
- zx_handle_t rpc_channel;
-} test_t;
-
-static void test_release(void* ctx) {
- test_t* dev = (test_t*)ctx;
- zx_handle_close(dev->rpc_channel);
- free(dev);
-}
-
-static zx_status_t test_get_protocol(void* ctx, uint32_t protocol_id, void* proto) {
- // Lie about supporting the CLOCK protocol. The composite device will just
- // check that we claimed to support it. Note the non-proxied device does
- // not claim to support this protocol, so if we see it, we must be talking
- // to the proxy.
- if (protocol_id == ZX_PROTOCOL_CLOCK) {
- // Zero out the proto ops in case something tries using it
- memset(proto, 0, sizeof(uintptr_t));
- return ZX_OK;
- }
- return ZX_ERR_NOT_SUPPORTED;
-}
-
-static zx_protocol_device_t test_device_protocol = {
- .version = DEVICE_OPS_VERSION,
- .get_protocol = test_get_protocol,
- .release = test_release,
-};
-static zx_status_t test_create(void* ctx, zx_device_t* parent, const char* name, const char* args,
- zx_handle_t rpc_channel) {
- zx_status_t status;
-
- zxlogf(INFO, "test_create: %s \n", DRIVER_NAME);
-
- test_t* test = calloc(1, sizeof(test_t));
- if (!test) {
- return ZX_ERR_NO_MEMORY;
- }
- test->rpc_channel = rpc_channel;
-
- device_add_args_t dev_args = {
- .version = DEVICE_ADD_ARGS_VERSION,
- .name = "child-4",
- .ctx = test,
- .ops = &test_device_protocol,
- };
-
- status = device_add(parent, &dev_args, &test->zxdev);
- if (status != ZX_OK) {
- zxlogf(ERROR, "%s: device_add failed: %d\n", DRIVER_NAME, status);
- free(test);
- return status;
- }
-
- return ZX_OK;
-}
-
-static zx_driver_ops_t test_driver_ops = {
- .version = DRIVER_OPS_VERSION,
- .create = test_create,
-};
-
-ZIRCON_DRIVER_BEGIN(test_bus, test_driver_ops, "zircon", "0.1", 4)
- BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_PDEV),
- BI_ABORT_IF(NE, BIND_PLATFORM_DEV_VID, PDEV_VID_TEST),
- BI_ABORT_IF(NE, BIND_PLATFORM_DEV_PID, PDEV_PID_PBUS_TEST),
- BI_MATCH_IF(EQ, BIND_PLATFORM_DEV_DID, PDEV_DID_TEST_CHILD_4),
-ZIRCON_DRIVER_END(test_bus)
diff --git a/zircon/system/dev/board/test/test/composite.c b/zircon/system/dev/board/test/test/composite.c
index b5629f7..e29b5ef 100644
--- a/zircon/system/dev/board/test/test/composite.c
+++ b/zircon/system/dev/board/test/test/composite.c
@@ -28,7 +28,6 @@
COMPONENT_CLOCK,
COMPONENT_I2C,
COMPONENT_POWER,
- COMPONENT_CHILD4,
COMPONENT_COUNT,
};
@@ -152,7 +151,6 @@
clock_protocol_t clock;
i2c_protocol_t i2c;
power_protocol_t power;
- clock_protocol_t child4;
status = device_get_protocol(components[COMPONENT_PDEV], ZX_PROTOCOL_PDEV, &pdev);
if (status != ZX_OK) {
@@ -179,11 +177,6 @@
zxlogf(ERROR, "%s: could not get protocol ZX_PROTOCOL_POWER\n", DRIVER_NAME);
return status;
}
- status = device_get_protocol(components[COMPONENT_CHILD4], ZX_PROTOCOL_CLOCK, &child4);
- if (status != ZX_OK) {
- zxlogf(ERROR, "%s: could not get protocol from child4\n", DRIVER_NAME);
- return status;
- }
if ((status = test_gpio(&gpio)) != ZX_OK) {
zxlogf(ERROR, "%s: test_gpio failed: %d\n", DRIVER_NAME, status);
diff --git a/zircon/system/ulib/ddk/include/ddk/platform-defs.h b/zircon/system/ulib/ddk/include/ddk/platform-defs.h
index d59907b37..3146d50 100644
--- a/zircon/system/ulib/ddk/include/ddk/platform-defs.h
+++ b/zircon/system/ulib/ddk/include/ddk/platform-defs.h
@@ -194,7 +194,6 @@
#define PDEV_DID_TEST_CLOCK 7
#define PDEV_DID_TEST_I2C 8
#define PDEV_DID_TEST_POWER 9
-#define PDEV_DID_TEST_CHILD_4 10
// ARM
#define PDEV_VID_ARM 18
diff --git a/zircon/system/utest/platform-bus/main.cpp b/zircon/system/utest/platform-bus/main.cpp
index f7b30db..9e5e4d9 100644
--- a/zircon/system/utest/platform-bus/main.cpp
+++ b/zircon/system/utest/platform-bus/main.cpp
@@ -64,55 +64,55 @@
fbl::unique_fd fd;
ASSERT_EQ(RecursiveWaitForFile(devmgr.devfs_root(), "sys/platform",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(), "sys/platform/test-board",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(), "sys/platform/11:01:1",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(), "sys/platform/11:01:1/child-1",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
- EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(), "sys/platform/11:01:1/child-1/child-2",
- zx::time::infinite(), &fd),
+ EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(), "sys/platform/11:01:1/child-1/child-2-top",
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(),
- "sys/platform/11:01:1/child-1/child-2/child-4",
- zx::time::infinite(), &fd),
+ "sys/platform/11:01:1/child-1/child-2-top/child-2",
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(), "sys/platform/11:01:1/child-1/child-3-top",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(),
"sys/platform/11:01:1/child-1/child-3-top/child-3",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(),
"sys/platform/11:01:5/test-gpio/gpio-3/component",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(),
"sys/platform/11:01:7/test-clock/clock-1/component",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(),
"sys/platform/11:01:8/test-i2c/i2c/i2c-1-5/component",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(),
"sys/platform/11:01:6/component",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
EXPECT_EQ(RecursiveWaitForFile(devmgr.devfs_root(),
"composite-dev/composite",
- zx::time::infinite(), &fd),
+ zx::deadline_after(zx::sec(5)), &fd),
ZX_OK);
const int dirfd = devmgr.devfs_root().get();
@@ -120,9 +120,9 @@
EXPECT_EQ(fstatat(dirfd, "sys/platform/test-board", &st, 0), 0);
EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:1", &st, 0), 0);
EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:1/child-1", &st, 0), 0);
- EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:1/child-1/child-2", &st, 0), 0);
+ EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:1/child-1/child-2-top", &st, 0), 0);
EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:1/child-1/child-3-top", &st, 0), 0);
- EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:1/child-1/child-2/child-4", &st, 0), 0);
+ EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:1/child-1/child-2-top/child-2", &st, 0), 0);
EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:1/child-1/child-3-top/child-3", &st, 0), 0);
EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:5/test-gpio/gpio-3/component", &st, 0), 0);
EXPECT_EQ(fstatat(dirfd, "sys/platform/11:01:7/test-clock/clock-1/component", &st, 0), 0);
