Re: [PATCH 2.6.12-rc2 2/2] libata: add basic atapi error reporting

Re: [PATCH 2.6.12-rc2 2/2] libata: add basic atapi error reporting

[Eric A. Cottrell]

Hello Jeff,

Problem:
ATAPI errors are not fully reported to the SCSI subsystem.  A check condition was returned but the SCSI subsystem does not get the 
sense from the device.  This causes problems when unimplemented and bad SCSI commands are sent to an atapi device.

Solution:
In libata-scsi.c add a function that reads the error register and translates it to a basic sense block.  The error register on ATAPI 
devices contains the sense key, ILI, and EOM bits. These are the same as the third byte of a SCSI sense block.  Call the function 
when there is an ATAPI error.


Signed-off-by: Eric A. Cottrell  eac@shore.net



--- a/drivers/scsi/libata-scsi.c	2005-04-09 19:34:35.000000000 -0400
+++ b/drivers/scsi/libata-scsi.c	2005-04-16 19:40:14.000000000 -0400
@@ -613,6 +613,38 @@
  		sb[6] = tf->lbal;
  	}
  }
+/**
+ *	ata_gen_atapi_sense - generate a SCSI fixed sense block from ATAPI error
+ *	@qc: Command that we are erroring out
+ *
+ *
+ *	LOCKING:
+ *	inherited from caller
+ */
+void ata_gen_atapi_sense(struct ata_queued_cmd *qc)
+{
+	struct scsi_cmnd *cmd = qc->scsicmd;
+	struct ata_taskfile *tf = &qc->tf;
+	unsigned char *sb = cmd->sense_buffer;
+
+	memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
+
+	cmd->result = SAM_STAT_CHECK_CONDITION;
+
+	/*
+	 * Read the controller registers.
+	 */
+	assert(NULL != qc->ap->ops->tf_read);
+	qc->ap->ops->tf_read(qc->ap, tf);
+
+	if (unlikely(tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ))) {
+		sb[2] = (tf->feature >> 4) & 0x0f;  // Get sense key
+		sb[2] |= ((tf->feature & 0x3) << 5); // Get ILI and EOM
+	}
+
+	sb[0] = 0x70;
+	sb[7] = 0x00;  // No additional sense bytes
+}

  /**
   *	ata_scsi_slave_config - Set SCSI device attributes
@@ -1659,13 +1691,8 @@
  	struct scsi_cmnd *cmd = qc->scsicmd;

  	if (unlikely(drv_stat & (ATA_ERR | ATA_BUSY | ATA_DRQ))) {
-		DPRINTK("request check condition\n");
-
-		cmd->result = SAM_STAT_CHECK_CONDITION;
-
-		qc->scsidone(cmd);
-
-		return 1;
+		DPRINTK("generate atapi sense\n");
+		ata_gen_atapi_sense(qc);
  	} else {
  		u8 *scsicmd = cmd->cmnd;

Re: [PATCH 2.6.12-rc2 2/2] libata: add basic atapi error reporting

[Jeff Garzik]

Eric A. Cottrell wrote:
> Hello Jeff,
> 
> Problem:
> ATAPI errors are not fully reported to the SCSI subsystem.  A check 
> condition was returned but the SCSI subsystem does not get the sense 
> from the device.  This causes problems when unimplemented and bad SCSI 
> commands are sent to an atapi device.

This is a bit strange.  It should get the sense from the device.  It is 
requested from ata_qc_timeout().  Is this not happening for you?

	Jeff

Re: [PATCH 2.6.12-rc2 2/2] libata: add basic atapi error reporting

[Mark Lord]

Jeff Garzik wrote:
> Eric A. Cottrell wrote:
>> condition was returned but the SCSI subsystem does not get the sense 
>> from the device.  This causes problems when unimplemented and bad SCSI 
>> commands are sent to an atapi device.
> 
> This is a bit strange.  It should get the sense from the device.  It is 
> requested from ata_qc_timeout().  Is this not happening for you?

This could happen as a result of the _eh_ race condition issues
that we're working on right now, I suppose.

I wonder if the patch from Hannes Reinecke fixes this?

Re: [PATCH 2.6.12-rc2 2/2] libata: add basic atapi error reporting

[Eric A. Cottrell]

Jeff Garzik wrote:
> Eric A. Cottrell wrote:
>> Hello Jeff,
>>
>> Problem:
>> ATAPI errors are not fully reported to the SCSI subsystem.  A check 
>> condition was returned but the SCSI subsystem does not get the sense 
>> from the device.  This causes problems when unimplemented and bad SCSI 
>> commands are sent to an atapi device.
> 
> This is a bit strange.  It should get the sense from the device.  It is 
> requested from ata_qc_timeout().  Is this not happening for you?
> 
>     Jeff
> 
> 
> 
Hello,

It does not seem to work in all cases.  I had the most trouble with the 
AHCI driver.  I wish to withdraw part 2 of the patch because I realize 
that this is only an incomplete solution.  Full sense information needs 
to be sent to the SCSI subsystem for it to work properly.  I was able to 
modify the driver to Autosense on ATAPI errors and improved it.  It is 
not complete yet.

I am still having problems.  I can burn a CD but not a DVD.  On a 
working drive there is a long delay as the DVD lead-in is written then 
the written data amount will increase.  With libata it appears that 
something is timing out way too quickly (within a second) and the 
burning program stops with an error.

I need to setup git and grab the libata-dev stuff before I continue.

73 Eric