From 3cbb7b12cb4e1798f451768bddaf4a76e63c29e5 Mon Sep 17 00:00:00 2001 From: Mike Voydanoff <voydanoff@google.com> Date: Mon, 22 Apr 2019 19:28:11 +0000 Subject: [PATCH] [dev][sherlock-tdm-output] Convert to composite device driver sherlock-tdm-output now uses composite protocol to access I2C and GPIOs. Support for sherlock p2 was removed from the board driver but remains in the audio driver. TEST: "audio -c 4 tone" on sherlock Change-Id: I28660a7ffe20b136af5d1127a7da2fbbd02a0dac --- .../dev/audio/sherlock-tdm-output/BUILD.gn | 3 +- .../sherlock-tdm-output/audio-stream-out.cpp | 69 +++++++++-- .../dev/audio/sherlock-tdm-output/binding.c | 22 ---- .../dev/audio/sherlock-tdm-output/tas5720.cpp | 8 +- .../dev/audio/sherlock-tdm-output/tas5720.h | 2 +- .../dev/audio/sherlock-tdm-output/tas5760.cpp | 8 +- .../dev/audio/sherlock-tdm-output/tas5760.h | 2 +- .../dev/board/sherlock/sherlock-audio.cpp | 117 +++++++++--------- .../dev/board/sherlock/sherlock-gpio.cpp | 3 + .../dev/board/sherlock/sherlock-gpios.h | 2 + .../dev/board/sherlock/sherlock-i2c.cpp | 24 ++++ 11 files changed, 152 insertions(+), 108 deletions(-) delete mode 100644 zircon/system/dev/audio/sherlock-tdm-output/binding.c diff --git a/zircon/system/dev/audio/sherlock-tdm-output/BUILD.gn b/zircon/system/dev/audio/sherlock-tdm-output/BUILD.gn index 7ce3c28ece3..8e7191ab5d7 100644 --- a/zircon/system/dev/audio/sherlock-tdm-output/BUILD.gn +++ b/zircon/system/dev/audio/sherlock-tdm-output/BUILD.gn @@ -5,12 +5,11 @@ driver("sherlock-tdm-output") { sources = [ "audio-stream-out.cpp", - "binding.c", "tas5720.cpp", "tas5760.cpp", ] deps = [ - "$zx/system/banjo/ddk-protocol-clock", + "$zx/system/banjo/ddk-protocol-composite", "$zx/system/banjo/ddk-protocol-gpio", "$zx/system/banjo/ddk-protocol-i2c", "$zx/system/banjo/ddk-protocol-platform-bus", diff --git a/zircon/system/dev/audio/sherlock-tdm-output/audio-stream-out.cpp b/zircon/system/dev/audio/sherlock-tdm-output/audio-stream-out.cpp index ce4cf650f4a..0e38fd101ab 100644 --- a/zircon/system/dev/audio/sherlock-tdm-output/audio-stream-out.cpp +++ b/zircon/system/dev/audio/sherlock-tdm-output/audio-stream-out.cpp @@ -2,7 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include <ddk/binding.h> #include <ddk/debug.h> +#include <ddk/platform-defs.h> +#include <ddk/protocol/composite.h> #include <ddktl/pdev.h> #include <soc/aml-t931/t931-gpio.h> @@ -11,6 +14,16 @@ namespace audio { namespace sherlock { +enum { + COMPONENT_PDEV, + COMPONENT_FAULT_GPIO, + COMPONENT_ENABLE_GPIO, + COMPONENT_I2C_0, + COMPONENT_I2C_1, + COMPONENT_I2C_2, // Optional + COMPONENT_COUNT, +}; + // Expects L+R for tweeters + L+R for the 1 Woofer (mixed in HW). // The user must perform crossover filtering on these channels. constexpr size_t kNumberOfChannels = 4; @@ -23,12 +36,28 @@ SherlockAudioStreamOut::SherlockAudioStreamOut(zx_device_t* parent) } zx_status_t SherlockAudioStreamOut::InitPdev() { + composite_protocol_t composite; + + auto status = device_get_protocol(parent(), ZX_PROTOCOL_COMPOSITE, &composite); + if (status != ZX_OK) { + zxlogf(ERROR, "Could not get composite protocol\n"); + return status; + } + + zx_device_t* components[COMPONENT_COUNT] = {}; + size_t actual; + composite_get_components(&composite, components, countof(components), &actual); + if (actual < countof(components) - 1) { + zxlogf(ERROR, "could not get components\n"); + return ZX_ERR_NOT_SUPPORTED; + } + + pdev_ = components[COMPONENT_PDEV]; if (!pdev_.is_valid()) { return ZX_ERR_NO_RESOURCES; } - size_t actual = 0; - zx_status_t status = device_get_metadata(parent(), DEVICE_METADATA_PRIVATE, &codecs_types_, + status = device_get_metadata(parent(), DEVICE_METADATA_PRIVATE, &codecs_types_, sizeof(metadata::Codec), &actual); if (status != ZX_OK || sizeof(metadata::Codec) != actual) { zxlogf(ERROR, "%s device_get_metadata failed %d\n", __FILE__, status); @@ -42,12 +71,12 @@ zx_status_t SherlockAudioStreamOut::InitPdev() { if (!ac.check()) { return ZX_ERR_NO_MEMORY; } - codecs_[0] = Tas5760::Create(pdev_, 0); + codecs_[0] = Tas5760::Create(components[COMPONENT_I2C_0]); if (!codecs_[0]) { zxlogf(ERROR, "%s could not get tas5760\n", __func__); return ZX_ERR_NO_RESOURCES; } - codecs_[1] = Tas5720::Create(pdev_, 1); + codecs_[1] = Tas5720::Create(components[COMPONENT_I2C_1]); if (!codecs_[1]) { zxlogf(ERROR, "%s could not get tas5720\n", __func__); return ZX_ERR_NO_RESOURCES; @@ -60,7 +89,7 @@ zx_status_t SherlockAudioStreamOut::InitPdev() { return ZX_ERR_NO_MEMORY; } for (uint32_t i = 0; i < 3; ++i) { - codecs_[i] = Tas5720::Create(pdev_, i); + codecs_[i] = Tas5720::Create(components[COMPONENT_I2C_0 + i]); if (!codecs_[i]) { zxlogf(ERROR, "%s could not get tas5720\n", __func__); return ZX_ERR_NO_RESOURCES; @@ -71,8 +100,8 @@ zx_status_t SherlockAudioStreamOut::InitPdev() { return ZX_ERR_NO_RESOURCES; } - audio_fault_ = pdev_.GetGpio(0); - audio_en_ = pdev_.GetGpio(1); + audio_fault_ = components[COMPONENT_FAULT_GPIO]; + audio_en_ = components[COMPONENT_ENABLE_GPIO]; if (!audio_fault_.is_valid() || !audio_en_.is_valid()) { zxlogf(ERROR, "%s failed to allocate gpio\n", __func__); @@ -363,11 +392,7 @@ zx_status_t SherlockAudioStreamOut::InitBuffer(size_t size) { return ZX_OK; } -} // sherlock -} // audio - -extern "C" zx_status_t audio_bind(void* ctx, zx_device_t* device, void** cookie) { - +static zx_status_t audio_bind(void* ctx, zx_device_t* device) { auto stream = audio::SimpleAudioStream::Create<audio::sherlock::SherlockAudioStreamOut>(device); if (stream == nullptr) { @@ -376,3 +401,23 @@ extern "C" zx_status_t audio_bind(void* ctx, zx_device_t* device, void** cookie) return ZX_OK; } + +static zx_driver_ops_t driver_ops = [](){ + zx_driver_ops_t ops = {}; + ops.version = DRIVER_OPS_VERSION; + ops.bind = audio_bind; + return ops; +}(); + +} // sherlock +} // audio + +// clang-format off +ZIRCON_DRIVER_BEGIN(aml_sherlock_tdm, audio::sherlock::driver_ops, "zircon", "0.1", 4) + BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_COMPOSITE), + BI_ABORT_IF(NE, BIND_PLATFORM_DEV_VID, PDEV_VID_AMLOGIC), + BI_ABORT_IF(NE, BIND_PLATFORM_DEV_PID, PDEV_PID_AMLOGIC_T931), + BI_MATCH_IF(EQ, BIND_PLATFORM_DEV_DID, PDEV_DID_AMLOGIC_TDM), +ZIRCON_DRIVER_END(aml_sherlock_tdm) +// clang-format on + diff --git a/zircon/system/dev/audio/sherlock-tdm-output/binding.c b/zircon/system/dev/audio/sherlock-tdm-output/binding.c deleted file mode 100644 index aa66d038472..00000000000 --- a/zircon/system/dev/audio/sherlock-tdm-output/binding.c +++ /dev/null @@ -1,22 +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 <ddk/binding.h> -#include <ddk/driver.h> -#include <ddk/platform-defs.h> - -extern zx_status_t audio_bind(void* ctx, zx_device_t* parent); - -static zx_driver_ops_t aml_tdm_driver_ops = { - .version = DRIVER_OPS_VERSION, - .bind = audio_bind, -}; - -// clang-format off -ZIRCON_DRIVER_BEGIN(aml_sherlock_tdm, aml_tdm_driver_ops, "zircon", "0.1", 3) - BI_ABORT_IF(NE, BIND_PLATFORM_DEV_VID, PDEV_VID_AMLOGIC), - BI_ABORT_IF(NE, BIND_PLATFORM_DEV_PID, PDEV_PID_AMLOGIC_T931), - BI_MATCH_IF(EQ, BIND_PLATFORM_DEV_DID, PDEV_DID_AMLOGIC_TDM), -ZIRCON_DRIVER_END(aml_sherlock_tdm) -// clang-format on diff --git a/zircon/system/dev/audio/sherlock-tdm-output/tas5720.cpp b/zircon/system/dev/audio/sherlock-tdm-output/tas5720.cpp index 8461f9594ce..3f999c23cd6 100644 --- a/zircon/system/dev/audio/sherlock-tdm-output/tas5720.cpp +++ b/zircon/system/dev/audio/sherlock-tdm-output/tas5720.cpp @@ -26,13 +26,7 @@ constexpr uint8_t kRegDigitalClipper1 = 0x11; // clang-format on // static -fbl::unique_ptr<Tas5720> Tas5720::Create(ddk::PDev pdev, uint32_t index) { - auto i2c = pdev.GetI2c(index); - if (!i2c.is_valid()) { - zxlogf(ERROR, "%s pdev_get_protocol failed\n", __FUNCTION__); - return nullptr; - } - +fbl::unique_ptr<Tas5720> Tas5720::Create(ddk::I2cChannel i2c) { fbl::AllocChecker ac; auto ptr = fbl::make_unique_checked<Tas5720>(&ac, i2c); if (!ac.check()) { diff --git a/zircon/system/dev/audio/sherlock-tdm-output/tas5720.h b/zircon/system/dev/audio/sherlock-tdm-output/tas5720.h index a1d00831201..f8e8911ef34 100644 --- a/zircon/system/dev/audio/sherlock-tdm-output/tas5720.h +++ b/zircon/system/dev/audio/sherlock-tdm-output/tas5720.h @@ -14,7 +14,7 @@ namespace audio { class Tas5720 final : public Codec { public: - static fbl::unique_ptr<Tas5720> Create(ddk::PDev pdev, uint32_t index); + static fbl::unique_ptr<Tas5720> Create(ddk::I2cChannel i2c); explicit Tas5720(const ddk::I2cChannel& i2c) : i2c_(i2c) {} diff --git a/zircon/system/dev/audio/sherlock-tdm-output/tas5760.cpp b/zircon/system/dev/audio/sherlock-tdm-output/tas5760.cpp index 293b07e37ae..7e91b928ef3 100644 --- a/zircon/system/dev/audio/sherlock-tdm-output/tas5760.cpp +++ b/zircon/system/dev/audio/sherlock-tdm-output/tas5760.cpp @@ -27,13 +27,7 @@ constexpr uint8_t kRegDigitalClipper1 = 0x11; // clang-format on // static -fbl::unique_ptr<Tas5760> Tas5760::Create(ddk::PDev pdev, uint32_t index) { - auto i2c = pdev.GetI2c(index); - if (!i2c.is_valid()) { - zxlogf(ERROR, "%s pdev_get_protocol failed\n", __func__); - return nullptr; - } - +fbl::unique_ptr<Tas5760> Tas5760::Create(ddk::I2cChannel i2c) { fbl::AllocChecker ac; auto ptr = fbl::make_unique_checked<Tas5760>(&ac, i2c); if (!ac.check()) { diff --git a/zircon/system/dev/audio/sherlock-tdm-output/tas5760.h b/zircon/system/dev/audio/sherlock-tdm-output/tas5760.h index 561bab3f658..0225bb048f1 100644 --- a/zircon/system/dev/audio/sherlock-tdm-output/tas5760.h +++ b/zircon/system/dev/audio/sherlock-tdm-output/tas5760.h @@ -14,7 +14,7 @@ namespace audio { class Tas5760 final : public Codec { public: - static fbl::unique_ptr<Tas5760> Create(ddk::PDev pdev, uint32_t index); + static fbl::unique_ptr<Tas5760> Create(ddk::I2cChannel i2c); explicit Tas5760(const ddk::I2cChannel& i2c) : i2c_(i2c) {} diff --git a/zircon/system/dev/board/sherlock/sherlock-audio.cpp b/zircon/system/dev/board/sherlock/sherlock-audio.cpp index 29c16f233e7..c6aa548d1b3 100644 --- a/zircon/system/dev/board/sherlock/sherlock-audio.cpp +++ b/zircon/system/dev/board/sherlock/sherlock-audio.cpp @@ -2,8 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "sherlock.h" - +#include <ddk/binding.h> #include <ddk/debug.h> #include <ddk/device.h> #include <ddk/platform-defs.h> @@ -13,21 +12,12 @@ #include <soc/aml-t931/t931-gpio.h> #include <soc/aml-t931/t931-hw.h> +#include "sherlock.h" +#include "sherlock-gpios.h" + namespace sherlock { zx_status_t Sherlock::AudioInit() { - - static constexpr pbus_gpio_t audio_gpios[] = { - { - // AUDIO_SOC_FAULT_L - .gpio = T931_GPIOZ(8), - }, - { - // SOC_AUDIO_EN - .gpio = T931_GPIOH(7), - }, - }; - static constexpr pbus_mmio_t audio_mmios[] = { { .base = T931_EE_AUDIO_BASE, @@ -50,32 +40,6 @@ zx_status_t Sherlock::AudioInit() { }, }; - static constexpr pbus_i2c_channel_t p2_codecs_i2cs[] = { - { - .bus_id = SHERLOCK_I2C_A0_0, - .address = 0x6c, // Tweeters. - }, - { - .bus_id = SHERLOCK_I2C_A0_0, - .address = 0x6f, // Woofer. - }, - }; - - static constexpr pbus_i2c_channel_t evt_codecs_i2cs[] = { - { - .bus_id = SHERLOCK_I2C_A0_0, - .address = 0x6c, // Tweeter left. - }, - { - .bus_id = SHERLOCK_I2C_A0_0, - .address = 0x6d, // Tweeter right. - }, - { - .bus_id = SHERLOCK_I2C_A0_0, - .address = 0x6f, // Woofer. - }, - }; - pdev_board_info_t board_info = {}; zx_status_t status = pbus_.GetBoardInfo(&board_info); if (status != ZX_OK) { @@ -85,10 +49,8 @@ zx_status_t Sherlock::AudioInit() { // We treat EVT and higher the same (having 3 TAS5720s). metadata::Codec out_codec = metadata::Codec::Tas5720x3; - if (board_info.board_revision < BOARD_REV_P2) { - return ZX_ERR_NOT_SUPPORTED; // For audio we don't support boards revision lower than P2. - } else if (board_info.board_revision < BOARD_REV_EVT1) { - out_codec = metadata::Codec::Tas5760_Tas5720; // We treat all P2 variants the same. + if (board_info.board_revision < BOARD_REV_EVT1) { + return ZX_ERR_NOT_SUPPORTED; // For audio we don't support boards revision lower than EVT. } pbus_metadata_t out_metadata[] = { @@ -99,13 +61,65 @@ zx_status_t Sherlock::AudioInit() { }, }; + constexpr zx_bind_inst_t root_match[] = { + BI_MATCH(), + }; + constexpr zx_bind_inst_t fault_gpio_match[] = { + BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_GPIO), + BI_MATCH_IF(EQ, BIND_GPIO_PIN, GPIO_AUDIO_SOC_FAULT_L), + }; + constexpr zx_bind_inst_t enable_gpio_match[] = { + BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_GPIO), + BI_MATCH_IF(EQ, BIND_GPIO_PIN, GPIO_SOC_AUDIO_EN), + }; + constexpr zx_bind_inst_t tweeter_left_i2c_match[] = { + BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_I2C), + BI_ABORT_IF(NE, BIND_I2C_BUS_ID, SHERLOCK_I2C_A0_0), + BI_MATCH_IF(EQ, BIND_I2C_ADDRESS, 0x6c), + }; + constexpr zx_bind_inst_t tweeter_right_i2c_match[] = { + BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_I2C), + BI_ABORT_IF(NE, BIND_I2C_BUS_ID, SHERLOCK_I2C_A0_0), + BI_MATCH_IF(EQ, BIND_I2C_ADDRESS, 0x6d), + }; + constexpr zx_bind_inst_t woofer_i2c_match[] = { + BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_I2C), + BI_ABORT_IF(NE, BIND_I2C_BUS_ID, SHERLOCK_I2C_A0_0), + BI_MATCH_IF(EQ, BIND_I2C_ADDRESS, 0x6f), + }; + const device_component_part_t fault_gpio_component[] = { + { countof(root_match), root_match }, + { countof(fault_gpio_match), fault_gpio_match }, + }; + const device_component_part_t enable_gpio_component[] = { + { countof(root_match), root_match }, + { countof(enable_gpio_match), enable_gpio_match }, + }; + const device_component_part_t tweeter_left_i2c_component[] = { + { countof(root_match), root_match }, + { countof(tweeter_left_i2c_match), tweeter_left_i2c_match }, + }; + const device_component_part_t tweeter_right_i2c_component[] = { + { countof(root_match), root_match }, + { countof(tweeter_right_i2c_match), tweeter_right_i2c_match }, + }; + const device_component_part_t woofer_i2c_component[] = { + { countof(root_match), root_match }, + { countof(woofer_i2c_match), woofer_i2c_match }, + }; + const device_component_t components[] = { + { countof(fault_gpio_component), fault_gpio_component }, + { countof(enable_gpio_component), enable_gpio_component }, + { countof(tweeter_left_i2c_component), tweeter_left_i2c_component }, + { countof(tweeter_right_i2c_component), tweeter_right_i2c_component }, + { countof(woofer_i2c_component), woofer_i2c_component }, + }; + pbus_dev_t tdm_dev = {}; tdm_dev.name = "SherlockAudio"; tdm_dev.vid = PDEV_VID_AMLOGIC; tdm_dev.pid = PDEV_PID_AMLOGIC_T931; tdm_dev.did = PDEV_DID_AMLOGIC_TDM; - tdm_dev.gpio_list = audio_gpios; - tdm_dev.gpio_count = countof(audio_gpios); tdm_dev.mmio_list = audio_mmios; tdm_dev.mmio_count = countof(audio_mmios); tdm_dev.bti_list = tdm_btis; @@ -113,14 +127,6 @@ zx_status_t Sherlock::AudioInit() { tdm_dev.metadata_list = out_metadata; tdm_dev.metadata_count = countof(out_metadata); - if (board_info.board_revision < BOARD_REV_EVT1) { - tdm_dev.i2c_channel_list = p2_codecs_i2cs; - tdm_dev.i2c_channel_count = countof(p2_codecs_i2cs); - } else { - tdm_dev.i2c_channel_list = evt_codecs_i2cs; - tdm_dev.i2c_channel_count = countof(evt_codecs_i2cs); - } - static constexpr pbus_mmio_t pdm_mmios[] = { { .base = T931_EE_PDM_BASE, @@ -149,7 +155,6 @@ zx_status_t Sherlock::AudioInit() { pdm_dev.bti_list = pdm_btis; pdm_dev.bti_count = countof(pdm_btis); - aml_hiu_dev_t hiu; status = s905d2_hiu_init(&hiu); if (status != ZX_OK) { @@ -180,7 +185,7 @@ zx_status_t Sherlock::AudioInit() { gpio_impl_.ConfigOut(T931_GPIOH(7), 1); // SOC_AUDIO_EN. - status = pbus_.DeviceAdd(&tdm_dev); + status = pbus_.CompositeDeviceAdd(&tdm_dev, components, countof(components), UINT32_MAX); if (status != ZX_OK) { zxlogf(ERROR, "%s pbus_.DeviceAdd failed %d\n", __FUNCTION__, status); return status; diff --git a/zircon/system/dev/board/sherlock/sherlock-gpio.cpp b/zircon/system/dev/board/sherlock/sherlock-gpio.cpp index e5ad6607c8e..2ad9c2a3727 100644 --- a/zircon/system/dev/board/sherlock/sherlock-gpio.cpp +++ b/zircon/system/dev/board/sherlock/sherlock-gpio.cpp @@ -77,6 +77,9 @@ static const gpio_pin_t gpio_pins[] = { // For touch screen. { GPIO_TOUCH_INTERRUPT }, { GPIO_TOUCH_RESET }, + // For audio out. + { GPIO_AUDIO_SOC_FAULT_L }, + { GPIO_SOC_AUDIO_EN }, }; static const pbus_metadata_t gpio_metadata[] = { diff --git a/zircon/system/dev/board/sherlock/sherlock-gpios.h b/zircon/system/dev/board/sherlock/sherlock-gpios.h index 83171e62590..7905e2cc0c5 100644 --- a/zircon/system/dev/board/sherlock/sherlock-gpios.h +++ b/zircon/system/dev/board/sherlock/sherlock-gpios.h @@ -11,3 +11,5 @@ #define GPIO_PANEL_DETECT T931_GPIOH(0) #define GPIO_TOUCH_INTERRUPT T931_GPIOZ(1) #define GPIO_TOUCH_RESET T931_GPIOZ(9) +#define GPIO_AUDIO_SOC_FAULT_L T931_GPIOZ(8) +#define GPIO_SOC_AUDIO_EN T931_GPIOH(7) diff --git a/zircon/system/dev/board/sherlock/sherlock-i2c.cpp b/zircon/system/dev/board/sherlock/sherlock-i2c.cpp index 185515bbc3b..994809f2e1f 100644 --- a/zircon/system/dev/board/sherlock/sherlock-i2c.cpp +++ b/zircon/system/dev/board/sherlock/sherlock-i2c.cpp @@ -64,6 +64,30 @@ static const i2c_channel_t i2c_channels[] = { .pid = 0, .did = 0, }, + // Tweeter left + { + .bus_id = SHERLOCK_I2C_A0_0, + .address = 0x6c, + .vid = 0, + .pid = 0, + .did = 0, + }, + // Tweeter right + { + .bus_id = SHERLOCK_I2C_A0_0, + .address = 0x6d, + .vid = 0, + .pid = 0, + .did = 0, + }, + // Woofer + { + .bus_id = SHERLOCK_I2C_A0_0, + .address = 0x6f, + .vid = 0, + .pid = 0, + .did = 0, + }, }; static const pbus_metadata_t i2c_metadata[] = { -- GitLab