From 4b5d4da6f6a863638c8100cde5af834a83ac8956 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 7 Jun 2017 12:11:19 -0700 Subject: [PATCH] adb: remove SendConnectOnHost. This logic appears to be racy, and it shouldn't actually be needed, if our devices follow the USB spec. Use libusb_set_interface_alt_setting on device initialization as well, to add one more thing that should reset the data toggles. Bug: http://b/32952319 Test: python test_device.py Change-Id: I392198af3d72c524b893e5056afa2b4617cea49c --- adb/adb.cpp | 15 +-------------- adb/adb.h | 3 --- adb/client/usb_libusb.cpp | 7 +++++++ adb/transport.cpp | 7 ------- adb/transport.h | 2 -- adb/transport_usb.cpp | 10 +--------- 6 files changed, 9 insertions(+), 35 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index 8c24bbba3..6b30be884 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -257,19 +257,6 @@ void send_connect(atransport* t) { send_packet(cp, t); } -#if ADB_HOST - -void SendConnectOnHost(atransport* t) { - // Send an empty message before A_CNXN message. This is because the data toggle of the ep_out on - // host and ep_in on device may not be the same. - apacket* p = get_apacket(); - CHECK(p); - send_packet(p, t); - send_connect(t); -} - -#endif - // qual_overwrite is used to overwrite a qualifier string. dst is a // pointer to a char pointer. It is assumed that if *dst is non-NULL, it // was malloc'ed and needs to freed. *dst will be set to a dup of src. @@ -370,7 +357,7 @@ void handle_packet(apacket *p, atransport *t) if (p->msg.arg0){ send_packet(p, t); #if ADB_HOST - SendConnectOnHost(t); + send_connect(t); #endif } else { t->SetConnectionState(kCsOffline); diff --git a/adb/adb.h b/adb/adb.h index 6a9897f63..a4d233ead 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -224,9 +224,6 @@ void handle_online(atransport *t); void handle_offline(atransport *t); void send_connect(atransport *t); -#if ADB_HOST -void SendConnectOnHost(atransport* t); -#endif void parse_banner(const std::string&, atransport* t); diff --git a/adb/client/usb_libusb.cpp b/adb/client/usb_libusb.cpp index 7025f283c..81201995a 100644 --- a/adb/client/usb_libusb.cpp +++ b/adb/client/usb_libusb.cpp @@ -333,6 +333,13 @@ static void process_device(libusb_device* device) { return; } + rc = libusb_set_interface_alt_setting(handle.get(), interface_num, 0); + if (rc != 0) { + LOG(WARNING) << "failed to set interface alt setting for device '" << device_serial + << "'" << libusb_error_name(rc); + return; + } + for (uint8_t endpoint : {bulk_in, bulk_out}) { rc = libusb_clear_halt(handle.get(), endpoint); if (rc != 0) { diff --git a/adb/transport.cpp b/adb/transport.cpp index 34cc026a5..089a1ecf4 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -1101,11 +1101,4 @@ std::shared_ptr atransport::NextKey() { keys_.pop_front(); return result; } -bool atransport::SetSendConnectOnError() { - if (has_send_connect_on_error_) { - return false; - } - has_send_connect_on_error_ = true; - return true; -} #endif diff --git a/adb/transport.h b/adb/transport.h index 79e307561..8c101fdc5 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -122,7 +122,6 @@ class atransport { #if ADB_HOST std::shared_ptr NextKey(); - bool SetSendConnectOnError(); #endif char token[TOKEN_SIZE] = {}; @@ -181,7 +180,6 @@ private: std::atomic connection_state_; #if ADB_HOST std::deque> keys_; - bool has_send_connect_on_error_ = false; #endif DISALLOW_COPY_AND_ASSIGN(atransport); diff --git a/adb/transport_usb.cpp b/adb/transport_usb.cpp index 6768d31a1..fdecccfc3 100644 --- a/adb/transport_usb.cpp +++ b/adb/transport_usb.cpp @@ -103,13 +103,6 @@ static int remote_read(apacket* p, atransport* t) { err_msg: p->msg.command = 0; - if (t->GetConnectionState() == kCsOffline) { - // If the data toggle of ep_out on device and ep_in on host are not the same, we may receive - // an error message. In this case, resend one A_CNXN message to connect the device. - if (t->SetSendConnectOnError()) { - SendConnectOnHost(t); - } - } return 0; } @@ -162,8 +155,7 @@ static int remote_write(apacket *p, atransport *t) return 0; } -static void remote_close(atransport *t) -{ +static void remote_close(atransport* t) { usb_close(t->usb); t->usb = 0; }