commit 95931b8f3b1b53cb337c8359c0495e2798863318
author Gilad Arnold <garnold@chromium.org> Wed Jan 09 10:37:17 2013 -0800
committer ChromeBot <chrome-bot@google.com> Thu Jan 10 00:30:59 2013 -0800
tree 2baf215cb4d72ccc9b56c592df9713c4169f05cb
parent 8e3f1263218c5046d2281c1d1d3c886acda85b47

AU: fix bug in GPIO handler that caused failed discovery to be ignored

This solves two issues:

* The GPIO handler will attempt to use GPIO devices even when discovery
  via udev has failed. Not any more.

* GPIO discovery would return success even when it failed on a previous
  attempt. Now it'll reflect the actual result of the discovery attempt.

* Reporting of file descriptor open errors is obscured due to
  intermittent operations that reset errno, now fixed.

* Added test case to ensure that repeat GPIO discovery is not attempted,
  and that test mode check will automatically fail if a previous
  initialization/discovery had failed.

BUG=None
TEST=Passes unit tests
TEST=All symptoms gone on real update attempt w/ x86-alex

Change-Id: I01a7b1e316dbb5b94487a5aad1560d994feae9ff
Reviewed-on: https://gerrit.chromium.org/gerrit/40946
Commit-Queue: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>

gpio_handler.cc

diff --git a/gpio_handler.cc b/gpio_handler.cc
index f49a555..8627ba2 100644
--- a/gpio_handler.cc
+++ b/gpio_handler.cc
@@ -46,7 +46,8 @@
     : udev_iface_(udev_iface),
       fd_(fd),
       is_cache_test_mode_(is_cache_test_mode),
-      is_discovery_attempted_(false) {
+      is_discovery_attempted_(false),
+      is_discovery_successful_(false) {
   CHECK(udev_iface && fd);
 
   // Ensure there's only one instance of this class.
@@ -57,9 +58,8 @@
   ResetTestModeSignalingFlags();
 
   // If GPIO discovery not deferred, do it.
-  if (!(is_defer_discovery || DiscoverGpios())) {
-    LOG(WARNING) << "GPIO discovery failed";
-  }
+  if (!is_defer_discovery)
+    DiscoverGpios();
 }
 
 StandardGpioHandler::~StandardGpioHandler() {
@@ -67,19 +67,19 @@
 }
 
 bool StandardGpioHandler::IsTestModeSignaled() {
-  // Attempt GPIO discovery.
-  if (!DiscoverGpios()) {
-    LOG(WARNING) << "GPIO discovery failed";
-  }
+  bool is_returning_cached = false;  // for logging purposes
 
-  // Force a check if so requested.
-  if (!is_cache_test_mode_)
-    ResetTestModeSignalingFlags();
+  // Attempt GPIO discovery first.
+  if (DiscoverGpios()) {
+    // Force a check if so requested.
+    if (!is_cache_test_mode_)
+      ResetTestModeSignalingFlags();
 
-  bool is_returning_cached = !is_first_check_;  // for logging purposes
-  if (is_first_check_) {
-    is_first_check_ = false;
-    DoTestModeSignalingProtocol();
+    is_returning_cached = !is_first_check_;  // for logging purposes
+    if (is_first_check_) {
+      is_first_check_ = false;
+      DoTestModeSignalingProtocol();
+    }
   }
 
   LOG(INFO) << "result: " << (is_test_mode_ ? "test" : "normal") << " mode"
@@ -255,14 +255,14 @@
 
 bool StandardGpioHandler::DiscoverGpios() {
   if (is_discovery_attempted_)
-    return true;
+    return is_discovery_successful_;
 
   is_discovery_attempted_ = true;
 
   // Obtain libudev instance and attach to a dedicated closer.
   struct udev* udev;
   if (!(udev = udev_iface_->New())) {
-    LOG(ERROR) << "failed to obtain libudev instance";
+    LOG(ERROR) << "failed to obtain libudev instance, aborting GPIO discovery";
     return false;
   }
   scoped_ptr<UdevInterface::UdevCloser>
@@ -284,6 +284,7 @@
     }
   }
 
+  is_discovery_successful_ = true;
   return true;
 }
 
@@ -302,13 +303,14 @@
   string file_name = StringPrintf("%s/%s", gpios_[id].dev_path.c_str(),
                                   dev_name);
   if (!fd_->Open(file_name.c_str(), (is_write ? O_WRONLY : O_RDONLY))) {
-    const string err_str = StringPrintf("failed to open %s (%s) for %s",
-                                        file_name.c_str(), gpio_defs_[id].name,
-                                        (is_write ? "writing" : "reading"));
     if (fd_->IsSettingErrno()) {
-      PLOG(ERROR) << err_str;
+      PLOG(ERROR) << "failed to open " << file_name
+                  << " (" << gpio_defs_[id].name << ") for "
+                  << (is_write ? "writing" : "reading");
     } else {
-      LOG(ERROR) << err_str;
+      LOG(ERROR) << "failed to open " << file_name
+                 << " (" << gpio_defs_[id].name << ") for "
+                 << (is_write ? "writing" : "reading");
     }
     return false;
   }