when a usb device is removed from a port, a hub interrupt is
generated. this causes a hub explore task to be scheduled,
which will find that the device has been removed then detach
the driver via usb_disconnect_port(). so there is some time
between when the device is removed and the driver detachment.
if the kernel tries to access the device after it has been
removed but before the driver is detached, a kernel crash is
likely. also, the kernel does not know the device has been
removed until the driver is detached; config_deactivate() is
never called.
the following patch adds a 'dying' field to struct usbd_device,
and a couple functions, usbd_deactivate() and usbd_is_dying().
usbd_deactivate() sets the new dying field, and usbd_is_dying()
checks whether the dying field is set. usb_disconnect_port()
is changed to call config_deactivate() for devices to be detached
before calling config_detach(), and usbd_deactivate() is added
to the DVACT_DEACTIVATE case for many devices' activate()
functions. also, usbd_is_dying() is either added or replaces
driver specific state variables at places where it would be
good to know if the hardware is still usable.
the idea is to give drivers a chance to quit accessing the
hardware before the driver is detached, so we don't detach
the driver while a request is stalled trying to read/write
nonexistent hardware.
oh, usbd_is_dying() also checks if the underlying bus is
detached, because if the bus is detached, the device has
also been detached. I think this will avoid the crash
described in
http://marc.info/?l=openbsd-bugsm=129050458925460w=2
it seems that that device generates a hub interrupt (the
only thing that causes uhub_explore() to be run) when
detached. on the one hand this makes sense: hub interrupt
means the status of the hub changed. on the other, it's
nonsensical: the hub is gone, we don't need an interrupt to
know that the hub state has changed.
the idea is really from yuo@, in some commit messages from
a couple months ago.
I've been running this diff for quite some time, and it was
actually in snapshots for some time in early October.
thoughts?
p.s. yes, this doesn't convert *all* drivers at this time.
that will happen, I promise.
--
jake...@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org
Index: if_atu.c
===
RCS file: /cvs/src/sys/dev/usb/if_atu.c,v
retrieving revision 1.96
diff -u -p -r1.96 if_atu.c
--- if_atu.c27 Oct 2010 17:51:11 - 1.96
+++ if_atu.c24 Nov 2010 19:53:58 -
@@ -902,7 +902,7 @@ atu_internal_firmware(void *arg)
if (err != 0) {
printf(%s: %s loadfirmware error %d\n,
sc-atu_dev.dv_xname, name, err);
- return;
+ goto fail;
}
ptr = firm;
@@ -918,7 +918,7 @@ atu_internal_firmware(void *arg)
DPRINTF((%s: dfu_getstatus failed!\n,
sc-atu_dev.dv_xname));
free(firm, M_DEVBUF);
- return;
+ goto fail;
}
/* success means state = DnLoadIdle */
state = DFUState_DnLoadIdle;
@@ -940,7 +940,7 @@ atu_internal_firmware(void *arg)
DPRINTF((%s: dfu_dnload failed\n,
sc-atu_dev.dv_xname));
free(firm, M_DEVBUF);
- return;
+ goto fail;
}
ptr += block_size;
@@ -969,14 +969,14 @@ atu_internal_firmware(void *arg)
if (err) {
DPRINTF((%s: dfu_getstatus failed!\n,
sc-atu_dev.dv_xname));
- return;
+ goto fail;
}
DPRINTFN(15, (%s: sending remap\n, sc-atu_dev.dv_xname));
err = atu_usb_request(sc, DFU_REMAP, 0, 0, 0, NULL);
if ((err) (!ISSET(sc-atu_quirk, ATU_QUIRK_NO_REMAP))) {
DPRINTF((%s: remap failed!\n, sc-atu_dev.dv_xname));
- return;
+ goto fail;
}
/* after a lot of trying and measuring I found out the device needs
@@ -989,6 +989,9 @@ atu_internal_firmware(void *arg)
printf(%s: reattaching after firmware upload\n,
sc-atu_dev.dv_xname);
usb_needs_reattach(sc-atu_udev);
+
+fail:
+ usbd_deactivate(sc-atu_udev);
}
void
@@ -1169,7 +1172,7 @@ atu_task(void *arg)
DPRINTFN(10, (%s: atu_task\n, sc-atu_dev.dv_xname));
- if (sc-sc_state != ATU_S_OK)
+ if (usbd_is_dying(sc-atu_udev))
return;
switch (sc-sc_cmd) {
@@ -1259,25 +1262,23 @@ atu_attach(struct device *parent, struct
u_int8_tmode, channel;
int i;
- sc-sc_state = ATU_S_UNCONFIG;
+
