Skip to contentSkip to conversation

Interoperability problem with Quantum CD72SH SATA tape drive and libata

PATCH Applied John Clark 2 participants 6 messages Sep 27, 2008 – Oct 7, 2008 libata

lore
Share & tools
lore

John Clark

author
#0 lore raw

Hello,

I've discovered an issue with the Quantum CD72SH SATA tape drive. I set it to variable block size, try to write a 5317 byte block with an ATAPI issued WRITE(6), and it hangs. The drive is connected to an Intel ICH9R in AHCI mode.

My debugging efforts have determined that it's a problem with the drive's firmware. It does not like ATAPI CDBs with odd (& 1) transfer lengths with PIO. If I let it use DMA, even if it's not a multiple of 16 bytes, then it seems to work:

libata-core.c 2008-09-27 16:11:29.000000000 -0400 renamed from libata-core.c.orig 2008-09-27 16:12:46.000000000 -0400 +2 -2
@@ -4669,8 +4669,8 @@
    /* Don't allow DMA if it isn't multiple of 16 bytes.  Quite a
     * few ATAPI devices choke on such DMA requests.
     */
-    if (unlikely(qc->nbytes & 15))
-        return 1;
+    // if (unlikely(qc->nbytes & 15))
+    //     return 1;
 
    if (ap->ops->check_atapi_dma)
        return ap->ops->check_atapi_dma(qc);

This is an acceptable solution for my limited use, but I'd like to see it work out of the box. It seems frightening that there are ATAPI devices out there that choke on PIO requests. What would you like to do?

Thank you.

- John

#1 lore raw
Quoted 28 lines · John Clark

Hello,

I've discovered an issue with the Quantum CD72SH SATA tape drive. I set it to variable block size, try to write a 5317 byte block with an ATAPI issued WRITE(6), and it hangs. The drive is connected to an Intel ICH9R in AHCI mode.

My debugging efforts have determined that it's a problem with the drive's firmware. It does not like ATAPI CDBs with odd (& 1) transfer lengths with PIO. If I let it use DMA, even if it's not a multiple of 16 bytes, then it seems to work:

libata-core.c 2008-09-27 16:11:29.000000000 -0400 renamed from libata-core.c.orig 2008-09-27 16:12:46.000000000 -0400 +2 -2
@@ -4669,8 +4669,8 @@
    /* Don't allow DMA if it isn't multiple of 16 bytes.  Quite a
     * few ATAPI devices choke on such DMA requests.
     */
-    if (unlikely(qc->nbytes & 15))
-        return 1;
+    // if (unlikely(qc->nbytes & 15))
+    //     return 1;
 
    if (ap->ops->check_atapi_dma)
        return ap->ops->check_atapi_dma(qc);
Quoted 3 lines

This is an acceptable solution for my limited use, but I'd like to see it work out of the box. It seems frightening that there are ATAPI devices out there that choke on PIO requests. What would you like to do?

We can create a HORKAGE for it. Can you please post the result of "hdparm -I" of the device?

Signature · 1 line
tejun
author
#2 lore raw
Quoted 30 lines · Tejun Heo
Quoted 28 lines · John Clark

Hello,

I've discovered an issue with the Quantum CD72SH SATA tape drive. I set it to variable block size, try to write a 5317 byte block with an ATAPI issued WRITE(6), and it hangs. The drive is connected to an Intel ICH9R in AHCI mode.

My debugging efforts have determined that it's a problem with the drive's firmware. It does not like ATAPI CDBs with odd (& 1) transfer lengths with PIO. If I let it use DMA, even if it's not a multiple of 16 bytes, then it seems to work:

libata-core.c 2008-09-27 16:11:29.000000000 -0400 renamed from libata-core.c.orig 2008-09-27 16:12:46.000000000 -0400 +2 -2
@@ -4669,8 +4669,8 @@
    /* Don't allow DMA if it isn't multiple of 16 bytes.  Quite a
     * few ATAPI devices choke on such DMA requests.
     */
-    if (unlikely(qc->nbytes & 15))
-        return 1;
+    // if (unlikely(qc->nbytes & 15))
+    //     return 1;
 
    if (ap->ops->check_atapi_dma)
        return ap->ops->check_atapi_dma(qc);
Quoted 9 lines
Quoted 4 lines

This is an acceptable solution for my limited use, but I'd like to see it work out of the box. It seems frightening that there are ATAPI devices out there that choke on PIO requests. What would you like to do?

We can create a HORKAGE for it. Can you please post the result of "hdparm -I" of the device?

Tejun,

Here it is:

/dev/nst0:

ATAPI Sequential-access device, with removable media

   Model Number:       QUANTUM DAT    DAT72-000               
   Serial Number:      JR001H1            
   Firmware Revision:  VC090000

Standards:
Used: ATA/ATAPI-7 T13 1532D revision 4a
Supported: 7 6 5 4
Configuration:
DRQ response: 50us.
Packet size: 12 bytes
Capabilities:
LBA, IORDY(can be disabled)
DMA: mdma0 mdma1 mdma2 udma0 udma1 udma2 udma3 udma4 udma5 *udma6
Cycle time: min=120ns recommended=120ns
PIO: pio0 pio1 pio2 pio3 pio4
Cycle time: no flow control=120ns IORDY flow control=120ns
Commands/features:
Enabled Supported:

      *    Power Management feature set
      *    PACKET command feature set
      *    DEVICE_RESET command
      *    NOP cmd
      *    SATA-I signaling speed (1.5Gb/s)

Integrity word not set (found 0x0000, expected 0x6da5)

Thank you.

- John

#3 lore raw
Quoted 4 lines · John Clark
Quoted 4 lines

We can create a HORKAGE for it. Can you please post the result of "hdparm -I" of the device?

Quoted 8 lines

Tejun,

Here it is:

/dev/nst0:

ATAPI Sequential-access device, with removable media
Model Number: QUANTUM DAT DAT72-000

Can you please test the attached patch?

Alan, John Clark is reporting that the above tape drive chokes on odd byte PIO transfer, so the default condition check (qc->nbytes & 15) in atapi_check_dma() breaks the SATA tape drive, so there's always one which breaks the other way around. :-(

For the time being, I think the horkage should do. I was wondering whether you still think we should lift the default mod16 check in atapi_check_dma().

Thanks.

Signature · 1 line
tejun
drivers/ata/libata-core.c +3 -1
index 1ee9499..e97afce 100644
@@ -4023,6 +4023,7 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 
 	/* Weird ATAPI devices */
 	{ "TORiSAN DVD-ROM DRD-N216", NULL,	ATA_HORKAGE_MAX_SEC_128 },
+	{ "QUANTUM DAT    DAT72-000", NULL,	ATA_HORKAGE_ATAPI_MOD16_DMA },
 
 	/* Devices we expect to fail diagnostics */
 
@@ -4436,7 +4437,8 @@ int atapi_check_dma(struct ata_queued_cmd *qc)
 	/* Don't allow DMA if it isn't multiple of 16 bytes.  Quite a
 	 * few ATAPI devices choke on such DMA requests.
 	 */
-	if (unlikely(qc->nbytes & 15))
+	if (!(qc->dev->horkage & ATA_HORKAGE_ATAPI_MOD16_DMA) &&
+	    unlikely(qc->nbytes & 15))
 		return 1;
 
 	if (ap->ops->check_atapi_dma)
include/linux/libata.h +2 -0
index 8bb81d0..9d903b3 100644
@@ -371,6 +371,8 @@ enum {
 	ATA_HORKAGE_IPM		= (1 << 7),	/* Link PM problems */
 	ATA_HORKAGE_IVB		= (1 << 8),	/* cbl det validity bit bugs */
 	ATA_HORKAGE_STUCK_ERR	= (1 << 9),	/* stuck ERR on next PACKET */
+	ATA_HORKAGE_ATAPI_MOD16_DMA = (1 << 10), /* use ATAPI DMA for commands
+						    not multiple of 16 bytes */
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */
author
#4 lore raw
Quoted 7 lines · Tejun Heo

Can you please test the attached patch?

Alan, John Clark is reporting that the above tape drive chokes on odd byte PIO transfer, so the default condition check (qc->nbytes & 15) in atapi_check_dma() breaks the SATA tape drive, so there's always one which breaks the other way around. :-(

Quoted 6 lines

For the time being, I think the horkage should do. I was wondering whether you still think we should lift the default mod16 check in atapi_check_dma().

Thanks.

Tejun,

I have tested the patch you supplied. It works, as expected.

Thanks.

- John

Applied
#5 lore raw
Quoted 12 lines · John Clark
Quoted 11 lines · Tejun Heo

Can you please test the attached patch?

Alan, John Clark is reporting that the above tape drive chokes on odd byte PIO transfer, so the default condition check (qc->nbytes & 15) in atapi_check_dma() breaks the SATA tape drive, so there's always one which breaks the other way around. :-(

For the time being, I think the horkage should do. I was wondering whether you still think we should lift the default mod16 check in atapi_check_dma().

Quoted 6 lines
Quoted 2 lines

Thanks.

Tejun,

I have tested the patch you supplied. It works, as expected.

applied to #tj-upstream. Thanks.

Signature · 1 line
tejun