[ddk][camera] Use new protocol methods to get fragments
Preference towards lookup via name string is given, but in some cases,
the relationship is not static and we must continue to rely on indicies.
Change-Id: Idd41e5a476842778b5b19e11aa36e1d7e7a53835
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/438675
Reviewed-by: Abdulla Kamar <abdulla@google.com>
Commit-Queue: Suraj Malhotra <surajmalhotra@google.com>
Testability-Review: Abdulla Kamar <abdulla@google.com>
diff --git a/src/camera/drivers/controller/controller_device.cc b/src/camera/drivers/controller/controller_device.cc
index c80717f..21a7f43 100644
--- a/src/camera/drivers/controller/controller_device.cc
+++ b/src/camera/drivers/controller/controller_device.cc
@@ -18,16 +18,6 @@
constexpr auto kTag = "camera_controller";
-namespace {
-enum {
- FRAGMENT_ISP,
- FRAGMENT_GDC,
- FRAGMENT_GE2D,
- FRAGMENT_SYSMEM,
- FRAGMENT_COUNT,
-};
-} // namespace
-
void ControllerDevice::DdkUnbind(ddk::UnbindTxn txn) {
ShutDown();
txn.Reply();
@@ -99,33 +89,25 @@
return ZX_ERR_NOT_SUPPORTED;
}
- zx_device_t* fragments[FRAGMENT_COUNT];
- size_t actual;
- composite.GetFragments(fragments, FRAGMENT_COUNT, &actual);
- if (actual != FRAGMENT_COUNT) {
- zxlogf(ERROR, "%s: Could not get fragments", __func__);
- return ZX_ERR_NOT_SUPPORTED;
- }
-
- ddk::GdcProtocolClient gdc(fragments[FRAGMENT_GDC]);
+ ddk::GdcProtocolClient gdc(composite, "gdc");
if (!gdc.is_valid()) {
zxlogf(ERROR, "%s: ZX_PROTOCOL_GDC not available", __func__);
return ZX_ERR_NO_RESOURCES;
}
- ddk::Ge2dProtocolClient ge2d(fragments[FRAGMENT_GE2D]);
+ ddk::Ge2dProtocolClient ge2d(composite, "ge2d");
if (!ge2d.is_valid()) {
zxlogf(ERROR, "%s: ZX_PROTOCOL_GE2D not available", __func__);
return ZX_ERR_NO_RESOURCES;
}
- ddk::IspProtocolClient isp(fragments[FRAGMENT_ISP]);
+ ddk::IspProtocolClient isp(composite, "isp");
if (!isp.is_valid()) {
zxlogf(ERROR, "%s: ZX_PROTOCOL_ISP not available", __func__);
return ZX_ERR_NO_RESOURCES;
}
- ddk::SysmemProtocolClient sysmem(fragments[FRAGMENT_SYSMEM]);
+ ddk::SysmemProtocolClient sysmem(composite, "sysmem");
if (!sysmem.is_valid()) {
zxlogf(ERROR, "%s: ZX_PROTOCOL_SYSMEM not available", __func__);
return ZX_ERR_NO_RESOURCES;
@@ -138,9 +120,7 @@
return status;
}
- auto controller = std::make_unique<ControllerDevice>(
- parent, fragments[FRAGMENT_ISP], fragments[FRAGMENT_GDC], fragments[FRAGMENT_GE2D],
- fragments[FRAGMENT_SYSMEM], std::move(event));
+ auto controller = std::make_unique<ControllerDevice>(parent, composite, std::move(event));
status = controller->StartThread();
if (status != ZX_OK) {
diff --git a/src/camera/drivers/controller/controller_device.h b/src/camera/drivers/controller/controller_device.h
index c8148b9..21518f1 100644
--- a/src/camera/drivers/controller/controller_device.h
+++ b/src/camera/drivers/controller/controller_device.h
@@ -41,15 +41,15 @@
public ddk::EmptyProtocol<ZX_PROTOCOL_CAMERA> {
public:
DISALLOW_COPY_AND_ASSIGN_ALLOW_MOVE(ControllerDevice);
- explicit ControllerDevice(zx_device_t* parent, zx_device_t* isp, zx_device_t* gdc,
- zx_device_t* ge2d, zx_device_t* sysmem, zx::event event)
+ explicit ControllerDevice(zx_device_t* parent, ddk::CompositeProtocolClient& composite,
+ zx::event event)
: ControllerDeviceType(parent),
- isp_(isp),
- gdc_(gdc),
- ge2d_(ge2d),
+ isp_(composite, "isp"),
+ gdc_(composite, "gdc"),
+ ge2d_(composite, "ge2d"),
shutdown_event_(std::move(event)),
loop_(&kAsyncLoopConfigNoAttachToCurrentThread),
- sysmem_(sysmem) {}
+ sysmem_(composite, "sysmem") {}
~ControllerDevice() { ShutDown(); }
diff --git a/src/camera/drivers/controller/test/device_test.cc b/src/camera/drivers/controller/test/device_test.cc
index 58c8ba2..0756190 100644
--- a/src/camera/drivers/controller/test/device_test.cc
+++ b/src/camera/drivers/controller/test/device_test.cc
@@ -28,9 +28,16 @@
zx::event event;
ASSERT_EQ(ZX_OK, zx::event::create(0, &event));
- controller_device_ = std::make_unique<ControllerDevice>(
- fake_ddk::kFakeParent, fake_ddk::kFakeParent, fake_ddk::kFakeParent, fake_ddk::kFakeParent,
- fake_ddk::kFakeParent, std::move(event));
+ composite_protocol_ops_t ops = {
+ .get_fragment = [](void* ctx, const char* name, zx_device_t** out) -> bool {
+ *out = fake_ddk::kFakeParent;
+ return true;
+ }};
+ composite_protocol_t proto{&ops, &ops};
+
+ ddk::CompositeProtocolClient composite(&proto);
+ controller_device_ =
+ std::make_unique<ControllerDevice>(fake_ddk::kFakeParent, composite, std::move(event));
}
void TearDown() override {
diff --git a/src/camera/drivers/hw_accel/gdc/gdc.cc b/src/camera/drivers/hw_accel/gdc/gdc.cc
index 11e86e7..71bda17c 100644
--- a/src/camera/drivers/hw_accel/gdc/gdc.cc
+++ b/src/camera/drivers/hw_accel/gdc/gdc.cc
@@ -34,12 +34,6 @@
constexpr uint32_t kAxiAlignment = 16;
constexpr uint32_t kWordSize = 4;
-enum {
- FRAGMENT_PDEV,
- FRAGMENT_SENSOR,
- FRAGMENT_COUNT,
-};
-
} // namespace
static inline uint32_t AxiWordAlign(zx_paddr_t value) {
@@ -483,15 +477,7 @@
return ZX_ERR_NOT_SUPPORTED;
}
- zx_device_t* fragments[FRAGMENT_COUNT];
- size_t actual;
- composite.GetFragments(fragments, FRAGMENT_COUNT, &actual);
- if (actual != FRAGMENT_COUNT) {
- FX_LOGST(ERROR, kTag) << "Could not get fragments";
- return ZX_ERR_NOT_SUPPORTED;
- }
-
- ddk::PDev pdev(fragments[FRAGMENT_PDEV]);
+ ddk::PDev pdev(composite);
if (!pdev.is_valid()) {
FX_LOGST(ERROR, kTag) << "ZX_PROTOCOL_PDEV not available";
return ZX_ERR_NO_RESOURCES;
diff --git a/src/camera/drivers/hw_accel/ge2d/ge2d.cc b/src/camera/drivers/hw_accel/ge2d/ge2d.cc
index ca9c1f7..2ab7dc9 100644
--- a/src/camera/drivers/hw_accel/ge2d/ge2d.cc
+++ b/src/camera/drivers/hw_accel/ge2d/ge2d.cc
@@ -29,13 +29,6 @@
constexpr uint32_t kGe2d = 0;
constexpr auto kTag = "ge2d";
-enum {
- FRAGMENT_PDEV,
- FRAGMENT_SENSOR,
- FRAGMENT_CANVAS,
- FRAGMENT_COUNT,
-};
-
} // namespace
zx_status_t Ge2dDevice::Ge2dInitTaskResize(
@@ -935,15 +928,7 @@
return ZX_ERR_NOT_SUPPORTED;
}
- zx_device_t* fragments[FRAGMENT_COUNT];
- size_t actual;
- composite.GetFragments(fragments, FRAGMENT_COUNT, &actual);
- if (actual != FRAGMENT_COUNT) {
- FX_LOGST(ERROR, kTag) << "Could not get fragments";
- return ZX_ERR_NOT_SUPPORTED;
- }
-
- ddk::PDev pdev(fragments[FRAGMENT_PDEV]);
+ ddk::PDev pdev(composite);
if (!pdev.is_valid()) {
FX_LOGST(ERROR, kTag) << "ZX_PROTOCOL_PDEV not available";
return ZX_ERR_NO_RESOURCES;
@@ -983,7 +968,7 @@
return status;
}
- ddk::AmlogicCanvasProtocolClient canvas(fragments[FRAGMENT_CANVAS]);
+ ddk::AmlogicCanvasProtocolClient canvas(composite, "canvas");
if (!canvas.is_valid()) {
FX_LOGST(ERROR, kTag) << "Could not get Amlogic Canvas protocol";
return ZX_ERR_NO_RESOURCES;
diff --git a/src/camera/drivers/sensors/imx227/imx227.cc b/src/camera/drivers/sensors/imx227/imx227.cc
index d042eb5..a011e990 100644
--- a/src/camera/drivers/sensors/imx227/imx227.cc
+++ b/src/camera/drivers/sensors/imx227/imx227.cc
@@ -433,17 +433,7 @@
return ZX_ERR_NOT_SUPPORTED;
}
- std::array<zx_device_t*, FRAGMENT_COUNT> fragments;
- size_t actual;
- composite.GetFragments(fragments.data(), FRAGMENT_COUNT, &actual);
- if (actual != FRAGMENT_COUNT) {
- zxlogf(ERROR, "%s Could not get fragments", __func__);
- return ZX_ERR_NOT_SUPPORTED;
- }
-
- auto sensor_device = std::make_unique<Imx227Device>(
- parent, fragments[FRAGMENT_I2C], fragments[FRAGMENT_GPIO_VANA], fragments[FRAGMENT_GPIO_VDIG],
- fragments[FRAGMENT_GPIO_CAM_RST], fragments[FRAGMENT_CLK24], fragments[FRAGMENT_MIPICSI]);
+ auto sensor_device = std::make_unique<Imx227Device>(parent, composite);
zx_status_t status = sensor_device->InitPdev();
if (status != ZX_OK) {
diff --git a/src/camera/drivers/sensors/imx227/imx227.h b/src/camera/drivers/sensors/imx227/imx227.h
index b553cb5..49011c9 100644
--- a/src/camera/drivers/sensors/imx227/imx227.h
+++ b/src/camera/drivers/sensors/imx227/imx227.h
@@ -67,27 +67,14 @@
class Imx227Device : public DeviceType,
public ddk::CameraSensor2Protocol<Imx227Device, ddk::base_protocol> {
public:
- enum {
- FRAGMENT_PDEV,
- FRAGMENT_MIPICSI,
- FRAGMENT_I2C,
- FRAGMENT_GPIO_VANA,
- FRAGMENT_GPIO_VDIG,
- FRAGMENT_GPIO_CAM_RST,
- FRAGMENT_CLK24,
- FRAGMENT_COUNT,
- };
-
- Imx227Device(zx_device_t* device, zx_device_t* i2c, zx_device_t* gpio_vana,
- zx_device_t* gpio_vdig, zx_device_t* gpio_cam_rst, zx_device_t* clk24,
- zx_device_t* mipicsi)
+ Imx227Device(zx_device_t* device, ddk::CompositeProtocolClient composite)
: DeviceType(device),
- i2c_(i2c),
- gpio_vana_enable_(gpio_vana),
- gpio_vdig_enable_(gpio_vdig),
- gpio_cam_rst_(gpio_cam_rst),
- clk24_(clk24),
- mipi_(mipicsi) {}
+ i2c_(composite, "i2c"),
+ gpio_vana_enable_(composite, "gpio-vana"),
+ gpio_vdig_enable_(composite, "gpio-vdig"),
+ gpio_cam_rst_(composite, "gpio-reset"),
+ clk24_(composite, "clock-sensor"),
+ mipi_(composite, "mipicsi") {}
static zx_status_t Create(zx_device_t* parent, std::unique_ptr<Imx227Device>* device_out);
static zx_status_t CreateAndBind(void* ctx, zx_device_t* parent);
diff --git a/src/camera/drivers/sensors/imx227/test/imx227_test.cc b/src/camera/drivers/sensors/imx227/test/imx227_test.cc
index 94d79e0..a890334 100644
--- a/src/camera/drivers/sensors/imx227/test/imx227_test.cc
+++ b/src/camera/drivers/sensors/imx227/test/imx227_test.cc
@@ -45,10 +45,30 @@
return std::vector<uint8_t>{static_cast<uint8_t>(bytes >> 8), static_cast<uint8_t>(bytes & 0xff)};
}
+class FakeComposite : public ddk::CompositeProtocol<FakeComposite> {
+ public:
+ explicit FakeComposite()
+ : proto_({&composite_protocol_ops_, this}) {}
+
+ const composite_protocol_t* proto() const { return &proto_; }
+
+ uint32_t CompositeGetFragmentCount() { return 0; }
+
+ void CompositeGetFragments(zx_device_t** comp_list, size_t comp_count, size_t* comp_actual) {}
+
+ void CompositeGetFragmentsNew(composite_device_fragment_t* comp_list, size_t comp_count,
+ size_t* comp_actual) {}
+
+ bool CompositeGetFragment(const char* name, zx_device_t** out) { return false; }
+
+ private:
+ composite_protocol_t proto_;
+};
+
class FakeImx227Device : public Imx227Device {
public:
- FakeImx227Device()
- : Imx227Device(fake_ddk::FakeParent(), nullptr, nullptr, nullptr, nullptr, nullptr, nullptr),
+ FakeImx227Device(ddk::CompositeProtocolClient composite)
+ : Imx227Device(fake_ddk::FakeParent(), composite),
proto_({&camera_sensor2_protocol_ops_, this}) {
SetProtocols();
ExpectInitPdev();
@@ -135,7 +155,7 @@
class Imx227DeviceTest : public zxtest::Test {
public:
- Imx227DeviceTest() {
+ Imx227DeviceTest() : dut_(ddk::CompositeProtocolClient(fake_composite_.proto())) {
fbl::Array<fake_ddk::ProtocolEntry> protocols(new fake_ddk::ProtocolEntry[1], 1);
protocols[0] = {ZX_PROTOCOL_CAMERA_SENSOR2,
*reinterpret_cast<const fake_ddk::Protocol*>(dut_.proto())};
@@ -143,7 +163,6 @@
}
void SetUp() override {
- auto dut = std::make_unique<FakeImx227Device>();
dut_.ExpectInit();
dut_.ExpectDeInit();
}
@@ -156,6 +175,7 @@
FakeImx227Device& dut() { return dut_; }
private:
+ FakeComposite fake_composite_;
fake_ddk::Bind ddk_;
FakeImx227Device dut_;
};
diff --git a/src/devices/board/drivers/sherlock/sherlock-camera.cc b/src/devices/board/drivers/sherlock/sherlock-camera.cc
index 285ca03..8a1a5a7 100644
--- a/src/devices/board/drivers/sherlock/sherlock-camera.cc
+++ b/src/devices/board/drivers/sherlock/sherlock-camera.cc
@@ -298,7 +298,7 @@
{countof(mipicsi_match), mipicsi_match},
};
static const device_fragment_t imx227_sensor_fragments[] = {
- {"mipcsi", countof(mipicsi_fragment), mipicsi_fragment},
+ {"mipicsi", countof(mipicsi_fragment), mipicsi_fragment},
{"i2c", countof(i2c_fragment), i2c_fragment},
{"gpio-vana", countof(gpio_vana_fragment), gpio_vana_fragment},
{"gpio-vdig", countof(gpio_vdig_fragment), gpio_vdig_fragment},
