Source-Changes-HG archive

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

[src/trunk]: src/sys/dev/usb various umidi clean ups:



details:   https://anonhg.NetBSD.org/src/rev/96d4ce9cafa3
branches:  trunk
changeset: 335025:96d4ce9cafa3
user:      mrg <mrg%NetBSD.org@localhost>
date:      Sun Dec 21 23:00:35 2014 +0000

description:
various umidi clean ups:
- move the contents of umidi{reg,var}.h into umidi.c directly as they
  are not referenced by any other file.
- remove the useless include of umidi{reg,var}.h from umidi_quirks.c.
- add reference counting and wait/broadcast support to the IO paths.
- fix the error handling in midi_attach() and midi_open().
- sprinkle KASSERT() in several places.
- drop the local interrupt lock before calling into various parts of
  the USB code.  fixes lockdebug issues, and likely hangs.
- rename "binded" member as "bound".

with these most of the panics and problems i've seen are gone.  there
is still one lockdebug panic to deal with that happens when unplugging
umidi while midiplay(1) is running.

diffstat:

 sys/dev/usb/FILES          |    2 -
 sys/dev/usb/umidi.c        |  346 +++++++++++++++++++++++++++++++++++++-------
 sys/dev/usb/umidi_quirks.c |    6 +-
 sys/dev/usb/umidireg.h     |   79 ----------
 sys/dev/usb/umidivar.h     |  129 ----------------
 5 files changed, 294 insertions(+), 268 deletions(-)

diffs (truncated from 962 to 300 lines):

diff -r f0e408ce7fa6 -r 96d4ce9cafa3 sys/dev/usb/FILES
--- a/sys/dev/usb/FILES Sun Dec 21 22:21:03 2014 +0000
+++ b/sys/dev/usb/FILES Sun Dec 21 23:00:35 2014 +0000
@@ -55,8 +55,6 @@
 umidi.c                        USB MIDI driver
 umidi_quirks.c         Strange MIDI devices
 umidi_quirks.h           and definitions for it
-umidireg.h             Protocol definitions for umidi.c
-umidivar.h             definitions for umidi.c
 umodem.c               USB modem (CDC ACM) driver
 ums.c                  USB mouse driver
 urio.c                 USB Diamond Rio500 driver
diff -r f0e408ce7fa6 -r 96d4ce9cafa3 sys/dev/usb/umidi.c
--- a/sys/dev/usb/umidi.c       Sun Dec 21 22:21:03 2014 +0000
+++ b/sys/dev/usb/umidi.c       Sun Dec 21 23:00:35 2014 +0000
@@ -1,6 +1,7 @@
-/*     $NetBSD: umidi.c,v 1.65 2013/01/22 21:29:53 jmcneill Exp $      */
+/*     $NetBSD: umidi.c,v 1.66 2014/12/21 23:00:35 mrg Exp $   */
+
 /*
- * Copyright (c) 2001, 2012 The NetBSD Foundation, Inc.
+ * Copyright (c) 2001, 2012, 2014 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -31,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: umidi.c,v 1.65 2013/01/22 21:29:53 jmcneill Exp $");
+__KERNEL_RCSID(0, "$NetBSD: umidi.c,v 1.66 2014/12/21 23:00:35 mrg Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -54,12 +55,161 @@
 
 #include <dev/auconv.h>
 #include <dev/usb/usbdevs.h>
-#include <dev/usb/uaudioreg.h>
-#include <dev/usb/umidireg.h>
-#include <dev/usb/umidivar.h>
 #include <dev/usb/umidi_quirks.h>
+#include <dev/midi_if.h>
 
-#include <dev/midi_if.h>
+/* Jack Descriptor */
+#define UMIDI_MS_HEADER        0x01
+#define UMIDI_IN_JACK  0x02
+#define UMIDI_OUT_JACK 0x03
+
+/* Jack Type */
+#define UMIDI_EMBEDDED 0x01
+#define UMIDI_EXTERNAL 0x02
+
+/* generic, for iteration */
+typedef struct {
+       uByte           bLength;
+       uByte           bDescriptorType;
+       uByte           bDescriptorSubtype;
+} UPACKED umidi_cs_descriptor_t;
+
+typedef struct {
+       uByte           bLength;
+       uByte           bDescriptorType;
+       uByte           bDescriptorSubtype;
+       uWord           bcdMSC;
+       uWord           wTotalLength;
+} UPACKED umidi_cs_interface_descriptor_t;
+#define UMIDI_CS_INTERFACE_DESCRIPTOR_SIZE 7
+
+typedef struct {
+       uByte           bLength;
+       uByte           bDescriptorType;
+       uByte           bDescriptorSubtype;
+       uByte           bNumEmbMIDIJack;
+} UPACKED umidi_cs_endpoint_descriptor_t;
+#define UMIDI_CS_ENDPOINT_DESCRIPTOR_SIZE 4
+
+typedef struct {
+       uByte           bLength;
+       uByte           bDescriptorType;
+       uByte           bDescriptorSubtype;
+       uByte           bJackType;
+       uByte           bJackID;
+} UPACKED umidi_jack_descriptor_t;
+#define        UMIDI_JACK_DESCRIPTOR_SIZE      5
+
+
+#define TO_D(p) ((usb_descriptor_t *)(p))
+#define NEXT_D(desc) TO_D((char *)(desc)+(desc)->bLength)
+#define TO_IFD(desc) ((usb_interface_descriptor_t *)(desc))
+#define TO_CSIFD(desc) ((umidi_cs_interface_descriptor_t *)(desc))
+#define TO_EPD(desc) ((usb_endpoint_descriptor_t *)(desc))
+#define TO_CSEPD(desc) ((umidi_cs_endpoint_descriptor_t *)(desc))
+
+
+#define UMIDI_PACKET_SIZE 4
+
+/*
+ * hierarchie
+ *
+ * <-- parent         child -->
+ *
+ * umidi(sc) -> endpoint -> jack   <- (dynamically assignable) - mididev
+ *        ^     |    ^     |
+ *        +-----+    +-----+
+ */
+
+/* midi device */
+struct umidi_mididev {
+       struct umidi_softc      *sc;
+       device_t                mdev;
+       /* */
+       struct umidi_jack       *in_jack;
+       struct umidi_jack       *out_jack;
+       char                    *label;
+       size_t                  label_len;
+       /* */
+       int                     opened;
+       int                     closing;
+       int                     flags;
+};
+
+/* Jack Information */
+struct umidi_jack {
+       struct umidi_endpoint   *endpoint;
+       /* */
+       int                     cable_number;
+       void                    *arg;
+       int                     bound;
+       int                     opened;
+       unsigned char           *midiman_ppkt;
+       union {
+               struct {
+                       void                    (*intr)(void *);
+               } out;
+               struct {
+                       void                    (*intr)(void *, int);
+               } in;
+       } u;
+};
+
+#define UMIDI_MAX_EPJACKS      16
+typedef unsigned char (*umidi_packet_bufp)[UMIDI_PACKET_SIZE];
+/* endpoint data */
+struct umidi_endpoint {
+       struct umidi_softc      *sc;
+       /* */
+       int                     addr;
+       usbd_pipe_handle        pipe;
+       usbd_xfer_handle        xfer;
+       umidi_packet_bufp       buffer;
+       umidi_packet_bufp       next_slot;
+       u_int32_t               buffer_size;
+       int                     num_scheduled;
+       int                     num_open;
+       int                     num_jacks;
+       int                     soliciting;
+       void                    *solicit_cookie;
+       int                     armed;
+       struct umidi_jack       *jacks[UMIDI_MAX_EPJACKS];
+       u_int16_t               this_schedule; /* see UMIDI_MAX_EPJACKS */
+       u_int16_t               next_schedule;
+};
+
+/* software context */
+struct umidi_softc {
+       device_t                sc_dev;
+       usbd_device_handle      sc_udev;
+       usbd_interface_handle   sc_iface;
+       const struct umidi_quirk        *sc_quirk;
+
+       int                     sc_dying;
+
+       int                     sc_out_num_jacks;
+       struct umidi_jack       *sc_out_jacks;
+       int                     sc_in_num_jacks;
+       struct umidi_jack       *sc_in_jacks;
+       struct umidi_jack       *sc_jacks;
+
+       int                     sc_num_mididevs;
+       struct umidi_mididev    *sc_mididevs;
+
+       int                     sc_out_num_endpoints;
+       struct umidi_endpoint   *sc_out_ep;
+       int                     sc_in_num_endpoints;
+       struct umidi_endpoint   *sc_in_ep;
+       struct umidi_endpoint   *sc_endpoints;
+       size_t                  sc_endpoints_len;
+       int                     cblnums_global;
+
+       kmutex_t                sc_lock;
+       kcondvar_t              sc_cv;
+       kcondvar_t              sc_detach_cv;
+
+       int                     sc_refcnt;
+};
 
 #ifdef UMIDI_DEBUG
 #define DPRINTF(x)     if (umididebug) printf x
@@ -210,36 +360,33 @@
 
        mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_USB);
        cv_init(&sc->sc_cv, "umidopcl");
+       cv_init(&sc->sc_detach_cv, "umidetcv");
+       sc->sc_refcnt = 0;
 
        err = alloc_all_endpoints(sc);
        if (err != USBD_NORMAL_COMPLETION) {
                aprint_error_dev(self,
                    "alloc_all_endpoints failed. (err=%d)\n", err);
-               goto error;
+               goto out;
        }
        err = alloc_all_jacks(sc);
        if (err != USBD_NORMAL_COMPLETION) {
-               free_all_endpoints(sc);
                aprint_error_dev(self, "alloc_all_jacks failed. (err=%d)\n",
                    err);
-               goto error;
+               goto out_free_endpoints;
        }
        aprint_normal_dev(self, "out=%d, in=%d\n",
               sc->sc_out_num_jacks, sc->sc_in_num_jacks);
 
        err = assign_all_jacks_automatically(sc);
        if (err != USBD_NORMAL_COMPLETION) {
-               unbind_all_jacks(sc);
-               free_all_jacks(sc);
-               free_all_endpoints(sc);
                aprint_error_dev(self,
                    "assign_all_jacks_automatically failed. (err=%d)\n", err);
-               goto error;
+               goto out_free_jacks;
        }
        err = attach_all_mididevs(sc);
        if (err != USBD_NORMAL_COMPLETION) {
-               free_all_jacks(sc);
-               free_all_endpoints(sc);
+               goto out_free_jacks;
                aprint_error_dev(self,
                    "attach_all_mididevs failed. (err=%d)\n", err);
        }
@@ -252,7 +399,15 @@
                           sc->sc_udev, sc->sc_dev);
 
        return;
-error:
+
+out_free_jacks:
+       unbind_all_jacks(sc);
+       free_all_jacks(sc);
+       
+out_free_endpoints:
+       free_all_endpoints(sc);
+
+out:
        aprint_error_dev(self, "disabled.\n");
        sc->sc_dying = 1;
        KERNEL_UNLOCK_ONE(curlwp);
@@ -299,7 +454,12 @@
 
        DPRINTFN(1,("umidi_detach\n"));
 
+       mutex_enter(&sc->sc_lock);
        sc->sc_dying = 1;
+       if (--sc->sc_refcnt >= 0)
+               usb_detach_wait(sc->sc_dev, &sc->sc_detach_cv, &sc->sc_lock);
+       mutex_exit(&sc->sc_lock);
+
        detach_all_mididevs(sc, flags);
        free_all_mididevs(sc);
        free_all_jacks(sc);
@@ -309,6 +469,7 @@
                           sc->sc_dev);
 
        mutex_destroy(&sc->sc_lock);
+       cv_destroy(&sc->sc_detach_cv);
        cv_destroy(&sc->sc_cv);
 
        return 0;
@@ -329,6 +490,7 @@
        struct umidi_softc *sc = mididev->sc;
        usbd_status err;
 
+       KASSERT(mutex_owned(&sc->sc_lock));
        DPRINTF(("umidi_open: sc=%p\n", sc));
 
        if (!sc)
@@ -339,24 +501,27 @@
                return EIO;
 
        mididev->opened = 1;
-       mididev->closing = 0;
        mididev->flags = flags;
        if ((mididev->flags & FWRITE) && mididev->out_jack) {
                err = open_out_jack(mididev->out_jack, arg, ointr);
-               if ( err != USBD_NORMAL_COMPLETION )
+               if (err != USBD_NORMAL_COMPLETION)
                        goto bad;



Home | Main Index | Thread Index | Old Index