tech-kern archive

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

Re: Implement mode_select for atapi tape drives - round 4



As David Young pointed out, it might be better to extract the set_mode code out from both st_atapi.c and st_scsi.c into something common. I'll take a look at doing that, and see if I can reconcile the differences between them.

OK, that wasn't so hard!    :)

The attached patch moves the mode_select functionality into common code in st.c and invokes the common routine for both scsi and atapi tapes.

It also replaces a numeric constant with some sizeof's when calculating the size of the mode_select command buffer, clears the entire buffer, and has a KASSERT to ensure that the page_0_size loaded from the quirk table is valid.

Further comment/criticism is certainly welcomed. In the absence of any objection, I'd like to commit this in the next 7 to 10 days.



-------------------------------------------------------------------------
|   Paul Goyette   | PGP DSS Key fingerprint: |  E-mail addresses:      |
| Customer Service | FA29 0E3B 35AF E8AE 6651 |  paul at whooppee.com   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |                          | pgoyette at netbsd.org  |
-------------------------------------------------------------------------
Index: st.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/st.c,v
retrieving revision 1.211
diff -u -p -r1.211 st.c
--- st.c        12 May 2009 14:44:31 -0000      1.211
+++ st.c        9 Aug 2009 02:51:32 -0000
@@ -306,6 +306,13 @@ static const struct st_quirk_inquiry_pat
                {0, 0, 0},                             /* minor 8-11 */
                {0, 0, 0}                              /* minor 12-15 */
        }}},
+       {{T_SEQUENTIAL, T_REMOV,
+        "Seagate STT3401A", "hp0atxa", ""},    {0, 0, {
+               {ST_Q_FORCE_BLKSIZE, 512, 0},           /* minor 0-3 */
+               {ST_Q_FORCE_BLKSIZE, 1024, 0},          /* minor 4-7 */
+               {ST_Q_FORCE_BLKSIZE, 512, 0},           /* minor 8-11 */
+               {ST_Q_FORCE_BLKSIZE, 512, 0}            /* minor 12-15 */
+       }}},
 };
 
 #define NOEJECT 0
@@ -488,6 +495,7 @@ st_identify_drive(struct st_softc *st, s
                st->drive_quirks = finger->quirkdata.quirks;
                st->quirks = finger->quirkdata.quirks;  /* start value */
                st->page_0_size = finger->quirkdata.page_0_size;
+               KASSERT(st->page_0_size <= MAX_PAGE_0_SIZE);
                st_loadquirks(st);
        }
 }
@@ -2401,3 +2409,57 @@ stdump(dev_t dev, daddr_t blkno, void *v
        /* Not implemented. */
        return (ENXIO);
 }
+
+/*
+ * Send a filled out parameter structure to the drive to
+ * set it into the desire modes etc.
+ */
+int
+st_mode_select(struct st_softc *st, int flags)
+{
+       u_int select_len;
+       struct select {
+               struct scsi_mode_parameter_header_6 header;
+               struct scsi_general_block_descriptor blk_desc;
+               u_char sense_data[MAX_PAGE_0_SIZE];
+       } select;
+       struct scsipi_periph *periph = st->sc_periph;
+
+       select_len = sizeof(select.header) + sizeof(select.blk_desc) +
+                    st->page_0_size;
+
+       /*
+        * This quirk deals with drives that have only one valid mode
+        * and think this gives them license to reject all mode selects,
+        * even if the selected mode is the one that is supported.
+        */
+       if (st->quirks & ST_Q_UNIMODAL) {
+               SC_DEBUG(periph, SCSIPI_DB3,
+                   ("not setting density 0x%x blksize 0x%x\n",
+                   st->density, st->blksize));
+               return (0);
+       }
+
+       /*
+        * Set up for a mode select
+        */
+       memset(&select, 0, sizeof(select));
+       select.header.blk_desc_len = sizeof(struct 
scsi_general_block_descriptor);
+       select.header.dev_spec &= ~SMH_DSP_BUFF_MODE;
+       select.blk_desc.density = st->density;
+       if (st->flags & ST_DONTBUFFER)
+               select.header.dev_spec |= SMH_DSP_BUFF_MODE_OFF;
+       else
+               select.header.dev_spec |= SMH_DSP_BUFF_MODE_ON;
+       if (st->flags & ST_FIXEDBLOCKS)
+               _lto3b(st->blksize, select.blk_desc.blklen);
+       if (st->page_0_size)
+               memcpy(select.sense_data, st->sense_data, st->page_0_size);
+
+       /*
+        * do the command
+        */
+       return scsipi_mode_select(periph, 0, &select.header, select_len,
+                                 flags | XS_CTL_DATA_ONSTACK, ST_RETRIES,
+                                 ST_CTL_TIME);
+}
Index: st_atapi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/st_atapi.c,v
retrieving revision 1.22
diff -u -p -r1.22 st_atapi.c
--- st_atapi.c  12 May 2009 14:44:31 -0000      1.22
+++ st_atapi.c  9 Aug 2009 02:51:32 -0000
@@ -51,7 +51,6 @@ static int    st_atapibus_match(device_t, c
 static void    st_atapibus_attach(device_t, device_t, void *);
 static int     st_atapibus_ops(struct st_softc *, int, int);
 static int     st_atapibus_mode_sense(struct st_softc *, int);
-static int     st_atapibus_mode_select(struct st_softc *, int);
 
 CFATTACH_DECL(st_atapibus, sizeof(struct st_softc),
     st_atapibus_match, st_atapibus_attach, stdetach, stactivate);
@@ -122,7 +121,7 @@ st_atapibus_ops(struct st_softc *st, int
        case ST_OPS_MODESENSE:
                return st_atapibus_mode_sense(st, flags);
        case ST_OPS_MODESELECT:
-               return st_atapibus_mode_select(st, flags);
+               return st_mode_select(st, flags);
        case ST_OPS_CMPRSS_ON:
        case ST_OPS_CMPRSS_OFF:
                return ENODEV;
@@ -176,9 +175,3 @@ st_atapibus_mode_sense(struct st_softc *
        }
        return error;
 }
-
-static int
-st_atapibus_mode_select(struct st_softc *st, int flags)
-{
-       return ENODEV; /* for now ... */
-}
Index: st_scsi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/st_scsi.c,v
retrieving revision 1.29
diff -u -p -r1.29 st_scsi.c
--- st_scsi.c   12 May 2009 14:44:31 -0000      1.29
+++ st_scsi.c   9 Aug 2009 02:51:32 -0000
@@ -72,7 +72,6 @@ static void   st_scsibus_attach(device_t, 
 static int     st_scsibus_ops(struct st_softc *, int, int);
 static int     st_scsibus_read_block_limits(struct st_softc *, int);
 static int     st_scsibus_mode_sense(struct st_softc *, int);
-static int     st_scsibus_mode_select(struct st_softc *, int);
 static int     st_scsibus_cmprss(struct st_softc *, int, int);
 
 CFATTACH_DECL(st_scsibus, sizeof(struct st_softc),
@@ -118,7 +117,7 @@ st_scsibus_ops(struct st_softc *st, int 
        case ST_OPS_MODESENSE:
                return st_scsibus_mode_sense(st, flags);
        case ST_OPS_MODESELECT:
-               return st_scsibus_mode_select(st, flags);
+               return st_mode_select(st, flags);
        case ST_OPS_CMPRSS_ON:
        case ST_OPS_CMPRSS_OFF:
                return st_scsibus_cmprss(st, flags,
@@ -187,7 +186,9 @@ st_scsibus_mode_sense(struct st_softc *s
        } scsipi_sense;
        struct scsipi_periph *periph = st->sc_periph;
 
-       scsipi_sense_len = 12 + st->page_0_size;
+       scsipi_sense_len = sizeof(scsipi_sense.header) +
+                          sizeof(scsipi_sense.blk_desc) +
+                          st->page_0_size;
 
        /*
         * Set up a mode sense
@@ -222,59 +223,6 @@ st_scsibus_mode_sense(struct st_softc *s
        return (0);
 }
 
-/*
- * Send a filled out parameter structure to the drive to
- * set it into the desire modes etc.
- */
-static int
-st_scsibus_mode_select(struct st_softc *st, int flags)
-{
-       u_int scsi_select_len;
-       struct scsi_select {
-               struct scsi_mode_parameter_header_6 header;
-               struct scsi_general_block_descriptor blk_desc;
-               u_char sense_data[MAX_PAGE_0_SIZE];
-       } scsi_select;
-       struct scsipi_periph *periph = st->sc_periph;
-
-       scsi_select_len = 12 + st->page_0_size;
-
-       /*
-        * This quirk deals with drives that have only one valid mode
-        * and think this gives them license to reject all mode selects,
-        * even if the selected mode is the one that is supported.
-        */
-       if (st->quirks & ST_Q_UNIMODAL) {
-               SC_DEBUG(periph, SCSIPI_DB3,
-                   ("not setting density 0x%x blksize 0x%x\n",
-                   st->density, st->blksize));
-               return (0);
-       }
-
-       /*
-        * Set up for a mode select
-        */
-       memset(&scsi_select, 0, scsi_select_len);
-       scsi_select.header.blk_desc_len = sizeof(struct 
scsi_general_block_descriptor);
-       scsi_select.header.dev_spec &= ~SMH_DSP_BUFF_MODE;
-       scsi_select.blk_desc.density = st->density;
-       if (st->flags & ST_DONTBUFFER)
-               scsi_select.header.dev_spec |= SMH_DSP_BUFF_MODE_OFF;
-       else
-               scsi_select.header.dev_spec |= SMH_DSP_BUFF_MODE_ON;
-       if (st->flags & ST_FIXEDBLOCKS)
-               _lto3b(st->blksize, scsi_select.blk_desc.blklen);
-       if (st->page_0_size)
-               memcpy(scsi_select.sense_data, st->sense_data, st->page_0_size);
-
-       /*
-        * do the command
-        */
-       return scsipi_mode_select(periph, 0, &scsi_select.header,
-           scsi_select_len, flags | XS_CTL_DATA_ONSTACK,
-           ST_RETRIES, ST_CTL_TIME);
-}
-
 static int
 st_scsibus_cmprss(struct st_softc *st, int flags, int onoff)
 {
Index: stvar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/stvar.h,v
retrieving revision 1.19
diff -u -p -r1.19 stvar.h
--- stvar.h     12 May 2009 14:44:31 -0000      1.19
+++ stvar.h     9 Aug 2009 02:51:32 -0000
@@ -181,5 +181,6 @@ struct st_softc {
 void   stattach(device_t, struct st_softc *, void *);
 int    stactivate(device_t, enum devact);
 int    stdetach(device_t, int);
+int    st_mode_select(struct st_softc *, int);
 
 extern struct cfdriver st_cd;


Home | Main Index | Thread Index | Old Index