Module Name: src
Committed By: mrg
Date: Thu Nov 24 22:12:52 UTC 2011
Modified Files:
src/sys/dev/usb: umidi.c umidivar.h
Log Message:
several steps towards making umidi appear to work:
- split out_solicit() into locked and unlocked front end, and use the
unlocked version from the softintr
- kill sc_intr_lock, midi doesn't really use it (XXX finish this?
change this? in the midi* code)
- convert tsleep()/wakeup() to cv
- move some free's out of the lock
- KASSERT() lock is held in a few more places
TODO:
- malloc -> kmem
- crashes in midiclose(), doesn't actually play things to the right
device. "midiplay -d midi1 -xv" plays out my midi@pcppi speaker,
and then the above crash. clearly something is calling the wrong
sub-device callbacks!
To generate a diff of this commit:
cvs rdiff -u -r1.46 -r1.47 src/sys/dev/usb/umidi.c
cvs rdiff -u -r1.15 -r1.16 src/sys/dev/usb/umidivar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/usb/umidi.c
diff -u src/sys/dev/usb/umidi.c:1.46 src/sys/dev/usb/umidi.c:1.47
--- src/sys/dev/usb/umidi.c:1.46 Wed Nov 23 23:50:46 2011
+++ src/sys/dev/usb/umidi.c Thu Nov 24 22:12:51 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: umidi.c,v 1.46 2011/11/23 23:50:46 mrg Exp $ */
+/* $NetBSD: umidi.c,v 1.47 2011/11/24 22:12:51 mrg Exp $ */
/*
* Copyright (c) 2001 The NetBSD Foundation, Inc.
* All rights reserved.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: umidi.c,v 1.46 2011/11/23 23:50:46 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: umidi.c,v 1.47 2011/11/24 22:12:51 mrg Exp $");
#include <sys/types.h>
#include <sys/param.h>
@@ -125,6 +125,7 @@ static int out_jack_output(struct umidi_
static void in_intr(usbd_xfer_handle, usbd_private_handle, usbd_status);
static void out_intr(usbd_xfer_handle, usbd_private_handle, usbd_status);
static void out_solicit(void *); /* struct umidi_endpoint* for softintr */
+static void out_solicit_locked(void *); /* pre-locked version */
const struct midi_hw_if umidi_hw_if = {
@@ -201,18 +202,18 @@ umidi_attach(device_t parent, device_t s
aprint_normal_dev(self, "");
umidi_print_quirk(sc->sc_quirk);
- mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_NONE);
- mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_USB);
+ mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_USB);
+ cv_init(&sc->sc_cv, "umidopcl");
KERNEL_LOCK(1, curlwp);
err = alloc_all_endpoints(sc);
- if (err!=USBD_NORMAL_COMPLETION) {
+ if (err != USBD_NORMAL_COMPLETION) {
aprint_error_dev(self,
"alloc_all_endpoints failed. (err=%d)\n", err);
goto error;
}
err = alloc_all_jacks(sc);
- if (err!=USBD_NORMAL_COMPLETION) {
+ if (err != USBD_NORMAL_COMPLETION) {
free_all_endpoints(sc);
aprint_error_dev(self, "alloc_all_jacks failed. (err=%d)\n",
err);
@@ -222,7 +223,7 @@ umidi_attach(device_t parent, device_t s
sc->sc_out_num_jacks, sc->sc_in_num_jacks);
err = assign_all_jacks_automatically(sc);
- if (err!=USBD_NORMAL_COMPLETION) {
+ if (err != USBD_NORMAL_COMPLETION) {
unbind_all_jacks(sc);
free_all_jacks(sc);
free_all_endpoints(sc);
@@ -231,7 +232,7 @@ umidi_attach(device_t parent, device_t s
goto error;
}
err = attach_all_mididevs(sc);
- if (err!=USBD_NORMAL_COMPLETION) {
+ if (err != USBD_NORMAL_COMPLETION) {
free_all_jacks(sc);
free_all_endpoints(sc);
aprint_error_dev(self,
@@ -293,19 +294,21 @@ umidi_detach(device_t self, int flags)
DPRINTFN(1,("umidi_detach\n"));
- KERNEL_LOCK(1, curlwp);
+ mutex_enter(&sc->sc_lock);
sc->sc_dying = 1;
detach_all_mididevs(sc, flags);
free_all_mididevs(sc);
free_all_jacks(sc);
free_all_endpoints(sc);
+ mutex_exit(&sc->sc_lock);
+ KERNEL_LOCK(1, curlwp);
usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->sc_udev,
sc->sc_dev);
KERNEL_UNLOCK_ONE(curlwp);
mutex_destroy(&sc->sc_lock);
- mutex_destroy(&sc->sc_intr_lock);
+ cv_destroy(&sc->sc_cv);
return 0;
}
@@ -444,12 +447,12 @@ umidi_getinfo(void *addr, struct midi_in
}
static void
-umidi_get_locks(void *addr, kmutex_t **intr, kmutex_t **thread)
+umidi_get_locks(void *addr, kmutex_t **thread, kmutex_t **intr)
{
struct umidi_mididev *mididev = addr;
struct umidi_softc *sc = mididev->sc;
- *intr = &sc->sc_intr_lock;
+ *intr = NULL;
*thread = &sc->sc_lock;
}
@@ -912,23 +915,23 @@ alloc_all_jacks(struct umidi_softc *sc)
sc->sc_in_num_jacks ? sc->sc_jacks+sc->sc_out_num_jacks : NULL;
jack = &sc->sc_out_jacks[0];
- for (i=0; i<sc->sc_out_num_jacks; i++) {
+ for (i = 0; i < sc->sc_out_num_jacks; i++) {
jack->opened = 0;
jack->binded = 0;
jack->arg = NULL;
jack->u.out.intr = NULL;
jack->midiman_ppkt = NULL;
- if ( sc->cblnums_global )
+ if (sc->cblnums_global)
jack->cable_number = i;
jack++;
}
jack = &sc->sc_in_jacks[0];
- for (i=0; i<sc->sc_in_num_jacks; i++) {
+ for (i = 0; i < sc->sc_in_num_jacks; i++) {
jack->opened = 0;
jack->binded = 0;
jack->arg = NULL;
jack->u.in.intr = NULL;
- if ( sc->cblnums_global )
+ if (sc->cblnums_global)
jack->cable_number = i;
jack++;
}
@@ -936,12 +939,12 @@ alloc_all_jacks(struct umidi_softc *sc)
/* assign each jacks to each endpoints */
jack = &sc->sc_out_jacks[0];
ep = &sc->sc_out_ep[0];
- for (i=0; i<sc->sc_out_num_endpoints; i++) {
- for (j=0; j<ep->num_jacks; j++) {
+ for (i = 0; i < sc->sc_out_num_endpoints; i++) {
+ for (j = 0; j < ep->num_jacks; j++) {
jack->endpoint = ep;
- if ( cn_spec != NULL )
+ if (cn_spec != NULL)
jack->cable_number = *cn_spec++;
- else if ( !sc->cblnums_global )
+ else if (!sc->cblnums_global)
jack->cable_number = j;
ep->jacks[jack->cable_number] = jack;
jack++;
@@ -950,12 +953,12 @@ alloc_all_jacks(struct umidi_softc *sc)
}
jack = &sc->sc_in_jacks[0];
ep = &sc->sc_in_ep[0];
- for (i=0; i<sc->sc_in_num_endpoints; i++) {
- for (j=0; j<ep->num_jacks; j++) {
+ for (i = 0; i < sc->sc_in_num_endpoints; i++) {
+ for (j = 0; j < ep->num_jacks; j++) {
jack->endpoint = ep;
- if ( cn_spec != NULL )
+ if (cn_spec != NULL)
jack->cable_number = *cn_spec++;
- else if ( !sc->cblnums_global )
+ else if (!sc->cblnums_global)
jack->cable_number = j;
ep->jacks[jack->cable_number] = jack;
jack++;
@@ -969,13 +972,15 @@ alloc_all_jacks(struct umidi_softc *sc)
static void
free_all_jacks(struct umidi_softc *sc)
{
+ struct umidi_jack *jacks;
- mutex_spin_enter(&sc->sc_intr_lock);
- if (sc->sc_out_jacks) {
- free(sc->sc_jacks, M_USBDEV);
- sc->sc_jacks = sc->sc_in_jacks = sc->sc_out_jacks = NULL;
- }
- mutex_spin_exit(&sc->sc_intr_lock);
+ mutex_enter(&sc->sc_lock);
+ jacks = sc->sc_out_jacks;
+ sc->sc_jacks = sc->sc_in_jacks = sc->sc_out_jacks = NULL;
+ mutex_exit(&sc->sc_lock);
+
+ if (jacks)
+ free(jacks, M_USBDEV);
}
static usbd_status
@@ -1020,9 +1025,8 @@ unbind_all_jacks(struct umidi_softc *sc)
int i;
if (sc->sc_mididevs)
- for (i=0; i<sc->sc_num_mididevs; i++) {
+ for (i = 0; i < sc->sc_num_mididevs; i++)
unbind_jacks_from_mididev(&sc->sc_mididevs[i]);
- }
}
static usbd_status
@@ -1045,14 +1049,14 @@ assign_all_jacks_automatically(struct um
else
asg_spec = NULL;
- for (i=0; i<sc->sc_num_mididevs; i++) {
- if ( asg_spec != NULL ) {
- if ( *asg_spec == -1 )
+ for (i = 0; i < sc->sc_num_mididevs; i++) {
+ if (asg_spec != NULL) {
+ if (*asg_spec == -1)
out = NULL;
else
out = &sc->sc_out_jacks[*asg_spec];
++ asg_spec;
- if ( *asg_spec == -1 )
+ if (*asg_spec == -1)
in = NULL;
else
in = &sc->sc_in_jacks[*asg_spec];
@@ -1081,6 +1085,8 @@ open_out_jack(struct umidi_jack *jack, v
umidi_packet_bufp end;
int err;
+ KASSERT(mutex_owned(&sc->sc_lock));
+
if (jack->opened)
return USBD_IN_USE;
@@ -1088,7 +1094,6 @@ open_out_jack(struct umidi_jack *jack, v
jack->u.out.intr = intr;
jack->midiman_ppkt = NULL;
end = ep->buffer + ep->buffer_size / sizeof *ep->buffer;
- mutex_spin_enter(&sc->sc_intr_lock);
jack->opened = 1;
ep->num_open++;
/*
@@ -1097,16 +1102,15 @@ open_out_jack(struct umidi_jack *jack, v
* just incremented num_open, the buffer may be too full to satisfy
* the invariant until a transfer completes, for which we must wait.
*/
- while ( end - ep->next_slot < ep->num_open - ep->num_scheduled ) {
- err = tsleep(ep, PWAIT|PCATCH, "umi op", mstohz(10));
- if ( err ) {
+ while (end - ep->next_slot < ep->num_open - ep->num_scheduled) {
+ err = cv_timedwait_sig(&sc->sc_cv, &sc->sc_lock,
+ mstohz(10));
+ if (err) {
ep->num_open--;
jack->opened = 0;
- mutex_spin_exit(&sc->sc_intr_lock);
return USBD_IOERROR;
}
}
- mutex_spin_exit(&sc->sc_intr_lock);
return USBD_NORMAL_COMPLETION;
}
@@ -1116,6 +1120,9 @@ open_in_jack(struct umidi_jack *jack, vo
{
usbd_status err = USBD_NORMAL_COMPLETION;
struct umidi_endpoint *ep = jack->endpoint;
+ struct umidi_softc *sc = ep->sc;
+
+ KASSERT(mutex_owned(&sc->sc_lock));
if (jack->opened)
return USBD_IN_USE;
@@ -1123,7 +1130,7 @@ open_in_jack(struct umidi_jack *jack, vo
jack->arg = arg;
jack->u.in.intr = intr;
jack->opened = 1;
- if (ep->num_open++==0 && UE_GET_DIR(ep->addr)==UE_DIR_IN) {
+ if (ep->num_open++ == 0 && UE_GET_DIR(ep->addr)==UE_DIR_IN) {
err = start_input_transfer(ep);
if (err != USBD_NORMAL_COMPLETION &&
err != USBD_IN_PROGRESS) {
@@ -1142,21 +1149,22 @@ close_out_jack(struct umidi_jack *jack)
u_int16_t mask;
int err;
+ KASSERT(mutex_owned(&sc->sc_lock));
+
if (jack->opened) {
ep = jack->endpoint;
sc = ep->sc;
mask = 1 << (jack->cable_number);
- mutex_spin_enter(&sc->sc_intr_lock);
- while ( mask & (ep->this_schedule | ep->next_schedule) ) {
- err = tsleep(ep, PWAIT|PCATCH, "umi dr", mstohz(10));
- if ( err )
+ while (mask & (ep->this_schedule | ep->next_schedule)) {
+ err = cv_timedwait_sig(&sc->sc_cv, &sc->sc_lock,
+ mstohz(10));
+ if (err)
break;
}
jack->opened = 0;
jack->endpoint->num_open--;
ep->this_schedule &= ~mask;
ep->next_schedule &= ~mask;
- mutex_spin_exit(&sc->sc_intr_lock);
}
}
@@ -1246,9 +1254,9 @@ attach_all_mididevs(struct umidi_softc *
int i;
if (sc->sc_mididevs)
- for (i=0; i<sc->sc_num_mididevs; i++) {
+ for (i = 0; i < sc->sc_num_mididevs; i++) {
err = attach_mididev(sc, &sc->sc_mididevs[i]);
- if (err!=USBD_NORMAL_COMPLETION)
+ if (err != USBD_NORMAL_COMPLETION)
return err;
}
@@ -1262,9 +1270,9 @@ detach_all_mididevs(struct umidi_softc *
int i;
if (sc->sc_mididevs)
- for (i=0; i<sc->sc_num_mididevs; i++) {
+ for (i = 0; i < sc->sc_num_mididevs; i++) {
err = detach_mididev(&sc->sc_mididevs[i], flags);
- if (err!=USBD_NORMAL_COMPLETION)
+ if (err != USBD_NORMAL_COMPLETION)
return err;
}
@@ -1277,7 +1285,7 @@ deactivate_all_mididevs(struct umidi_sof
int i;
if (sc->sc_mididevs) {
- for (i=0; i<sc->sc_num_mididevs; i++)
+ for (i = 0; i < sc->sc_num_mididevs; i++)
deactivate_mididev(&sc->sc_mididevs[i]);
}
}
@@ -1506,7 +1514,6 @@ out_jack_output(struct umidi_jack *out_j
umidi_tv.tv_sec%100, (uint64_t)umidi_tv.tv_usec,
ep, out_jack->cable_number, len, cin));
- mutex_spin_enter(&sc->sc_intr_lock);
packet = *ep->next_slot++;
KASSERT(ep->buffer_size >=
(ep->next_slot - ep->buffer) * sizeof *ep->buffer);
@@ -1554,7 +1561,6 @@ out_jack_output(struct umidi_jack *out_j
ep->soliciting = 1;
softint_schedule(ep->solicit_cookie);
}
- mutex_spin_exit(&sc->sc_intr_lock);
return 0;
}
@@ -1576,9 +1582,9 @@ in_intr(usbd_xfer_handle xfer, usbd_priv
if (ep->sc->sc_dying || !ep->num_open)
return;
- mutex_enter(&sc->sc_intr_lock);
+ mutex_enter(&sc->sc_lock);
usbd_get_xfer_status(xfer, NULL, NULL, &count, NULL);
- if ( 0 == count % UMIDI_PACKET_SIZE ) {
+ if (0 == count % UMIDI_PACKET_SIZE) {
DPRINTFN(200,("%s: input endpoint %p transfer length %u\n",
device_xname(ep->sc->sc_dev), ep, count));
} else {
@@ -1589,9 +1595,9 @@ in_intr(usbd_xfer_handle xfer, usbd_priv
slot = ep->buffer;
end = slot + count / sizeof *slot;
- for ( packet = *slot; slot < end; packet = *++slot ) {
+ for (packet = *slot; slot < end; packet = *++slot) {
- if ( UMQ_ISTYPE(ep->sc, UMQ_TYPE_MIDIMAN_GARBLE) ) {
+ if (UMQ_ISTYPE(ep->sc, UMQ_TYPE_MIDIMAN_GARBLE)) {
cn = (0xf0&(packet[3]))>>4;
len = 0x0f&(packet[3]);
data = packet;
@@ -1608,7 +1614,7 @@ in_intr(usbd_xfer_handle xfer, usbd_priv
(unsigned)data[0],
(unsigned)data[1],
(unsigned)data[2]));
- mutex_exit(&sc->sc_intr_lock);
+ mutex_exit(&sc->sc_lock);
return;
}
@@ -1623,7 +1629,7 @@ in_intr(usbd_xfer_handle xfer, usbd_priv
(unsigned)data[2]));
if (jack->u.in.intr) {
- for (i=0; i<len; i++) {
+ for (i = 0; i < len; i++) {
(*jack->u.in.intr)(jack->arg, data[i]);
}
}
@@ -1631,7 +1637,7 @@ in_intr(usbd_xfer_handle xfer, usbd_priv
}
(void)start_input_transfer(ep);
- mutex_exit(&sc->sc_intr_lock);
+ mutex_exit(&sc->sc_lock);
}
static void
@@ -1645,7 +1651,7 @@ out_intr(usbd_xfer_handle xfer, usbd_pri
if (sc->sc_dying)
return;
- mutex_enter(&sc->sc_intr_lock);
+ mutex_enter(&sc->sc_lock);
#ifdef UMIDI_DEBUG
if ( umididebug >= 200 )
microtime(&umidi_tv);
@@ -1666,21 +1672,21 @@ out_intr(usbd_xfer_handle xfer, usbd_pri
* move them to the start of the buffer.
*/
ep->next_slot -= count;
- if ( ep->buffer < ep->next_slot ) {
+ if (ep->buffer < ep->next_slot) {
memcpy(ep->buffer, ep->buffer + count,
(char *)ep->next_slot - (char *)ep->buffer);
}
- wakeup(ep);
+ cv_broadcast(&sc->sc_cv);
/*
* Do not want anyone else to see armed <- 0 before soliciting <- 1.
* Running at IPL_USB so the following should happen to be safe.
*/
ep->armed = 0;
- if ( !ep->soliciting ) {
+ if (!ep->soliciting) {
ep->soliciting = 1;
- out_solicit(ep);
+ out_solicit_locked(ep);
}
- mutex_exit(&sc->sc_intr_lock);
+ mutex_exit(&sc->sc_lock);
}
/*
@@ -1699,23 +1705,25 @@ out_intr(usbd_xfer_handle xfer, usbd_pri
* packets, starting the USB transfer only when the buffer space is down to
* the minimum or no jack has any more to send.
*/
+
static void
-out_solicit(void *arg)
+out_solicit_locked(void *arg)
{
struct umidi_endpoint *ep = arg;
struct umidi_softc *sc = ep->sc;
umidi_packet_bufp end;
u_int16_t which;
struct umidi_jack *jack;
+
+ KASSERT(mutex_owned(&sc->sc_lock));
end = ep->buffer + ep->buffer_size / sizeof *ep->buffer;
for ( ;; ) {
- mutex_spin_enter(&sc->sc_intr_lock);
- if ( end - ep->next_slot <= ep->num_open - ep->num_scheduled )
+ if (end - ep->next_slot <= ep->num_open - ep->num_scheduled)
break; /* at IPL_USB */
- if ( ep->this_schedule == 0 ) {
- if ( ep->next_schedule == 0 )
+ if (ep->this_schedule == 0) {
+ if (ep->next_schedule == 0)
break; /* at IPL_USB */
ep->this_schedule = ep->next_schedule;
ep->next_schedule = 0;
@@ -1734,25 +1742,33 @@ out_solicit(void *arg)
which = ep->this_schedule;
which &= (~which)+1; /* now mask of least set bit */
ep->this_schedule &= ~which;
- -- ep->num_scheduled;
- mutex_spin_exit(&sc->sc_intr_lock);
+ --ep->num_scheduled;
- -- which; /* now 1s below mask - count 1s to get index */
+ --which; /* now 1s below mask - count 1s to get index */
which -= ((which >> 1) & 0x5555);/* SWAR credit aggregate.org */
which = (((which >> 2) & 0x3333) + (which & 0x3333));
which = (((which >> 4) + which) & 0x0f0f);
which += (which >> 8);
which &= 0x1f; /* the bit index a/k/a jack number */
- KERNEL_LOCK(1, curlwp);
jack = ep->jacks[which];
if (jack->u.out.intr)
(*jack->u.out.intr)(jack->arg);
- KERNEL_UNLOCK_ONE(curlwp);
}
/* intr lock held at loop exit */
- if ( !ep->armed && ep->next_slot > ep->buffer )
+ if (!ep->armed && ep->next_slot > ep->buffer)
ep->armed = (USBD_IN_PROGRESS == start_output_transfer(ep));
ep->soliciting = 0;
- mutex_spin_exit(&sc->sc_intr_lock);
+}
+
+/* Entry point for the softintr. */
+static void
+out_solicit(void *arg)
+{
+ struct umidi_endpoint *ep = arg;
+ struct umidi_softc *sc = ep->sc;
+
+ mutex_enter(&sc->sc_lock);
+ out_solicit_locked(arg);
+ mutex_exit(&sc->sc_lock);
}
Index: src/sys/dev/usb/umidivar.h
diff -u src/sys/dev/usb/umidivar.h:1.15 src/sys/dev/usb/umidivar.h:1.16
--- src/sys/dev/usb/umidivar.h:1.15 Wed Nov 23 23:07:36 2011
+++ src/sys/dev/usb/umidivar.h Thu Nov 24 22:12:52 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: umidivar.h,v 1.15 2011/11/23 23:07:36 jmcneill Exp $ */
+/* $NetBSD: umidivar.h,v 1.16 2011/11/24 22:12:52 mrg Exp $ */
/*
* Copyright (c) 2001 The NetBSD Foundation, Inc.
* All rights reserved.
@@ -121,5 +121,5 @@ struct umidi_softc {
int cblnums_global;
kmutex_t sc_lock;
- kmutex_t sc_intr_lock;
+ kcondvar_t sc_cv;
};
