Re: Implement mode_select for atapi tape drives

[Paul Goyette <paul%whooppee.com@localhost>]

On Sat, 8 Aug 2009, Marc Balmer wrote:

> > But the calculated value of *_select_len is actually passed to 
> > routine scsipi_mode_select() a little bit later on, so we still need 
> > the calculated value.
>
> why cant sizeof be used there, too?

It actually controls amount of data that is sent to the device.  Most 
devices have 0 bytes for st->page_0_size but some have 12 bytes.


The attached patch revision includes the change to use sizeof() in the 
calls to memset(), adds a KASSERT when loading the quirk data to ensure 
that the st->page_0_size is not larger than MAX_PAGE_0_SIZE, and adds 
the necessary quirk for my tape drive.

Additional comments/criticisms welcomed!



-------------------------------------------------------------------------
|   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        8 Aug 2009 13:23:14 -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", ""},    {ST_Q_ATAPI_MODESEL, 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);
        }
 }
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  8 Aug 2009 13:23:14 -0000
@@ -46,6 +46,7 @@ __KERNEL_RCSID(0, "$NetBSD: st_atapi.c,v
 
 #include <dev/scsipi/stvar.h>
 #include <dev/scsipi/atapi_tape.h>
+#include <dev/scsipi/scsi_spc.h>
 
 static int     st_atapibus_match(device_t, cfdata_t, void *);
 static void    st_atapibus_attach(device_t, device_t, void *);
@@ -177,8 +178,41 @@ st_atapibus_mode_sense(struct st_softc *
        return error;
 }
 
+/*
+ * Send a filled out parameter structure to the drive to
+ * set it into the desire modes etc.
+ */
 static int
 st_atapibus_mode_select(struct st_softc *st, int flags)
 {
-       return ENODEV; /* for now ... */
+       u_int atapi_select_len;
+       struct atapi_select {
+               struct scsi_mode_parameter_header_6  header;
+               struct scsi_general_block_descriptor blk_desc;
+               u_char sense_data[MAX_PAGE_0_SIZE];
+       } atapi_select;
+       struct scsipi_periph *periph = st->sc_periph;
+
+       if (!(st->quirkdata->quirks & ST_Q_ATAPI_MODESEL))
+               return ENODEV;
+
+       atapi_select_len = 12 + st->page_0_size;
+
+       /*
+        * Set up for a mode select
+        */
+       memset(&atapi_select, 0, sizeof(atapi_select));
+       atapi_select.header.blk_desc_len = sizeof(struct 
scsi_general_block_descriptor);
+       atapi_select.blk_desc.density = st->density;
+       if (st->flags & ST_FIXEDBLOCKS)
+               _lto3b(st->blksize, atapi_select.blk_desc.blklen);
+       if (st->page_0_size)
+               memcpy(atapi_select.sense_data, st->sense_data, 
st->page_0_size);
+
+       /*
+        * do the command
+        */
+       return scsipi_mode_select(periph, 0, &atapi_select.header,
+           atapi_select_len, flags | XS_CTL_DATA_ONSTACK,
+           ST_RETRIES, ST_CTL_TIME);
 }
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   8 Aug 2009 13:23:14 -0000
@@ -254,7 +254,7 @@ st_scsibus_mode_select(struct st_softc *
        /*
         * Set up for a mode select
         */
-       memset(&scsi_select, 0, scsi_select_len);
+       memset(&scsi_select, 0, sizeof(scsi_select));
        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;
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     8 Aug 2009 13:23:14 -0000
@@ -79,6 +79,7 @@ struct quirkdata {
 #define        ST_Q_NOPREVENT          0x0020  /* does not support PREVENT */
 #define        ST_Q_ERASE_NOIMM        0x0040  /* drive rejects ERASE/w Immed 
bit */
 #define        ST_Q_NOFILEMARKS        0x0080  /* can only write 0 filemarks */
+#define        ST_Q_ATAPI_MODESEL      0x0100  /* ATAPI drive does SCSI Mode 
Select */
        u_int page_0_size;
 #define        MAX_PAGE_0_SIZE 64
        struct modes modes[4];