Mathieu - wrote:
> 
> Hello,
> 
> Martin Pieuchot wrote:
> > Hello,
> > 
> > On 07/12/15(Mon) 16:48, Mathieu - wrote:
> > > Hello,
> > > 
> > > I worked a bit on umass(4) recently and had a diff to pass the
> > > umassbus_softc's real size to free so here it is.  At some point I
> > > pondered about deleting the whole abstraction, as it would simplify the
> > > free'ing, for we only have one implementation (umass_scsi_softc, as atapi
> > > uses it too). But I figured it would be against the whole design of the
> > > umass driver, thoughts?
> > 
> > I'd rather create a umass_scsi_detach() function symmetrical to
> > umass_scsi_attach().  This way you don't need an extra variable
> > for the size, keep the autoconf(9) glue inside umass_scsi.c and
> > can turn "struct umassbus_softc" into an opaque type.
> 
> Please find a patch implementing the suggested approach below.

Oups I left a debugging printf in there. Here is the clean patch, sorry
again for the noise.

Index: usb/umass.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/umass.c,v
retrieving revision 1.70
diff -u -p -r1.70 umass.c
--- usb/umass.c 14 Mar 2015 03:38:50 -0000      1.70
+++ usb/umass.c 10 Dec 2015 11:03:16 -0000
@@ -129,7 +129,6 @@
 #include <sys/kernel.h>
 #include <sys/conf.h>
 #include <sys/buf.h>
-#include <sys/device.h>
 #include <sys/malloc.h>
 #include <sys/timeout.h>
 #undef KASSERT
@@ -616,7 +615,6 @@ int
 umass_detach(struct device *self, int flags)
 {
        struct umass_softc *sc = (struct umass_softc *)self;
-       struct umassbus_softc *scbus;
        int rv = 0, i, s;
 
        DPRINTF(UDMASS_USB, ("%s: detached\n", sc->sc_dev.dv_xname));
@@ -647,12 +645,16 @@ umass_detach(struct device *self, int fl
        }
        splx(s);
 
-       scbus = sc->bus;
-       if (scbus != NULL) {
-               if (scbus->sc_child != NULL)
-                       rv = config_detach(scbus->sc_child, flags);
-               free(scbus, M_DEVBUF, 0);
-               sc->bus = NULL;
+       switch (sc->sc_cmd) {
+       case UMASS_CPROTO_RBC:
+       case UMASS_CPROTO_SCSI:
+               rv = umass_scsi_detach(sc, flags);
+               break;
+
+       case UMASS_CPROTO_UFI:
+       case UMASS_CPROTO_ATAPI:
+               rv = umass_atapi_detach(sc, flags);
+               break;
        }
 
        if (rv != 0)
Index: usb/umass_scsi.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/umass_scsi.c,v
retrieving revision 1.42
diff -u -p -r1.42 umass_scsi.c
--- usb/umass_scsi.c    14 Mar 2015 03:38:50 -0000      1.42
+++ usb/umass_scsi.c    10 Dec 2015 11:03:17 -0000
@@ -51,8 +51,8 @@
 #include <scsi/scsi_disk.h>
 #include <machine/bus.h>
 
-struct umass_scsi_softc {
-       struct umassbus_softc   base;
+struct umassbus_softc {
+       struct device       *sc_child;      /* child device, for detach */
        struct scsi_link        sc_link;
        struct scsi_iopool      sc_iopool;
        int                     sc_open;
@@ -78,7 +78,7 @@ void umass_scsi_cb(struct umass_softc *s
                   int status);
 void umass_scsi_sense_cb(struct umass_softc *sc, void *priv, int residue,
                         int status);
-struct umass_scsi_softc *umass_scsi_setup(struct umass_softc *);
+struct umassbus_softc *umass_scsi_setup(struct umass_softc *);
 
 void *umass_io_get(void *);
 void umass_io_put(void *, void *);
@@ -87,7 +87,7 @@ int
 umass_scsi_attach(struct umass_softc *sc)
 {
        struct scsibus_attach_args saa;
-       struct umass_scsi_softc *scbus;
+       struct umassbus_softc *scbus;
 
        scbus = umass_scsi_setup(sc);
        scbus->sc_link.adapter_target = UMASS_SCSIID_HOST;
@@ -103,8 +103,7 @@ umass_scsi_attach(struct umass_softc *sc
                             sc->sc_dev.dv_xname, sc, scbus));
 
        sc->sc_refcnt++;
-       scbus->base.sc_child =
-         config_found((struct device *)sc, &saa, scsiprint);
+       scbus->sc_child = config_found((struct device *)sc, &saa, scsiprint);
        if (--sc->sc_refcnt < 0)
                usb_detach_wakeup(&sc->sc_dev);
 
@@ -115,7 +114,7 @@ int
 umass_atapi_attach(struct umass_softc *sc)
 {
        struct scsibus_attach_args saa;
-       struct umass_scsi_softc *scbus;
+       struct umassbus_softc *scbus;
 
        scbus = umass_scsi_setup(sc);
        scbus->sc_link.adapter_target = UMASS_SCSIID_HOST;
@@ -131,22 +130,21 @@ umass_atapi_attach(struct umass_softc *s
                             sc->sc_dev.dv_xname, sc, scbus));
 
        sc->sc_refcnt++;
-       scbus->base.sc_child = config_found((struct device *)sc,
-           &saa, scsiprint);
+       scbus->sc_child = config_found((struct device *)sc, &saa, scsiprint);
        if (--sc->sc_refcnt < 0)
                usb_detach_wakeup(&sc->sc_dev);
 
        return (0);
 }
 
-struct umass_scsi_softc *
+struct umassbus_softc *
 umass_scsi_setup(struct umass_softc *sc)
 {
-       struct umass_scsi_softc *scbus;
+       struct umassbus_softc *scbus;
 
        scbus = malloc(sizeof(*scbus), M_DEVBUF, M_WAITOK | M_ZERO);
 
-       sc->bus = (struct umassbus_softc *)scbus;
+       sc->bus = scbus;
 
        scsi_iopool_init(&scbus->sc_iopool, scbus, umass_io_get, umass_io_put);
 
@@ -162,6 +160,29 @@ umass_scsi_setup(struct umass_softc *sc)
 }
 
 int
+umass_scsi_detach(struct umass_softc *sc, int flags)
+{
+       struct umassbus_softc *scbus;
+       int rv = 0;
+
+       scbus = sc->bus;
+       if (scbus != NULL) {
+               if (scbus->sc_child != NULL)
+                       rv = config_detach(scbus->sc_child, flags);
+               free(scbus, M_DEVBUF, sizeof(*scbus));
+               sc->bus = NULL;
+       }
+
+       return rv;
+}
+
+int
+umass_atapi_detach(struct umass_softc *sc, int flags)
+{
+       return umass_scsi_detach(sc, flags);
+}
+
+int
 umass_scsi_probe(struct scsi_link *link)
 {
        struct umass_softc *sc = link->adapter_softc;
@@ -289,7 +310,7 @@ umass_scsi_minphys(struct buf *bp, struc
 void
 umass_scsi_cb(struct umass_softc *sc, void *priv, int residue, int status)
 {
-       struct umass_scsi_softc *scbus = (struct umass_scsi_softc *)sc->bus;
+       struct umassbus_softc *scbus = sc->bus;
        struct scsi_xfer *xs = priv;
        struct scsi_link *link = xs->sc_link;
        int cmdlen;
@@ -453,7 +474,7 @@ umass_scsi_sense_cb(struct umass_softc *
 void *
 umass_io_get(void *cookie)
 {
-       struct umass_scsi_softc *scbus = cookie;
+       struct umassbus_softc *scbus = cookie;
        void *io = NULL;
        int s;
 
@@ -470,7 +491,7 @@ umass_io_get(void *cookie)
 void
 umass_io_put(void *cookie, void *io)
 {
-       struct umass_scsi_softc *scbus = cookie;
+       struct umassbus_softc *scbus = cookie;
        int s;
 
        s = splusb();
Index: usb/umass_scsi.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/umass_scsi.h,v
retrieving revision 1.4
diff -u -p -r1.4 umass_scsi.h
--- usb/umass_scsi.h    26 Jun 2008 05:42:19 -0000      1.4
+++ usb/umass_scsi.h    10 Dec 2015 11:03:17 -0000
@@ -33,3 +33,5 @@
 
 int umass_scsi_attach(struct umass_softc *sc);
 int umass_atapi_attach(struct umass_softc *sc);
+int umass_scsi_detach(struct umass_softc *sc, int flags);
+int umass_atapi_detach(struct umass_softc *sc, int flags);
Index: usb/umassvar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/umassvar.h,v
retrieving revision 1.14
diff -u -p -r1.14 umassvar.h
--- usb/umassvar.h      6 Nov 2013 14:37:31 -0000       1.14
+++ usb/umassvar.h      10 Dec 2015 11:03:17 -0000
@@ -144,9 +144,7 @@ struct umass_wire_methods {
        umass_wire_state        wire_state;
 };
 
-struct umassbus_softc {
-       struct device           *sc_child;      /* child device, for detach */
-};
+struct umassbus_softc;
 
 /* the per device structure */
 struct umass_softc {

Reply via email to