From 82e228a715d595ea6572e45ebc20b103150eaa94 Mon Sep 17 00:00:00 2001 From: Christian Prochaska Date: Mon, 17 Oct 2016 14:13:51 +0200 Subject: [PATCH] usb_drv: raw session fixes - use the correct memory free functions on errors - report packet submit errors - rename 'Usb::Packet_descriptor::transfer.timeout' as 'Usb::Packet_descriptor::transfer.polling_interval' Fixes #2135 --- repos/dde_linux/src/lib/usb/raw/raw.cc | 45 +++++++++++++++---- .../dde_linux/src/server/usb_terminal/main.cc | 4 +- repos/libports/src/lib/qemu-usb/host.cc | 22 ++------- repos/os/include/usb/usb.h | 17 +++---- repos/os/include/usb_session/usb_session.h | 7 ++- repos/os/src/drivers/usb_block/main.cc | 8 ++-- 6 files changed, 60 insertions(+), 43 deletions(-) diff --git a/repos/dde_linux/src/lib/usb/raw/raw.cc b/repos/dde_linux/src/lib/usb/raw/raw.cc index 856f99be5..ca1b5bc18 100644 --- a/repos/dde_linux/src/lib/usb/raw/raw.cc +++ b/repos/dde_linux/src/lib/usb/raw/raw.cc @@ -297,7 +297,8 @@ class Usb::Worker urb *bulk_urb = usb_alloc_urb(0, GFP_KERNEL); if (!bulk_urb) { error("Failed to allocate bulk URB"); - kfree(buf); + dma_free(buf); + p.error = Usb::Packet_descriptor::SUBMIT_ERROR; return false; } @@ -308,8 +309,15 @@ class Usb::Worker usb_fill_bulk_urb(bulk_urb, _device->udev, pipe, buf, p.size(), _async_complete, data); - if (usb_submit_urb(bulk_urb, GFP_KERNEL)) - error("Failed to submit URB"); + int ret = usb_submit_urb(bulk_urb, GFP_KERNEL); + if (ret != 0) { + error("Failed to submit URB, error: ", ret); + p.error = Usb::Packet_descriptor::SUBMIT_ERROR; + kfree(data); + usb_free_urb(bulk_urb); + dma_free(buf); + return false; + } return true; } @@ -332,7 +340,8 @@ class Usb::Worker urb *irq_urb = usb_alloc_urb(0, GFP_KERNEL); if (!irq_urb) { error("Failed to allocate interrupt URB"); - kfree(buf); + dma_free(buf); + p.error = Usb::Packet_descriptor::SUBMIT_ERROR; return false; } @@ -340,11 +349,29 @@ class Usb::Worker data->packet = p; data->worker = this; - usb_fill_int_urb(irq_urb, _device->udev, pipe, buf, p.size(), - _async_complete, data, p.transfer.timeout); + int polling_interval; - if (usb_submit_urb(irq_urb, GFP_KERNEL)) - error("Failed to submit URB"); + if (p.transfer.polling_interval == Usb::Packet_descriptor::DEFAULT_POLLING_INTERVAL) { + + usb_host_endpoint *ep = read ? _device->udev->ep_in[p.transfer.ep & 0x0f] + : _device->udev->ep_out[p.transfer.ep & 0x0f]; + polling_interval = ep->desc.bInterval; + + } else + polling_interval = p.transfer.polling_interval; + + usb_fill_int_urb(irq_urb, _device->udev, pipe, buf, p.size(), + _async_complete, data, polling_interval); + + int ret = usb_submit_urb(irq_urb, GFP_KERNEL); + if (ret != 0) { + error("Failed to submit URB, error: ", ret); + p.error = Usb::Packet_descriptor::SUBMIT_ERROR; + kfree(data); + usb_free_urb(irq_urb); + dma_free(buf); + return false; + } return true; } @@ -436,10 +463,12 @@ class Usb::Worker case Packet_descriptor::BULK: if (_bulk(p, !!(p.transfer.ep & USB_DIR_IN))) continue; + break; case Packet_descriptor::IRQ: if (_irq(p, !!(p.transfer.ep & USB_DIR_IN))) continue; + break; case Packet_descriptor::ALT_SETTING: _alt_setting(p); diff --git a/repos/dde_linux/src/server/usb_terminal/main.cc b/repos/dde_linux/src/server/usb_terminal/main.cc index 5f816231d..9093d2728 100644 --- a/repos/dde_linux/src/server/usb_terminal/main.cc +++ b/repos/dde_linux/src/server/usb_terminal/main.cc @@ -139,7 +139,7 @@ struct Usb::Pl2303_driver : Completion Packet_descriptor p = iface.alloc(num_bytes); memcpy(iface.content(p), dst, num_bytes); - iface.bulk_transfer(p, ep, 0, false, this); + iface.bulk_transfer(p, ep, false, this); return num_bytes; } @@ -196,7 +196,7 @@ struct Usb::Pl2303_driver : Completion Usb::Endpoint &ep = iface.endpoint(IN); for (int i = 0; i < PACKET_BUFFER; i++) { p = iface.alloc(ep.max_packet_size); - iface.bulk_transfer(p, ep, 0, false, this); + iface.bulk_transfer(p, ep, false, this); } /* send signal to terminal client */ diff --git a/repos/libports/src/lib/qemu-usb/host.cc b/repos/libports/src/lib/qemu-usb/host.cc index 0213da542..ef74402d7 100644 --- a/repos/libports/src/lib/qemu-usb/host.cc +++ b/repos/libports/src/lib/qemu-usb/host.cc @@ -401,7 +401,6 @@ static void usb_host_handle_data(USBDevice *udev, USBPacket *p) Usb_host_device *dev = (Usb_host_device *)d->data; Genode::size_t size = 0; - unsigned timeout = 0; Usb::Packet_descriptor::Type type = Usb::Packet_descriptor::BULK; switch (usb_ep_get_type(udev, p->pid, p->ep->nr)) { @@ -412,21 +411,6 @@ static void usb_host_handle_data(USBDevice *udev, USBPacket *p) case USB_ENDPOINT_XFER_INT: type = Usb::Packet_descriptor::IRQ; size = p->iov.size; - - /* values match usb_drv */ - switch (udev->speed) { - case USB_SPEED_SUPER: - timeout = 1<<15; - break; - case USB_SPEED_HIGH: - timeout = 1<<13; - break; - case USB_SPEED_FULL: - case USB_SPEED_LOW: - timeout = 1<<7; - break; - default: break; - } break; default: error("not supported data request"); @@ -436,9 +420,9 @@ static void usb_host_handle_data(USBDevice *udev, USBPacket *p) bool const in = p->pid == USB_TOKEN_IN; Usb::Packet_descriptor packet = dev->alloc_packet(size); - packet.type = type; - packet.transfer.ep = p->ep->nr | (in ? USB_DIR_IN : 0); - packet.transfer.timeout = timeout; + packet.type = type; + packet.transfer.ep = p->ep->nr | (in ? USB_DIR_IN : 0); + packet.transfer.polling_interval = Usb::Packet_descriptor::DEFAULT_POLLING_INTERVAL; if (!in) { char * const content = dev->usb_raw.source()->packet_content(packet); diff --git a/repos/os/include/usb/usb.h b/repos/os/include/usb/usb.h index 11be15b58..669d6bb7d 100644 --- a/repos/os/include/usb/usb.h +++ b/repos/os/include/usb/usb.h @@ -320,7 +320,7 @@ class Usb::Interface : public Meta_data else _handler.submit(p); } - void bulk_transfer(Packet_descriptor &p, Endpoint &ep, int timeout, + void bulk_transfer(Packet_descriptor &p, Endpoint &ep, bool block = true, Completion *c = nullptr) { _check(); @@ -331,14 +331,15 @@ class Usb::Interface : public Meta_data p.type = Usb::Packet_descriptor::BULK; p.succeded = false; p.transfer.ep = ep.address; - p.transfer.timeout = timeout; p.completion = c; if(block) Sync_completion sync(_handler, p); else _handler.submit(p); } - void interrupt_transfer(Packet_descriptor &p, Endpoint &ep, int timeout, + void interrupt_transfer(Packet_descriptor &p, Endpoint &ep, + int polling_interval = + Usb::Packet_descriptor::DEFAULT_POLLING_INTERVAL, bool block = true, Completion *c = nullptr) { _check(); @@ -346,11 +347,11 @@ class Usb::Interface : public Meta_data if (!ep.is_interrupt()) throw Session::Invalid_endpoint(); - p.type = Usb::Packet_descriptor::IRQ; - p.succeded = false; - p.transfer.ep = ep.address; - p.transfer.timeout = timeout; - p.completion = c; + p.type = Usb::Packet_descriptor::IRQ; + p.succeded = false; + p.transfer.ep = ep.address; + p.transfer.polling_interval = polling_interval; + p.completion = c; if(block) Sync_completion sync(_handler, p); else _handler.submit(p); diff --git a/repos/os/include/usb_session/usb_session.h b/repos/os/include/usb_session/usb_session.h index 335d59e91..d1913d6c2 100644 --- a/repos/os/include/usb_session/usb_session.h +++ b/repos/os/include/usb_session/usb_session.h @@ -35,6 +35,9 @@ struct Usb::Packet_descriptor : Genode::Packet_descriptor { enum Type { STRING, CTRL, BULK, IRQ, ALT_SETTING, CONFIG, RELEASE_IF }; + /* use the polling interval stated in the endpoint descriptor */ + enum { DEFAULT_POLLING_INTERVAL = -1 }; + Type type; bool succeded = false; Completion *completion = nullptr; @@ -61,7 +64,7 @@ struct Usb::Packet_descriptor : Genode::Packet_descriptor { uint8_t ep; int actual_size; /* returned */ - int timeout; + int polling_interval; /* for interrupt transfers */ } transfer; struct @@ -76,7 +79,7 @@ struct Usb::Packet_descriptor : Genode::Packet_descriptor }; }; - enum Error { NO_ERROR, STALL_ERROR }; + enum Error { NO_ERROR, STALL_ERROR, SUBMIT_ERROR }; Error error = NO_ERROR; diff --git a/repos/os/src/drivers/usb_block/main.cc b/repos/os/src/drivers/usb_block/main.cc index 3ca2b6c70..efffb899b 100644 --- a/repos/os/src/drivers/usb_block/main.cc +++ b/repos/os/src/drivers/usb_block/main.cc @@ -304,7 +304,7 @@ struct Usb::Block_driver : Usb::Completion, Usb::Endpoint &ep = iface.endpoint(OUT); Usb::Packet_descriptor p = iface.alloc(CBW_VALID_SIZE); memcpy(iface.content(p), cb, CBW_VALID_SIZE); - iface.bulk_transfer(p, ep, 0, block, &c); + iface.bulk_transfer(p, ep, block, &c); } /** @@ -316,7 +316,7 @@ struct Usb::Block_driver : Usb::Completion, Usb::Interface &iface = device.interface(active_interface); Usb::Endpoint &ep = iface.endpoint(IN); Usb::Packet_descriptor p = iface.alloc(CSW_VALID_SIZE); - iface.bulk_transfer(p, ep, 0, block, &c); + iface.bulk_transfer(p, ep, block, &c); } /** @@ -327,7 +327,7 @@ struct Usb::Block_driver : Usb::Completion, Usb::Interface &iface = device.interface(active_interface); Usb::Endpoint &ep = iface.endpoint(IN); Usb::Packet_descriptor p = iface.alloc(size); - iface.bulk_transfer(p, ep, 0, block, &c); + iface.bulk_transfer(p, ep, block, &c); } /** @@ -548,7 +548,7 @@ struct Usb::Block_driver : Usb::Completion, if (!req.read) memcpy(iface.content(p), req.buffer, req.size); - iface.bulk_transfer(p, ep, 0, false, this); + iface.bulk_transfer(p, ep, false, this); return true; }