[audio][drivers][tas58xx] Detect and log faults in the TAS58xx codec

Enable interrupts on FAULT from the TAS58xx; on interrupt, save the
timestamp and fault syndromes.  A periodic task will pick up the
saved information and log it.

Bug: 104356

Test: Synthesized clock faults by flat-lining SCLK and verified behavior.
Change-Id: I6b006d68e485be5311c4a894cf27becec978abad
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/707239
Reviewed-by: Andres Oportus <[email protected]>
Commit-Queue: Jack Brennen <[email protected]>

Author: Jack Brennen

src/devices/board/drivers/nelson/nelson-audio.cc

@@ -61,35 +61,36 @@ static const device_fragment_part_t p2_out_codec_fragment[] = {
     {std::size(p2_out_codec_match), p2_out_codec_match},
 };
 
-static const zx_bind_inst_t ref_out_enable_gpio_match[] = {
+static const zx_bind_inst_t out_enable_gpio_match[] = {
     BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_GPIO),
     BI_MATCH_IF(EQ, BIND_GPIO_PIN, GPIO_SOC_AUDIO_EN),
 };
-static const zx_bind_inst_t ref_out_fault_gpio_match[] = {
+static const zx_bind_inst_t out_fault_gpio_match[] = {
     BI_ABORT_IF(NE, BIND_PROTOCOL, ZX_PROTOCOL_GPIO),
     BI_MATCH_IF(EQ, BIND_GPIO_PIN, GPIO_AUDIO_SOC_FAULT_L),
 };
-static const device_fragment_part_t ref_out_enable_gpio_fragment[] = {
-    {std::size(ref_out_enable_gpio_match), ref_out_enable_gpio_match},
+static const device_fragment_part_t out_enable_gpio_fragment[] = {
+    {std::size(out_enable_gpio_match), out_enable_gpio_match},
 };
-static const device_fragment_part_t ref_out_fault_gpio_fragment[] = {
-    {std::size(ref_out_fault_gpio_match), ref_out_fault_gpio_match},
+static const device_fragment_part_t out_fault_gpio_fragment[] = {
+    {std::size(out_fault_gpio_match), out_fault_gpio_match},
 };
 
 static const device_fragment_t ref_codec_fragments[] = {
     {"i2c", std::size(ref_out_i2c_fragment), ref_out_i2c_fragment},
-    {"gpio-enable", std::size(ref_out_enable_gpio_fragment), ref_out_enable_gpio_fragment},
-    {"gpio-fault", std::size(ref_out_fault_gpio_fragment), ref_out_fault_gpio_fragment},
+    {"gpio-enable", std::size(out_enable_gpio_fragment), out_enable_gpio_fragment},
+    {"gpio-fault", std::size(out_fault_gpio_fragment), out_fault_gpio_fragment},
 };
 static const device_fragment_t p2_codec_fragments[] = {
     {"i2c", std::size(p2_out_i2c_fragment), p2_out_i2c_fragment},
+    {"gpio-fault", std::size(out_fault_gpio_fragment), out_fault_gpio_fragment},
 };
 static const device_fragment_t ref_controller_fragments[] = {
-    {"gpio-enable", std::size(ref_out_enable_gpio_fragment), ref_out_enable_gpio_fragment},
+    {"gpio-enable", std::size(out_enable_gpio_fragment), out_enable_gpio_fragment},
     {"codec-01", std::size(ref_out_codec_fragment), ref_out_codec_fragment},
 };
 static const device_fragment_t p2_controller_fragments[] = {
-    {"gpio-enable", std::size(ref_out_enable_gpio_fragment), ref_out_enable_gpio_fragment},
+    {"gpio-enable", std::size(out_enable_gpio_fragment), out_enable_gpio_fragment},
     {"codec-01", std::size(p2_out_codec_fragment), p2_out_codec_fragment},
 };
 

src/media/audio/drivers/codecs/tas58xx/BUILD.gn

@@ -24,13 +24,15 @@ fuchsia_driver("tas58xx-driver") {
   sources = [ "tas58xx.cc" ]
   deps = [
     ":ti_tas58xx-bind",
+    "//sdk/banjo/fuchsia.hardware.gpio:fuchsia.hardware.gpio_banjo_cpp",
     "//sdk/banjo/fuchsia.hardware.platform.device:fuchsia.hardware.platform.device_banjo_cpp",
     "//src/devices/i2c/lib/device-protocol-i2c-channel",
     "//src/devices/lib/driver",
     "//src/devices/lib/ti",
     "//src/lib/ddk",
     "//src/lib/ddktl",
     "//src/media/audio/lib/simple-codec",
+    "//zircon/system/ulib/async:async-cpp",
     "//zircon/system/ulib/fbl",
     "//zircon/system/ulib/trace:trace-driver",
     "//zircon/system/ulib/zircon-internal",
@@ -54,6 +56,8 @@ test("tas58xx-test") {
   ]
   deps = [
     ":ti_tas58xx-bind",
+    "//sdk/banjo/fuchsia.hardware.gpio:fuchsia.hardware.gpio_banjo_cpp",
+    "//sdk/banjo/fuchsia.hardware.gpio:fuchsia.hardware.gpio_banjo_cpp_mock",
     "//sdk/fidl/fuchsia.hardware.i2c:fuchsia.hardware.i2c_cpp_wire",
     "//src/devices/i2c/lib/device-protocol-i2c-channel",
     "//src/devices/i2c/testing/mock-i2c",

src/media/audio/drivers/codecs/tas58xx/tas58xx-test.cc

@@ -4,6 +4,7 @@
 
 #include "tas58xx.h"
 
+#include <fuchsia/hardware/gpio/cpp/banjo-mock.h>
 #include <lib/async-loop/cpp/loop.h>
 #include <lib/async-loop/default.h>
 #include <lib/ddk/metadata.h>
@@ -25,15 +26,17 @@ namespace audio_fidl = ::fuchsia::hardware::audio;
 namespace signal_fidl = ::fuchsia::hardware::audio::signalprocessing;
 
 struct Tas58xxCodec : public Tas58xx {
-  explicit Tas58xxCodec(zx_device_t* parent, ddk::I2cChannel i2c)
-      : Tas58xx(parent, std::move(i2c)) {}
+  explicit Tas58xxCodec(zx_device_t* parent, ddk::I2cChannel i2c,
+                        ddk::GpioProtocolClient gpio_fault)
+      : Tas58xx(parent, std::move(i2c), std::move(gpio_fault)) {}
   codec_protocol_t GetProto() { return {&this->codec_protocol_ops_, this}; }
   uint64_t GetTopologyId() { return Tas58xx::GetTopologyId(); }
   uint64_t GetAglPeId() { return Tas58xx::GetAglPeId(); }
   uint64_t GetEqPeId() { return Tas58xx::GetEqPeId(); }
   zx_status_t SetBand(bool enabled, size_t index, uint32_t frequency, float Q, float gain_db) {
     return Tas58xx::SetBand(enabled, index, frequency, Q, gain_db);
   }
+  bool BackgroundFaultPollingIsEnabled() override { return false; }
 };
 
 class Tas58xxTest : public zxtest::Test {
@@ -52,8 +55,11 @@ class Tas58xxTest : public zxtest::Test {
     mock_i2c_.ExpectWrite({0x67}).ExpectReadStop({0x00}, ZX_ERR_INTERNAL);  // Error will retry.
     mock_i2c_.ExpectWrite({0x67}).ExpectReadStop({0x00}, ZX_ERR_INTERNAL);  // Error will retry.
     mock_i2c_.ExpectWrite({0x67}).ExpectReadStop({0x00}, ZX_OK);  // Check DIE ID, no error now.
-    ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(fake_parent_.get(),
-                                                                 std::move(endpoints->client)));
+
+    ddk::MockGpio mock_fault;
+
+    ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(
+        fake_parent_.get(), std::move(endpoints->client), mock_fault.GetProto()));
 
     auto* child_dev = fake_parent_->GetLatestChild();
     ASSERT_NOT_NULL(child_dev);
@@ -353,8 +359,11 @@ class Tas58xxSignalProcessingTest : public zxtest::Test {
 
     loop_.StartThread();
     mock_i2c_.ExpectWrite({0x67}).ExpectReadStop({0x00}, ZX_OK);  // Check DIE ID.
-    ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(fake_parent_.get(),
-                                                                 std::move(endpoints->client)));
+
+    ddk::MockGpio mock_fault;
+
+    ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(
+        fake_parent_.get(), std::move(endpoints->client), mock_fault.GetProto()));
 
     auto* child_dev = fake_parent_->GetLatestChild();
     ASSERT_NOT_NULL(child_dev);
@@ -431,8 +440,10 @@ TEST(Tas58xxSignalProcessingTest, SignalProcessingConnectTooManyConnections) {
   fidl::BindServer<mock_i2c::MockI2c>(loop.dispatcher(), std::move(endpoints->server), &mock_i2c);
   loop.StartThread();
 
-  ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(fake_parent.get(),
-                                                               std::move(endpoints->client)));
+  ddk::MockGpio mock_fault;
+
+  ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(
+      fake_parent.get(), std::move(endpoints->client), mock_fault.GetProto()));
   auto* child_dev = fake_parent->GetLatestChild();
   ASSERT_NOT_NULL(child_dev);
   auto codec = child_dev->GetDeviceContext<Tas58xxCodec>();
@@ -1219,8 +1230,10 @@ TEST(Tas58xxTest, Reset) {
   fidl::BindServer<mock_i2c::MockI2c>(loop.dispatcher(), std::move(endpoints->server), &mock_i2c);
   loop.StartThread();
 
-  ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(fake_parent.get(),
-                                                               std::move(endpoints->client)));
+  ddk::MockGpio mock_fault;
+
+  ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(
+      fake_parent.get(), std::move(endpoints->client), mock_fault.GetProto()));
   auto* child_dev = fake_parent->GetLatestChild();
   ASSERT_NOT_NULL(child_dev);
   auto codec = child_dev->GetDeviceContext<Tas58xxCodec>();
@@ -1268,8 +1281,10 @@ TEST(Tas58xxTest, Bridged) {
   fidl::BindServer<mock_i2c::MockI2c>(loop.dispatcher(), std::move(endpoints->server), &mock_i2c);
   loop.StartThread();
 
-  ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(fake_parent.get(),
-                                                               std::move(endpoints->client)));
+  ddk::MockGpio mock_fault;
+
+  ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(
+      fake_parent.get(), std::move(endpoints->client), mock_fault.GetProto()));
   auto* child_dev = fake_parent->GetLatestChild();
   ASSERT_NOT_NULL(child_dev);
   auto codec = child_dev->GetDeviceContext<Tas58xxCodec>();
@@ -1378,8 +1393,10 @@ TEST(Tas58xxTest, ExternalConfig) {
   fidl::BindServer<mock_i2c::MockI2c>(loop.dispatcher(), std::move(endpoints->server), &mock_i2c);
   loop.StartThread();
 
-  ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(fake_parent.get(),
-                                                               std::move(endpoints->client)));
+  ddk::MockGpio mock_fault;
+
+  ASSERT_OK(SimpleCodecServer::CreateAndAddToDdk<Tas58xxCodec>(
+      fake_parent.get(), std::move(endpoints->client), mock_fault.GetProto()));
   auto* child_dev = fake_parent->GetLatestChild();
   ASSERT_NOT_NULL(child_dev);
   auto codec = child_dev->GetDeviceContext<Tas58xxCodec>();

src/media/audio/drivers/codecs/tas58xx/tas58xx.cc

@@ -4,6 +4,7 @@
 
 #include "tas58xx.h"
 
+#include <lib/async/cpp/task.h>
 #include <lib/ddk/metadata.h>
 #include <lib/ddk/platform-defs.h>
 #include <lib/ddk/trace/event.h>
@@ -30,6 +31,10 @@ constexpr uint8_t kRegSapCtrl1    = 0x33;
 constexpr uint8_t kRegSapCtrl2    = 0x34;
 constexpr uint8_t kRegDigitalVol  = 0x4c;
 constexpr uint8_t kRegDspMisc     = 0x66;
+constexpr uint8_t kRegChanFault    = 0x70;
+constexpr uint8_t kRegGlobalFault1 = 0x71;
+constexpr uint8_t kRegGlobalFault2 = 0x72;
+constexpr uint8_t kRegOtWarning    = 0x73;
 constexpr uint8_t kRegClearFault  = 0x78;
 constexpr uint8_t kRegSelectBook  = 0x7f;
 
@@ -74,8 +79,8 @@ static const audio::DaiSupportedFormats kSupportedDaiDaiFormats = {
     .bits_per_sample = kSupportedDaiBitsPerSample,
 };
 
-Tas58xx::Tas58xx(zx_device_t* device, ddk::I2cChannel i2c)
-    : SimpleCodecServer(device), i2c_(std::move(i2c)) {
+Tas58xx::Tas58xx(zx_device_t* device, ddk::I2cChannel i2c, ddk::GpioProtocolClient fault_gpio)
+    : SimpleCodecServer(device), i2c_(std::move(i2c)), fault_gpio_(std::move(fault_gpio)) {
   size_t actual = 0;
   auto status = device_get_metadata(parent(), DEVICE_METADATA_PRIVATE, &metadata_,
                                     sizeof(metadata_), &actual);
@@ -180,7 +185,48 @@ zx_status_t Tas58xx::Reset() {
   return ZX_OK;
 }
 
+void Tas58xx::ScheduleFaultPolling() {
+  if (!BackgroundFaultPollingIsEnabled())
+    return;
+
+  async::PostDelayedTask(
+      dispatcher(), [this]() { PeriodicPollFaults(); }, poll_interval_);
+}
+
+void Tas58xx::PeriodicPollFaults() {
+  uint8_t fault_state;
+  auto status = fault_gpio_.Read(&fault_state);
+  if (status != ZX_OK) {
+    zxlogf(WARNING, "GPIO error while polling fault data");
+  } else if (fault_state == 0) {  // Active low; 0 means the pin is active!
+    // Codec is driving fault pin active.  Read the fault registers.
+    fault_info_.i2c_error = (ReadReg(kRegChanFault, &fault_info_.chan_fault) != ZX_OK) ||
+                            (ReadReg(kRegGlobalFault1, &fault_info_.global_fault1) != ZX_OK) ||
+                            (ReadReg(kRegGlobalFault2, &fault_info_.global_fault2) != ZX_OK) ||
+                            (ReadReg(kRegOtWarning, &fault_info_.ot_warning) != ZX_OK);
+
+    // Reset the fault indication at the codec.
+    WriteReg(kRegClearFault, kRegClearFaultBitsAnalog);
+
+    // Log the fault based on the data retrieved earlier.
+    zxlogf(WARNING, "Codec fault detected");
+    if (fault_info_.i2c_error)
+      zxlogf(WARNING, "I2C error while retrieving fault data");
+    if (fault_info_.chan_fault)
+      zxlogf(WARNING, "Channel fault seen: %02X", fault_info_.chan_fault);
+    if (fault_info_.global_fault1)
+      zxlogf(WARNING, "Global fault1 seen: %02X", fault_info_.global_fault1);
+    if (fault_info_.global_fault2)
+      zxlogf(WARNING, "Global fault2 seen: %02X", fault_info_.global_fault2);
+    if (fault_info_.ot_warning)
+      zxlogf(WARNING, "OT warning seen: %02X", fault_info_.ot_warning);
+  }
+
+  ScheduleFaultPolling();
+}
+
 zx::status<DriverIds> Tas58xx::Initialize() {
+  ScheduleFaultPolling();
   return zx::ok(DriverIds{
       .vendor_id = PDEV_VID_TI,
       .device_id = PDEV_DID_TI_TAS58xx,
@@ -195,7 +241,14 @@ zx_status_t Tas58xx::Create(zx_device_t* parent) {
     return ZX_ERR_NO_RESOURCES;
   }
 
-  return SimpleCodecServer::CreateAndAddToDdk<Tas58xx>(parent, std::move(i2c));
+  ddk::GpioProtocolClient fault_gpio(parent, "gpio-fault");
+  if (!fault_gpio.is_valid()) {
+    zxlogf(ERROR, "Could not get gpio-fault");
+    return ZX_ERR_INTERNAL;
+  }
+
+  return SimpleCodecServer::CreateAndAddToDdk<Tas58xx>(parent, std::move(i2c),
+                                                       std::move(fault_gpio));
 }
 
 Info Tas58xx::GetInfo() {

src/media/audio/drivers/codecs/tas58xx/tas58xx.h

@@ -5,6 +5,8 @@
 #ifndef SRC_MEDIA_AUDIO_DRIVERS_CODECS_TAS58XX_TAS58XX_H_
 #define SRC_MEDIA_AUDIO_DRIVERS_CODECS_TAS58XX_TAS58XX_H_
 
+#include <fuchsia/hardware/gpio/cpp/banjo.h>
+#include <lib/async/cpp/irq.h>
 #include <lib/ddk/debug.h>
 #include <lib/ddk/device.h>
 #include <lib/device-protocol/i2c-channel.h>
@@ -28,7 +30,7 @@ class Tas58xx : public SimpleCodecServer,
  public:
   static zx_status_t Create(zx_device_t* parent);
 
-  explicit Tas58xx(zx_device_t* device, ddk::I2cChannel i2c);
+  explicit Tas58xx(zx_device_t* device, ddk::I2cChannel i2c, ddk::GpioProtocolClient gpio_fault);
 
   // Implementation for SimpleCodecServer.
   zx_status_t Shutdown() override;
@@ -71,6 +73,10 @@ class Tas58xx : public SimpleCodecServer,
   uint64_t GetTopologyId() { return kTopologyId; }
   uint64_t GetAglPeId() { return kAglPeId; }
   uint64_t GetEqPeId() { return kEqPeId; }
+  virtual bool BackgroundFaultPollingIsEnabled() {
+    return true;  // Unit test can override to disable.
+  }
+  void PeriodicPollFaults();  // Unit test can invoke this directly.
 
  private:
   static constexpr float kMaxGain = 24.0;
@@ -105,6 +111,21 @@ class Tas58xx : public SimpleCodecServer,
   uint32_t number_of_channels_ = 2;
   uint32_t rate_ = 48'000;
 
+  // How often to poll for codec faults.  Would be nice to use IRQ for this,
+  // but we've exhausted the IRQ supply, so 20 seconds gives a balance between
+  // keeping overhead low but polling often enough to catch faults on a
+  // relatively timely basis.
+  static constexpr zx::duration poll_interval_ = zx::sec(20);
+  void ScheduleFaultPolling();
+  ddk::GpioProtocolClient fault_gpio_;
+  struct {
+    bool i2c_error;         // True if saw an I2C error while reading fault
+    uint8_t chan_fault;     // Channel fault bits
+    uint8_t global_fault1;  // Global fault bits
+    uint8_t global_fault2;  // More global fault bits
+    uint8_t ot_warning;     // Over-temperature warning bit
+  } fault_info_;
+
   // AGL.
   bool last_agl_ = false;
   std::optional<bool> last_reported_agl_;