From bee42c42aed86613ea73412642481382c29f741f Mon Sep 17 00:00:00 2001
From: Braden Kell <bradenkell@google.com>
Date: Mon, 29 Apr 2019 17:17:07 +0000
Subject: [PATCH] [cleo][touch] Use power driver to enable VGP1 regulator
Test: Used hid tool to verify touch screen still worked
Test: runtests -t mt8167s_ref-test
Change-Id: I07213a60a99c3c8f69bb4c1e4407deb5c1fbe791
---
zircon/system/dev/board/mt8167s_ref/BUILD.gn | 45 +++++++++++
.../dev/board/mt8167s_ref/mt8167-power.cpp | 21 +++++-
.../dev/board/mt8167s_ref/mt8167-test.cpp | 74 +++++++++++++++++++
.../dev/board/mt8167s_ref/mt8167-touch.cpp | 60 +--------------
.../system/dev/board/mt8167s_ref/mt8167.cpp | 2 -
zircon/system/dev/board/mt8167s_ref/mt8167.h | 46 +++++++-----
zircon/system/utest/BUILD.gn | 1 +
7 files changed, 172 insertions(+), 77 deletions(-)
create mode 100644 zircon/system/dev/board/mt8167s_ref/mt8167-test.cpp
diff --git a/zircon/system/dev/board/mt8167s_ref/BUILD.gn b/zircon/system/dev/board/mt8167s_ref/BUILD.gn
index 1961ef3b550..8cf74c899f3 100644
--- a/zircon/system/dev/board/mt8167s_ref/BUILD.gn
+++ b/zircon/system/dev/board/mt8167s_ref/BUILD.gn
@@ -28,6 +28,7 @@ driver("mt8167s_ref") {
"$zx/system/banjo/ddk.protocol.gpioimpl",
"$zx/system/banjo/ddk.protocol.platform.bus",
"$zx/system/banjo/ddk.protocol.platform.device",
+ "$zx/system/banjo/ddk.protocol.powerimpl",
"$zx/system/banjo/ddk.protocol.scpi",
"$zx/system/dev/lib/focaltech",
"$zx/system/dev/lib/mmio",
@@ -46,3 +47,47 @@ driver("mt8167s_ref") {
"$zx/kernel/target/arm64/boot-shim:mt8167s_ref",
]
}
+
+test("mt8167s_ref-test") {
+ output_name = "mt8167s_ref-test"
+ sources = [
+ "mt8167-audio.cpp",
+ "mt8167-backlight.cpp",
+ "mt8167-buttons.cpp",
+ "mt8167-clk.cpp",
+ "mt8167-display.cpp",
+ "mt8167-gpio.cpp",
+ "mt8167-gpu.cpp",
+ "mt8167-i2c.cpp",
+ "mt8167-msdc0.cpp",
+ "mt8167-msdc2.cpp",
+ "mt8167-power.cpp",
+ "mt8167-soc.cpp",
+ "mt8167-sysmem.cpp",
+ "mt8167-test.cpp",
+ "mt8167-thermal.cpp",
+ "mt8167-touch.cpp",
+ "mt8167-usb.cpp",
+ "mt8167.cpp",
+ ]
+ deps = [
+ "$zx/system/banjo/ddk.protocol.clockimpl",
+ "$zx/system/banjo/ddk.protocol.gpioimpl",
+ "$zx/system/banjo/ddk.protocol.platform.bus",
+ "$zx/system/banjo/ddk.protocol.platform.device",
+ "$zx/system/banjo/ddk.protocol.powerimpl",
+ "$zx/system/dev/lib/focaltech",
+ "$zx/system/dev/lib/mmio",
+ "$zx/system/dev/lib/mt8167",
+ "$zx/system/fidl/fuchsia-hardware-thermal:c",
+ "$zx/system/ulib/ddk",
+ "$zx/system/ulib/ddktl",
+ "$zx/system/ulib/driver",
+ "$zx/system/ulib/fbl",
+ "$zx/system/ulib/hwreg",
+ "$zx/system/ulib/zircon",
+ "$zx/system/ulib/zx",
+ "$zx/system/ulib/zxcpp",
+ "$zx/system/ulib/zxtest",
+ ]
+}
diff --git a/zircon/system/dev/board/mt8167s_ref/mt8167-power.cpp b/zircon/system/dev/board/mt8167s_ref/mt8167-power.cpp
index 2990d609090..af1410602ba 100644
--- a/zircon/system/dev/board/mt8167s_ref/mt8167-power.cpp
+++ b/zircon/system/dev/board/mt8167s_ref/mt8167-power.cpp
@@ -4,12 +4,28 @@
#include <ddk/debug.h>
#include <ddk/platform-defs.h>
+#include <ddktl/protocol/powerimpl.h>
#include <soc/mt8167/mt8167-hw.h>
#include "mt8167.h"
namespace board_mt8167 {
+zx_status_t Mt8167::Vgp1Enable() {
+ ddk::PowerImplProtocolClient power(parent());
+ if (!power.is_valid()) {
+ zxlogf(ERROR, "Failed to get power impl protocol\n");
+ return ZX_ERR_NO_RESOURCES;
+ }
+
+ zx_status_t status = power.EnablePowerDomain(kVDLdoVGp1);
+ if (status != ZX_OK) {
+ zxlogf(ERROR, "%s: Failed to enable VGP1 regulator: %d\n", __FUNCTION__, status);
+ }
+
+ return status;
+}
+
zx_status_t Mt8167::PowerInit() {
static const pbus_mmio_t power_mmios[] = {
{
@@ -28,9 +44,12 @@ zx_status_t Mt8167::PowerInit() {
zx_status_t status = pbus_.ProtocolDeviceAdd(ZX_PROTOCOL_POWER_IMPL, &power_dev);
if (status != ZX_OK) {
zxlogf(ERROR, "%s: Adding power device failed %d\n", __FUNCTION__, status);
+ return status;
}
- return status;
+ // Vgp1Enable() must be called before ThermalInit() as it uses the PMIC wrapper to enable the
+ // VGP1 regulator.
+ return Vgp1Enable();
}
} // namespace board_mt8167
diff --git a/zircon/system/dev/board/mt8167s_ref/mt8167-test.cpp b/zircon/system/dev/board/mt8167s_ref/mt8167-test.cpp
new file mode 100644
index 00000000000..f04ec358bb0
--- /dev/null
+++ b/zircon/system/dev/board/mt8167s_ref/mt8167-test.cpp
@@ -0,0 +1,74 @@
+// 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.
+
+#include "mt8167.h"
+
+#include <zxtest/zxtest.h>
+
+namespace board_mt8167 {
+
+class Mt8167Test : public Mt8167, public ddk::PBusProtocol<Mt8167Test> {
+public:
+ Mt8167Test() : Mt8167(nullptr) {
+ pbus_protocol_t pbus_proto = {.ops = &pbus_protocol_ops_, .ctx = this};
+ pbus_ = ddk::PBusProtocolClient(&pbus_proto);
+ }
+
+ bool Ok() { return vgp1_enable_called_ && thermal_enable_called_second_; }
+
+ // These stubs ensure the power device setup succeeds.
+ zx_status_t PBusDeviceAdd(const pbus_dev_t* dev) { return ZX_OK; }
+ zx_status_t PBusProtocolDeviceAdd(uint32_t proto_id, const pbus_dev_t* dev) { return ZX_OK; }
+ zx_status_t PBusRegisterProtocol(uint32_t proto_id, const void* protocol_buffer,
+ size_t protocol_size) {
+ return ZX_OK;
+ }
+ zx_status_t PBusGetBoardInfo(pdev_board_info_t* out_info) { return ZX_OK; }
+ zx_status_t PBusSetBoardInfo(const pbus_board_info_t* info) { return ZX_OK; }
+ zx_status_t PBusRegisterSysSuspendCallback(const pbus_sys_suspend_t* suspend_cb) {
+ return ZX_OK;
+ }
+ zx_status_t PBusCompositeDeviceAdd(const pbus_dev_t* dev,
+ const device_component_t* components_list,
+ size_t components_count,
+ uint32_t t_coresident_device_index) {
+ return ZX_OK;
+ }
+
+private:
+ zx_status_t Vgp1Enable() override {
+ vgp1_enable_called_ = true;
+ return ZX_OK;
+ }
+
+ zx_status_t Msdc0Init() override { return ZX_OK; }
+ zx_status_t Msdc2Init() override { return ZX_OK; }
+ zx_status_t SocInit() override { return ZX_OK; }
+ zx_status_t SysmemInit() override { return ZX_OK; }
+ zx_status_t GpioInit() override { return ZX_OK; }
+ zx_status_t GpuInit() override { return ZX_OK; }
+ zx_status_t DisplayInit() override { return ZX_OK; }
+ zx_status_t I2cInit() override { return ZX_OK; }
+ zx_status_t ButtonsInit() override { return ZX_OK; }
+ zx_status_t ClkInit() override { return ZX_OK; }
+ zx_status_t UsbInit() override { return ZX_OK; }
+ zx_status_t ThermalInit() override {
+ thermal_enable_called_second_ = vgp1_enable_called_;
+ return ZX_OK;
+ }
+ zx_status_t TouchInit() override { return ZX_OK; }
+ zx_status_t BacklightInit() override { return ZX_OK; }
+ zx_status_t AudioInit() override { return ZX_OK; }
+
+ bool vgp1_enable_called_ = false;
+ bool thermal_enable_called_second_ = false;
+};
+
+TEST(Mt8167Test, PmicInitOrder) {
+ Mt8167Test dut;
+ EXPECT_EQ(0, dut.Thread());
+ EXPECT_TRUE(dut.Ok());
+}
+
+} // namespace board_mt8167
diff --git a/zircon/system/dev/board/mt8167s_ref/mt8167-touch.cpp b/zircon/system/dev/board/mt8167s_ref/mt8167-touch.cpp
index e7fa1f010c3..1cd646ba6a4 100644
--- a/zircon/system/dev/board/mt8167s_ref/mt8167-touch.cpp
+++ b/zircon/system/dev/board/mt8167s_ref/mt8167-touch.cpp
@@ -2,55 +2,21 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include <limits.h>
-
#include <ddk/binding.h>
#include <ddk/debug.h>
#include <ddk/device.h>
#include <ddk/metadata.h>
#include <ddk/platform-defs.h>
#include <ddk/protocol/platform/bus.h>
-#include <lib/mmio/mmio.h>
#include <fbl/algorithm.h>
-#include <hwreg/bitfields.h>
#include <lib/focaltech/focaltech.h>
#include <soc/mt8167/mt8167-hw.h>
+#include <soc/mt8167/mt8167-power.h>
#include "mt8167.h"
-namespace {
-
-constexpr uintptr_t kPmicBaseAligned =
- fbl::round_down<uintptr_t, uintptr_t>(MT8167_PMIC_WRAP_BASE, PAGE_SIZE);
-constexpr size_t kPmicOffset = MT8167_PMIC_WRAP_BASE - kPmicBaseAligned;
-constexpr size_t kPmicSizeAligned =
- fbl::round_up<size_t, size_t>(kPmicOffset + MT8167_PMIC_WRAP_SIZE, PAGE_SIZE);
-
-constexpr uint32_t kDigLdoCon7 = 0x285;
-constexpr uint16_t kVgp1Enable = 0x8000;
-
-} // namespace
-
namespace board_mt8167 {
-class PmicCmd : public hwreg::RegisterBase<PmicCmd, uint32_t> {
-public:
- static auto Get() { return hwreg::RegisterAddr<PmicCmd>(0xa0 + kPmicOffset); }
-
- DEF_BIT(31, write);
- DEF_FIELD(30, 16, addr);
- DEF_FIELD(15, 0, data);
-};
-
-class PmicReadData : public hwreg::RegisterBase<PmicReadData, uint32_t> {
-public:
- static constexpr uint32_t kStateIdle = 0;
-
- static auto Get() { return hwreg::RegisterAddr<PmicReadData>(0xa4 + kPmicOffset); }
-
- DEF_FIELD(18, 16, status);
-};
-
zx_status_t Mt8167::TouchInit() {
if (board_info_.vid != PDEV_VID_GOOGLE || board_info_.pid != PDEV_PID_CLEO) {
return ZX_OK;
@@ -108,30 +74,12 @@ zx_status_t Mt8167::TouchInit() {
{ fbl::count_of(gpio_reset_component), gpio_reset_component },
};
- // Please do not use get_root_resource() in new code. See ZX-1497.
- zx::unowned_resource root_resource(get_root_resource());
- std::optional<ddk::MmioBuffer> pmic_mmio;
- auto status = ddk::MmioBuffer::Create(kPmicBaseAligned, kPmicSizeAligned, *root_resource,
- ZX_CACHE_POLICY_UNCACHED_DEVICE, &pmic_mmio);
- if (status != ZX_OK) {
- zxlogf(ERROR, "%s: Failed to enable VGP1 regulator: %d\n", __FUNCTION__, status);
- return status;
- }
-
- while (PmicReadData::Get().ReadFrom(&(*pmic_mmio)).status() != PmicReadData::kStateIdle) {}
-
- PmicCmd::Get()
- .FromValue(0)
- .set_write(1)
- .set_addr(kDigLdoCon7)
- .set_data(kVgp1Enable)
- .WriteTo(&(*pmic_mmio));
-
// platform device protocol is only needed to provide metadata to the driver.
// TODO(voydanoff) remove pdev after we have a better way to provide metadata to composite
// devices.
- if ((status = pbus_.CompositeDeviceAdd(&touch_dev, ft_components, fbl::count_of(ft_components),
- UINT32_MAX)) != ZX_OK) {
+ zx_status_t status = pbus_.CompositeDeviceAdd(&touch_dev, ft_components,
+ fbl::count_of(ft_components), UINT32_MAX);
+ if (status != ZX_OK) {
zxlogf(ERROR, "%s: Failed to add touch device: %d\n", __FUNCTION__, status);
}
diff --git a/zircon/system/dev/board/mt8167s_ref/mt8167.cpp b/zircon/system/dev/board/mt8167s_ref/mt8167.cpp
index bd564a72d75..152c0b27624 100644
--- a/zircon/system/dev/board/mt8167s_ref/mt8167.cpp
+++ b/zircon/system/dev/board/mt8167s_ref/mt8167.cpp
@@ -106,8 +106,6 @@ int Mt8167::Thread() {
if (UsbInit() != ZX_OK) {
zxlogf(ERROR, "UsbInit() failed\n");
}
- // TouchInit() must be called before ThermalInit() as the touch driver uses the PMIC wrapper to
- // enable the VGP1 regulator.
if (TouchInit() != ZX_OK) {
zxlogf(ERROR, "TouchInit() failed\n");
}
diff --git a/zircon/system/dev/board/mt8167s_ref/mt8167.h b/zircon/system/dev/board/mt8167s_ref/mt8167.h
index fbf46a3198b..e3af003e748 100644
--- a/zircon/system/dev/board/mt8167s_ref/mt8167.h
+++ b/zircon/system/dev/board/mt8167s_ref/mt8167.h
@@ -41,36 +41,46 @@ class Mt8167 : public Mt8167Type {
public:
explicit Mt8167(zx_device_t* parent, pbus_protocol_t* pbus, pdev_board_info_t* board_info)
: Mt8167Type(parent), pbus_(pbus), board_info_(*board_info) {}
+ virtual ~Mt8167() = default;
static zx_status_t Create(zx_device_t* parent);
// Device protocol implementation.
void DdkRelease();
+ // Visible for testing.
+ int Thread();
+
+protected:
+ explicit Mt8167(zx_device_t* parent) : Mt8167Type(parent) {}
+
+ virtual zx_status_t Vgp1Enable();
+
+ virtual zx_status_t Msdc0Init();
+ virtual zx_status_t Msdc2Init();
+ virtual zx_status_t SocInit();
+ virtual zx_status_t SysmemInit();
+ virtual zx_status_t GpioInit();
+ virtual zx_status_t GpuInit();
+ virtual zx_status_t DisplayInit();
+ virtual zx_status_t I2cInit();
+ virtual zx_status_t ButtonsInit();
+ virtual zx_status_t ClkInit();
+ virtual zx_status_t UsbInit();
+ virtual zx_status_t ThermalInit();
+ virtual zx_status_t TouchInit();
+ virtual zx_status_t BacklightInit();
+ virtual zx_status_t AudioInit();
+
+ ddk::PBusProtocolClient pbus_;
+
private:
DISALLOW_COPY_ASSIGN_AND_MOVE(Mt8167);
zx_status_t Start();
- zx_status_t Msdc0Init();
- zx_status_t Msdc1Init();
- zx_status_t Msdc2Init();
- zx_status_t SocInit();
- zx_status_t SysmemInit();
- zx_status_t GpioInit();
- zx_status_t GpuInit();
- zx_status_t DisplayInit();
- zx_status_t I2cInit();
- zx_status_t ButtonsInit();
- zx_status_t ClkInit();
+
zx_status_t PowerInit();
- zx_status_t UsbInit();
- zx_status_t ThermalInit();
- zx_status_t TouchInit();
- zx_status_t BacklightInit();
- zx_status_t AudioInit();
- int Thread();
- ddk::PBusProtocolClient pbus_;
gpio_impl_protocol_t gpio_impl_;
pdev_board_info_t board_info_;
thrd_t thread_;
diff --git a/zircon/system/utest/BUILD.gn b/zircon/system/utest/BUILD.gn
index 22d672875ad..10549673149 100644
--- a/zircon/system/utest/BUILD.gn
+++ b/zircon/system/utest/BUILD.gn
@@ -24,6 +24,7 @@ if (current_cpu != "") {
"$zx/system/dev/block/sdmmc:sdmmc-test",
"$zx/system/dev/block/usb-mass-storage:tests",
"$zx/system/dev/bluetooth/bt-hci-mediatek:bt-hci-mediatek-test",
+ "$zx/system/dev/board/mt8167s_ref:mt8167s_ref-test",
"$zx/system/dev/bus/pci:pci_tests",
"$zx/system/dev/bus/virtio:virtio-test",
"$zx/system/dev/gpio/mt-8167:mtk-gpio",
--
GitLab