On Fri, 18 Nov 2005, Michael van Elst wrote:
> xs->sense.scsi_sense.response_code & SSD_RCODE_VALID != 0
> SSD_RCODE(xs->sense.scsi_sense.response_code) == SSD_RCODE_CURRENT
> SSD_SENSE_KEY(xs->sense.scsi_sense.flags) == SKEY_ILLEGAL_REQUEST
> xs->sense.scsi_sense.asc   == 0x24
> xs->sense.scsi_sense.ascq  == 0x00
>
> This tells you that the command failed, you have retrieved valid
> sense_data, the device did think the command was illegal, the
> reason was that there was an invalid field in the command control
> block.
>
> The only place where you can handle this error is in the
> callback sd_interpret_sense where you have match this with
> the command in question. The command is xs->cmd and the
> command code is xs->cmd->opcode.

After sitting more over this, I came up with the patch below to analyze 
the whole situation more:

1) When I boot up the system with the cam not plugged in, I get none of 
the added message.

2) When I plug in the cam, I get:

 	HF: sd_interpret_sense: flags=0x6, asc=0x28, ascq=0x0

    which seems to be some "message buffer full" thing

3) when I run "disklabel sd0", I get:

 	HF: sd_interpret_sense: flags=0x5, asc=0x24, ascq=0x0
 	HF: OINK! SKEY_ILLEGAL_REQUEST/24/00
 	sd0(umass0:0:0): sdstart HF: scsipi_prevent #1 error=30
 	sd0(umass0:0:0): sdstart HF: scsipi_prevent #2 error=0
 	sd0(umass0:0:0): sdstart HF: scsipi_prevent #3 error=0

... but the same old error. Maybe just returning retval=EJUSTRETURN is bad 
here, but I have no idea what would be right.

Interesting enough, when I move the test to the end of 
sd_interpret_sense(), nothing is printed, and I guess something else exits 
the function before that... no idea what though.

Oh while here: all those magic numbers don't make that file easier to 
understand. :(


> Now, sdopen() specifies XS_CTL_IGNORE_ILLEGAL_REQUEST
> for scsipi_prevent() but this doesn't seem to be honored.

I saw that too... no idea.
Anybody got a clue on the whole scsipi framework? Manuel? :)


  - Hubert


Here's the debugging patch I added for the messages above:

Index: sd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/sd.c,v
retrieving revision 1.216.2.3
diff -u -r1.216.2.3 sd.c
--- sd.c	11 Sep 2004 12:55:11 -0000	1.216.2.3
+++ sd.c	19 Nov 2005 04:13:40 -0000
@@ -539,6 +539,7 @@
  			error = scsipi_prevent(periph, PR_PREVENT,
  			    XS_CTL_IGNORE_ILLEGAL_REQUEST |
  			    XS_CTL_IGNORE_MEDIA_CHANGE);
+printf("HF: scsipi_prevent #1 error=%d\n", error);
  			if (error)
  				goto bad;
  		}
@@ -600,10 +601,12 @@

  bad:
  	if (sd->sc_dk.dk_openmask == 0) {
-		if (periph->periph_flags & PERIPH_REMOVABLE)
-			scsipi_prevent(periph, PR_ALLOW,
+		if (periph->periph_flags & PERIPH_REMOVABLE) {
+			error = scsipi_prevent(periph, PR_ALLOW,
  			    XS_CTL_IGNORE_ILLEGAL_REQUEST |
  			    XS_CTL_IGNORE_MEDIA_CHANGE);
+printf("HF: scsipi_prevent #2 error=%d\n", error);
+		}
  		periph->periph_flags &= ~PERIPH_OPEN;
  	}

@@ -664,10 +667,12 @@

  		scsipi_wait_drain(periph);

-		if (periph->periph_flags & PERIPH_REMOVABLE)
-			scsipi_prevent(periph, PR_ALLOW,
+		if (periph->periph_flags & PERIPH_REMOVABLE) {
+			error = scsipi_prevent(periph, PR_ALLOW,
  			    XS_CTL_IGNORE_ILLEGAL_REQUEST |
  			    XS_CTL_IGNORE_NOT_READY);
+printf("HF: scsipi_prevent #3 error=%d\n", error);
+		}
  		periph->periph_flags &= ~PERIPH_OPEN;

  		scsipi_wait_drain(periph);
@@ -1175,8 +1180,10 @@
  		return (0);

  	case DIOCLOCK:
-		return (scsipi_prevent(periph,
+		error = (scsipi_prevent(periph,
  		    (*(int *)addr) ? PR_PREVENT : PR_ALLOW, 0));
+printf("HF: scsipi_prevent #4 error=%d\n", error);
+		return (error);

  	case DIOCEJECT:
  		if ((periph->periph_flags & PERIPH_REMOVABLE) == 0)
@@ -1191,6 +1198,7 @@
  			    sd->sc_dk.dk_openmask) {
  				error = scsipi_prevent(periph, PR_ALLOW,
  				    XS_CTL_IGNORE_NOT_READY);
+printf("HF: scsipi_prevent #5 error=%d\n", error);
  				if (error)
  					return (error);
  			} else {
@@ -1366,6 +1374,22 @@
  	struct sd_softc *sd = (void *)periph->periph_dev;
  	int s, error, retval = EJUSTRETURN;

+	printf("HF: sd_interpret_sense: flags=0x%x, asc=0x%x, ascq=0x%x\n",
+		sense->flags & SSD_KEY, sense->add_sense_code, 
+		sense->add_sense_code_qual);
+
+	/*
+	 * Ignore errors from accessing fields 
+	 * (e.g. trying to lock the door of a digicam, which doesn't
+	 *  have a door that can be locked - HF)
+	 */
+	if (((sense->flags & SSD_KEY) == SKEY_ILLEGAL_REQUEST) &&
+	    (sense->add_sense_code == 0x24) &&
+	    (sense->add_sense_code_qual == 0x00)) { /* Illegal field? *//*HF*/
+		printf("HF: OINK! SKEY_ILLEGAL_REQUEST/24/00\n");
+		return (retval); /* wild guess - HF */
+	}
+
  	/*
  	 * If the periph is already recovering, just do the normal
  	 * error processing.