uhub(4) USB device (dis)connect
Device below changes uhub(4) to no longer check *all* the ports status
of *all* the hubs on bus when one of those hubs requests an explore.
This has the nice effect of reducing the delay/bus traffic when a port
status change is detected which reduces (avoid) some timing related
problems when connection devices.
To do so, we only query the status of the ports having reported a status
changed in the interrupt.
I'd appreciate tests on various HC/archs.
Comments, ok?
Index: uhub.c
===
RCS file: /cvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.74
diff -u -p -r1.74 uhub.c
--- uhub.c 1 Oct 2014 08:29:01 - 1.74
+++ uhub.c 31 Oct 2014 22:00:38 -
@@ -57,8 +57,11 @@ struct uhub_softc {
struct device sc_dev; /* base device */
struct usbd_device *sc_hub;/* USB device */
struct usbd_pipe*sc_ipipe; /* interrupt pipe */
- u_int8_t*sc_statusbuf; /* per port status buffer */
- size_t sc_statuslen; /* status bufferlen */
+
+ uint32_t sc_status; /* status from last interrupt */
+ uint8_t *sc_statusbuf; /* per port status buffer */
+ size_t sc_statuslen; /* status bufferlen */
+
u_char sc_running;
};
#define UHUB_PROTO(sc) ((sc)-sc_hub-ddesc.bDeviceProtocol)
@@ -349,18 +352,24 @@ uhub_explore(struct usbd_device *dev)
for (port = 1; port = dev-hub-nports; port++) {
up = dev-hub-ports[port-1];
- err = usbd_get_port_status(dev, port, up-status);
- if (err) {
- DPRINTF(%s: get port %d status failed, error=%s\n,
- sc-sc_dev.dv_xname, port, usbd_errstr(err));
- continue;
- }
- status = UGETW(up-status.wPortStatus);
- change = UGETW(up-status.wPortChange);
+
reconnect = up-reattach;
up-reattach = 0;
- DPRINTF(%s: port %d status=0x%04x change=0x%04x\n,
- sc-sc_dev.dv_xname, port, status, change);
+ change = 0;
+ status = 0;
+
+ if (sc-sc_status (1 port)) {
+ sc-sc_status = (1 port);
+
+ if (usbd_get_port_status(dev, port, up-status))
+ continue;
+
+ status = UGETW(up-status.wPortStatus);
+ change = UGETW(up-status.wPortChange);
+ DPRINTF(%s: port %d status=0x%04x change=0x%04x\n,
+ sc-sc_dev.dv_xname, port, status, change);
+ }
+
if (change UPS_C_PORT_ENABLED) {
usbd_clear_port_feature(dev, port, UHF_C_PORT_ENABLE);
if (change UPS_C_CONNECT_STATUS) {
@@ -556,6 +565,8 @@ void
uhub_intr(struct usbd_xfer *xfer, void *addr, usbd_status status)
{
struct uhub_softc *sc = addr;
+ uint32_t stats;
+ int i;
if (usbd_is_dying(sc-sc_hub))
return;
@@ -563,6 +574,11 @@ uhub_intr(struct usbd_xfer *xfer, void *
DPRINTF(%s: intr status=%d\n, sc-sc_dev.dv_xname, status);
if (status == USBD_STALLED)
usbd_clear_endpoint_stall_async(sc-sc_ipipe);
- else if (status == USBD_NORMAL_COMPLETION)
+ else if (status == USBD_NORMAL_COMPLETION) {
+ for (i = 0; i xfer-actlen; i++)
+ stats |= (uint32_t)(xfer-buffer[i]) (i * 8);
+ sc-sc_status = stats;
+
usb_needs_explore(sc-sc_hub, 0);
+ }
}
Index: usbdivar.h
===
RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.64
diff -u -p -r1.64 usbdivar.h
--- usbdivar.h 31 Oct 2014 12:43:33 - 1.64
+++ usbdivar.h 31 Oct 2014 21:12:55 -
@@ -182,7 +182,7 @@ struct usbd_pipe {
struct usbd_xfer {
struct usbd_pipe *pipe;
void *priv;
- void *buffer;
+ char *buffer;
u_int32_t length;
u_int32_t actlen;
u_int16_t flags;
Re: uhub(4) USB device (dis)connect
On Fri, Oct 31, 2014 at 11:19:22PM +0100, Martin Pieuchot wrote:
Device below changes uhub(4) to no longer check *all* the ports status
of *all* the hubs on bus when one of those hubs requests an explore.
This has the nice effect of reducing the delay/bus traffic when a port
status change is detected which reduces (avoid) some timing related
problems when connection devices.
To do so, we only query the status of the ports having reported a status
changed in the interrupt.
I'd appreciate tests on various HC/archs.
Comments, ok?
wunderbar!
Index: uhub.c
===
RCS file: /cvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.74
diff -u -p -r1.74 uhub.c
--- uhub.c1 Oct 2014 08:29:01 - 1.74
+++ uhub.c31 Oct 2014 22:00:38 -
@@ -57,8 +57,11 @@ struct uhub_softc {
struct device sc_dev; /* base device */
struct usbd_device *sc_hub;/* USB device */
struct usbd_pipe*sc_ipipe; /* interrupt pipe */
- u_int8_t*sc_statusbuf; /* per port status buffer */
- size_t sc_statuslen; /* status bufferlen */
+
+ uint32_t sc_status; /* status from last interrupt */
+ uint8_t *sc_statusbuf; /* per port status buffer */
+ size_t sc_statuslen; /* status bufferlen */
+
u_char sc_running;
};
#define UHUB_PROTO(sc) ((sc)-sc_hub-ddesc.bDeviceProtocol)
@@ -349,18 +352,24 @@ uhub_explore(struct usbd_device *dev)
for (port = 1; port = dev-hub-nports; port++) {
up = dev-hub-ports[port-1];
- err = usbd_get_port_status(dev, port, up-status);
- if (err) {
- DPRINTF(%s: get port %d status failed, error=%s\n,
- sc-sc_dev.dv_xname, port, usbd_errstr(err));
- continue;
- }
- status = UGETW(up-status.wPortStatus);
- change = UGETW(up-status.wPortChange);
+
reconnect = up-reattach;
up-reattach = 0;
- DPRINTF(%s: port %d status=0x%04x change=0x%04x\n,
- sc-sc_dev.dv_xname, port, status, change);
+ change = 0;
+ status = 0;
+
+ if (sc-sc_status (1 port)) {
+ sc-sc_status = (1 port);
+
+ if (usbd_get_port_status(dev, port, up-status))
+ continue;
+
+ status = UGETW(up-status.wPortStatus);
+ change = UGETW(up-status.wPortChange);
+ DPRINTF(%s: port %d status=0x%04x change=0x%04x\n,
+ sc-sc_dev.dv_xname, port, status, change);
+ }
+
if (change UPS_C_PORT_ENABLED) {
usbd_clear_port_feature(dev, port, UHF_C_PORT_ENABLE);
if (change UPS_C_CONNECT_STATUS) {
@@ -556,6 +565,8 @@ void
uhub_intr(struct usbd_xfer *xfer, void *addr, usbd_status status)
{
struct uhub_softc *sc = addr;
+ uint32_t stats;
+ int i;
if (usbd_is_dying(sc-sc_hub))
return;
@@ -563,6 +574,11 @@ uhub_intr(struct usbd_xfer *xfer, void *
DPRINTF(%s: intr status=%d\n, sc-sc_dev.dv_xname, status);
if (status == USBD_STALLED)
usbd_clear_endpoint_stall_async(sc-sc_ipipe);
- else if (status == USBD_NORMAL_COMPLETION)
+ else if (status == USBD_NORMAL_COMPLETION) {
+ for (i = 0; i xfer-actlen; i++)
+ stats |= (uint32_t)(xfer-buffer[i]) (i * 8);
+ sc-sc_status = stats;
+
usb_needs_explore(sc-sc_hub, 0);
+ }
}
Index: usbdivar.h
===
RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.64
diff -u -p -r1.64 usbdivar.h
--- usbdivar.h31 Oct 2014 12:43:33 - 1.64
+++ usbdivar.h31 Oct 2014 21:12:55 -
@@ -182,7 +182,7 @@ struct usbd_pipe {
struct usbd_xfer {
struct usbd_pipe *pipe;
void *priv;
- void *buffer;
+ char *buffer;
u_int32_t length;
u_int32_t actlen;
u_int16_t flags;
--
Antoine
Re: uhub(4) USB device (dis)connect
On 31/10/14(Fri) 23:19, Martin Pieuchot wrote:
Device below changes uhub(4) to no longer check *all* the ports status
of *all* the hubs on bus when one of those hubs requests an explore.
This has the nice effect of reducing the delay/bus traffic when a port
status change is detected which reduces (avoid) some timing related
problems when connection devices.
To do so, we only query the status of the ports having reported a status
changed in the interrupt.
I'd appreciate tests on various HC/archs.
Comments, ok?
Updated diff that correctly deals with devices asking for a reconnect
and xhci(4)'s root hub reporting only one port change per interrupt.
Index: uhub.c
===
RCS file: /cvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.74
diff -u -p -r1.74 uhub.c
--- uhub.c 1 Oct 2014 08:29:01 - 1.74
+++ uhub.c 1 Nov 2014 00:21:04 -
@@ -57,8 +57,11 @@ struct uhub_softc {
struct device sc_dev; /* base device */
struct usbd_device *sc_hub;/* USB device */
struct usbd_pipe*sc_ipipe; /* interrupt pipe */
- u_int8_t*sc_statusbuf; /* per port status buffer */
- size_t sc_statuslen; /* status bufferlen */
+
+ uint32_t sc_status; /* status from last interrupt */
+ uint8_t *sc_statusbuf; /* per port status buffer */
+ size_t sc_statuslen; /* status bufferlen */
+
u_char sc_running;
};
#define UHUB_PROTO(sc) ((sc)-sc_hub-ddesc.bDeviceProtocol)
@@ -349,18 +352,24 @@ uhub_explore(struct usbd_device *dev)
for (port = 1; port = dev-hub-nports; port++) {
up = dev-hub-ports[port-1];
- err = usbd_get_port_status(dev, port, up-status);
- if (err) {
- DPRINTF(%s: get port %d status failed, error=%s\n,
- sc-sc_dev.dv_xname, port, usbd_errstr(err));
- continue;
- }
- status = UGETW(up-status.wPortStatus);
- change = UGETW(up-status.wPortChange);
+
reconnect = up-reattach;
up-reattach = 0;
- DPRINTF(%s: port %d status=0x%04x change=0x%04x\n,
- sc-sc_dev.dv_xname, port, status, change);
+ change = 0;
+ status = 0;
+
+ if ((sc-sc_status (1 port)) || reconnect) {
+ sc-sc_status = ~(1 port);
+
+ if (usbd_get_port_status(dev, port, up-status))
+ continue;
+
+ status = UGETW(up-status.wPortStatus);
+ change = UGETW(up-status.wPortChange);
+ DPRINTF(%s: port %d status=0x%04x change=0x%04x\n,
+ sc-sc_dev.dv_xname, port, status, change);
+ }
+
if (change UPS_C_PORT_ENABLED) {
usbd_clear_port_feature(dev, port, UHF_C_PORT_ENABLE);
if (change UPS_C_CONNECT_STATUS) {
@@ -393,7 +402,7 @@ uhub_explore(struct usbd_device *dev)
/* We have a connect status change, handle it. */
usbd_clear_port_feature(dev, port, UHF_C_PORT_CONNECTION);
- /*usbd_clear_port_feature(dev, port, UHF_C_PORT_ENABLE);*/
+
/*
* If there is already a device on the port the change status
* must mean that is has disconnected. Looking at the
@@ -556,13 +565,21 @@ void
uhub_intr(struct usbd_xfer *xfer, void *addr, usbd_status status)
{
struct uhub_softc *sc = addr;
+ uint32_t stats;
+ int i;
if (usbd_is_dying(sc-sc_hub))
return;
DPRINTF(%s: intr status=%d\n, sc-sc_dev.dv_xname, status);
+
if (status == USBD_STALLED)
usbd_clear_endpoint_stall_async(sc-sc_ipipe);
- else if (status == USBD_NORMAL_COMPLETION)
+ else if (status == USBD_NORMAL_COMPLETION) {
+ for (i = 0; i xfer-actlen; i++)
+ stats |= (uint32_t)(xfer-buffer[i]) (i * 8);
+ sc-sc_status |= stats;
+
usb_needs_explore(sc-sc_hub, 0);
+ }
}
Index: usbdivar.h
===
RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.64
diff -u -p -r1.64 usbdivar.h
--- usbdivar.h 31 Oct 2014 12:43:33 - 1.64
+++ usbdivar.h 31 Oct 2014 21:12:55 -
@@ -182,7 +182,7 @@ struct usbd_pipe {
struct usbd_xfer {
struct usbd_pipe *pipe;
void *priv;
- void *buffer;
+ char *buffer;
u_int32_t length;
u_int32_t actlen;
u_int16_t
