[dev][usb-bus] Use C++ USB request library
TEST: USB works on NUC and in qemu with USB virtual bus
Change-Id: I98aeff7c3c0ae7bbdc6a6620261286da7271d5a1
diff --git a/system/dev/usb/usb-bus/rules.mk b/system/dev/usb/usb-bus/rules.mk
index 9370ceb..e4ef1a1 100644
--- a/system/dev/usb/usb-bus/rules.mk
+++ b/system/dev/usb/usb-bus/rules.mk
@@ -19,7 +19,9 @@
system/ulib/fidl \
system/ulib/sync \
system/ulib/utf_conversion \
+ system/dev/lib/operation \
system/dev/lib/usb \
+ system/ulib/zx \
system/ulib/zxcpp \
MODULE_LIBS := \
diff --git a/system/dev/usb/usb-bus/usb-bus.h b/system/dev/usb/usb-bus/usb-bus.h
index c8675d5..4f6a08e 100644
--- a/system/dev/usb/usb-bus/usb-bus.h
+++ b/system/dev/usb/usb-bus/usb-bus.h
@@ -43,6 +43,8 @@
zx_status_t UsbBusInterfaceReinitializeDevice(uint32_t device_id);
private:
+ DISALLOW_COPY_ASSIGN_AND_MOVE(UsbBus);
+
zx_status_t Init();
zx_status_t GetDeviceId(zx_device_t* device, uint32_t* out);
@@ -53,5 +55,4 @@
fbl::Array<fbl::RefPtr<UsbDevice>> devices_;
};
-
} // namespace usb_bus
diff --git a/system/dev/usb/usb-bus/usb-device.cpp b/system/dev/usb/usb-bus/usb-device.cpp
index ef53a3e..e29df11 100644
--- a/system/dev/usb/usb-bus/usb-device.cpp
+++ b/system/dev/usb/usb-bus/usb-device.cpp
@@ -19,16 +19,6 @@
namespace usb_bus {
-struct UsbRequestInternal {
- // callback to client driver
- usb_request_complete_t complete_cb;
- // for queueing at the usb-bus level
- list_node_t node;
-};
-#define USB_REQ_TO_DEV_INTERNAL(req, size) \
- ((UsbRequestInternal *)((uintptr_t)(req) + (size)))
-#define DEV_INTERNAL_TO_USB_REQ(ctx, size) ((usb_request_t *)((uintptr_t)(ctx) - (size)))
-
// By default we create devices for the interfaces on the first configuration.
// This table allows us to specify a different configuration for certain devices
// based on their VID and PID.
@@ -55,7 +45,7 @@
bool done = false;
while (!done) {
- list_node_t temp_list = LIST_INITIAL_VALUE(temp_list);
+ UnownedRequestQueue temp_queue;
// Wait for new usb requests to complete or for signal to exit this thread.
sync_completion_wait(&callback_thread_completion_, ZX_TIME_INFINITE);
@@ -67,16 +57,13 @@
done = callback_thread_stop_;
// Copy completed requests to a temp list so we can process them outside of our lock.
- list_move(&completed_reqs_, &temp_list);
+ temp_queue = std::move(completed_reqs_);
}
// Call completion callbacks outside of the lock.
- usb_request_t* req;
- UsbRequestInternal* req_int;
- while ((req_int = list_remove_head_type(&temp_list, UsbRequestInternal, node))) {
- req = DEV_INTERNAL_TO_USB_REQ(req_int, parent_req_size_);
- usb_request_complete(req, req->response.status, req->response.actual,
- &req_int->complete_cb);
+ for (auto req = temp_queue.pop(); req; req = temp_queue.pop()) {
+ const auto& response = req->request()->response;
+ req->Complete(response.status, response.actual);
}
}
@@ -111,8 +98,7 @@
}
// move original request to completed_reqs list so it can be completed on the callback_thread
- UsbRequestInternal* req_int = USB_REQ_TO_DEV_INTERNAL(req, parent_req_size_);
- list_add_tail(&completed_reqs_, &req_int->node);
+ completed_reqs_.push(UnownedRequest(req, parent_req_size_));
sync_completion_signal(&callback_thread_completion_);
}
@@ -176,22 +162,21 @@
return ZX_ERR_OUT_OF_RANGE;
}
- usb_request_t* req = NULL;
+ std::optional<Request> req;
bool use_free_list = (length == 0);
if (use_free_list) {
- fbl::AutoLock lock(&free_reqs_lock_);
- req = usb_request_pool_get(&free_reqs_, length);
+ req = free_reqs_.Get(length);
}
- if (req == NULL) {
- auto status = usb_request_alloc(&req, length, 0, req_size_);
+ if (!req.has_value()) {
+ auto status = Request::Alloc(&req, length, 0, parent_req_size_);
if (status != ZX_OK) {
return status;
}
}
// fill in protocol data
- usb_setup_t* setup = &req->setup;
+ usb_setup_t* setup = &req->request()->setup;
setup->bmRequestType = request_type;
setup->bRequest = request;
setup->wValue = value;
@@ -199,14 +184,14 @@
setup->wLength = static_cast<uint16_t>(length);
if (out) {
- if (length > 0 && write_buffer == NULL) {
+ if (length > 0 && write_buffer == nullptr) {
return ZX_ERR_INVALID_ARGS;
}
if (length > write_size) {
return ZX_ERR_BUFFER_TOO_SMALL;
}
} else {
- if (length > 0 && out_read_buffer == NULL) {
+ if (length > 0 && out_read_buffer == nullptr) {
return ZX_ERR_INVALID_ARGS;
}
if (length > read_size) {
@@ -215,24 +200,25 @@
}
if (length > 0 && out) {
- usb_request_copy_to(req, write_buffer, length, 0);
+ req->CopyTo(write_buffer, length, 0);
}
sync_completion_t completion;
- req->header.device_id = device_id_;
- req->header.length = length;
+ req->request()->header.device_id = device_id_;
+ req->request()->header.length = length;
// We call this directly instead of via hci_queue, as it's safe to call our
// own completion callback, and prevents clients getting into odd deadlocks.
usb_request_complete_t complete = {
.callback = ControlComplete,
.ctx = &completion,
};
- hci_.RequestQueue(req, &complete);
+ // Use request() instead of take() since we keep referring to the request below.
+ hci_.RequestQueue(req->request(), &complete);
auto status = sync_completion_wait(&completion, timeout);
if (status == ZX_OK) {
- status = req->response.status;
+ status = req->request()->response.status;
} else if (status == ZX_ERR_TIMED_OUT) {
// cancel transactions and wait for request to be completed
sync_completion_reset(&completion);
@@ -243,22 +229,19 @@
}
}
if (status == ZX_OK && !out) {
+ auto actual = req->request()->response.actual;
if (length > 0) {
- usb_request_copy_from(req, out_read_buffer, req->response.actual, 0);
+ req->CopyFrom(out_read_buffer, actual, 0);
}
- if (out_read_actual != NULL) {
- *out_read_actual = req->response.actual;
+ if (out_read_actual != nullptr) {
+ *out_read_actual = actual;
}
}
if (use_free_list) {
- fbl::AutoLock lock(&free_reqs_lock_);
- if (usb_request_pool_add(&free_reqs_, req) != ZX_OK) {
- zxlogf(TRACE, "Unable to add back request to the free pool\n");
- usb_request_release(req);
- }
+ free_reqs_.Add(std::move(*req));
} else {
- usb_request_release(req);
+ req->Release();
}
return status;
}
@@ -284,19 +267,22 @@
}
void UsbDevice::UsbRequestQueue(usb_request_t* req, const usb_request_complete_t* complete_cb) {
- UsbRequestInternal* req_int = USB_REQ_TO_DEV_INTERNAL(req, parent_req_size_);
- req_int->complete_cb = *complete_cb;
-
req->header.device_id = device_id_;
- // save the existing callback, so we can replace them
- // with our own before passing the request to the HCI driver.
+
+ // Queue to HCI driver with our own completion callback so we can call client's completion
+ // on our own thread, to avoid drivers from deadlocking the HCI driver's interrupt thread.
usb_request_complete_t complete = {
.callback = [](void* ctx, usb_request_t* req) {
static_cast<UsbDevice*>(ctx)->RequestComplete(req);
},
.ctx = this,
};
- hci_.RequestQueue(req, &complete);
+
+ // Save client's callback in private storage.
+ UnownedRequest request(req, *complete_cb, parent_req_size_);
+
+ // Queue with our callback instead.
+ hci_.RequestQueue(request.take(), &complete);
}
usb_speed_t UsbDevice::UsbGetSpeed() {
@@ -541,7 +527,7 @@
}
size_t UsbDevice::UsbGetRequestSize() {
- return req_size_;
+ return UnownedRequest::RequestSize(parent_req_size_);
}
zx_status_t UsbDevice::MsgGetDeviceSpeed(fidl_txn_t* txn) {
@@ -680,10 +666,6 @@
ddk_proto_id_ = ZX_PROTOCOL_USB_DEVICE;
parent_req_size_ = hci_.GetRequestSize();
- req_size_ = parent_req_size_ + sizeof(UsbRequestInternal);
-
- list_initialize(&completed_reqs_);
- usb_request_pool_init(&free_reqs_, parent_req_size_ + offsetof(UsbRequestInternal, node));
// read device descriptor
size_t actual;
diff --git a/system/dev/usb/usb-bus/usb-device.h b/system/dev/usb/usb-bus/usb-device.h
index a4083dc..193cf14 100644
--- a/system/dev/usb/usb-bus/usb-device.h
+++ b/system/dev/usb/usb-bus/usb-device.h
@@ -14,6 +14,7 @@
#include <fbl/ref_ptr.h>
#include <usb/usb-request.h>
#include <lib/sync/completion.h>
+#include <usb/request-cpp.h>
#include <zircon/hw/usb.h>
#include <optional>
@@ -97,6 +98,12 @@
inline usb_speed_t GetSpeed() const { return speed_; }
private:
+ DISALLOW_COPY_ASSIGN_AND_MOVE(UsbDevice);
+
+ using Request = usb::Request<void>;
+ using UnownedRequest = usb::UnownedRequest<void>;
+ using UnownedRequestQueue = usb::UnownedRequestQueue<void>;
+
zx_status_t Init();
int CallbackThread();
@@ -140,16 +147,14 @@
// completion used for signalling callback_thread
sync_completion_t callback_thread_completion_;
// list of requests that need to have client's completion callback called
- list_node_t completed_reqs_ __TA_GUARDED(callback_lock_);
+ UnownedRequestQueue completed_reqs_ __TA_GUARDED(callback_lock_);
// mutex that protects the callback_* members above
fbl::Mutex callback_lock_;
- // pool of requests that can be reused
- usb_request_pool_t free_reqs_ __TA_GUARDED(free_reqs_lock_);
- fbl::Mutex free_reqs_lock_;
+ // Pool of requests USB control requests with zero data.
+ usb::RequestPool<void> free_reqs_;
size_t parent_req_size_;
- size_t req_size_;
};
} // namespace usb_bus
