Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/usb several steps towards making umidi appear to work:



details:   https://anonhg.NetBSD.org/src/rev/8be3ed95fee4
branches:  trunk
changeset: 771557:8be3ed95fee4
user:      mrg <mrg%NetBSD.org@localhost>
date:      Thu Nov 24 22:12:51 2011 +0000

description:
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!

diffstat:

 sys/dev/usb/umidi.c    |  176 ++++++++++++++++++++++++++----------------------
 sys/dev/usb/umidivar.h |    4 +-
 2 files changed, 98 insertions(+), 82 deletions(-)

diffs (truncated from 529 to 300 lines):

diff -r 04703851f54b -r 8be3ed95fee4 sys/dev/usb/umidi.c
--- a/sys/dev/usb/umidi.c       Thu Nov 24 21:59:35 2011 +0000
+++ b/sys/dev/usb/umidi.c       Thu Nov 24 22:12:51 2011 +0000
@@ -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 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 @@
        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 @@
               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 @@
                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 @@
 
        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 @@
 }
 
 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 @@
            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 @@
        /* 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 @@
        }
        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 @@
 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 @@
        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 @@
        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 @@
        umidi_packet_bufp end;
        int err;
 
+       KASSERT(mutex_owned(&sc->sc_lock));
+
        if (jack->opened)
                return USBD_IN_USE;
 
@@ -1088,7 +1094,6 @@
        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 @@
         * 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 @@
 {
        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 @@
        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 @@
        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;



Home | Main Index | Thread Index | Old Index