Re: libata machine check on Alpha

Re: libata machine check on Alpha

[Jonathan Benson]

 >Hi Jonathan,

 >Could you please give a try to the attached patch and post the  
content of word 62?
 >If word 62 + SiI 3611 looks good, we can turn on the DMADIR  
automatically. Thanks!

Per Albert's request, I've tested his patch to check the status of  
word 62 with the SiI 3611 bridge chip.  I used kernel version 2.6.17- 
rc1, as 2.6.16 reported fuzz when I applied the patch.  2.6.17-rc1  
reported an offset, but applied without any errors.  I've tested with  
the ATAPI_ENABLE_DMADIR workaround enabled, and disabled.  Debugging  
is disabled.  Looks like word 62 returns 0 as its value, dmesg with  
the relevant parts attached below.

If you need any more information, let me know and I'll be happy to  
provide it.

Jonathan Benson

Workaround Enabled

libata version 1.20 loaded.
sata_sil 0000:00:09.0: version 0.9
sata_sil 0000:00:09.0: cache line size not set.  Driver may not function
sata_sil 0000:00:09.0: Applying R_ERR on DMA activate FIS errata fix
PCI: Setting latency timer of device 0000:00:09.0 to 64
ata1: SATA max UDMA/100 cmd 0xFFFFFC88093C1080 ctl 0xFFFFFC88093C108A  
bmdma 0xFFFFFC88093C1000 irq 19
ata2: SATA max UDMA/100 cmd 0xFFFFFC88093C10C0 ctl 0xFFFFFC88093C10CA  
bmdma 0xFFFFFC88093C1008 irq 19
ata3: SATA max UDMA/100 cmd 0xFFFFFC88093C1280 ctl 0xFFFFFC88093C128A  
bmdma 0xFFFFFC88093C1200 irq 19
ata4: SATA max UDMA/100 cmd 0xFFFFFC88093C12C0 ctl 0xFFFFFC88093C12CA  
bmdma 0xFFFFFC88093C1208 irq 19
ata1: SATA link up 1.5 Gbps (SStatus 113)
ata1: dev 0 cfg 49:2f00 82:7c6b 83:7b09 84:4003 85:7c68 86:3a01  
87:4003 88:007f
ata1: dev 0 ATA-7, max UDMA/133, 240121728 sectors: LBA
ata1(0): applying bridge limits
ata1: dev 0 configured for UDMA/100
scsi0 : sata_sil
ata2: SATA link up 1.5 Gbps (SStatus 113)
ata2: dev 0 cfg 49:2f00 82:7c6b 83:7b09 84:4003 85:7c69 86:3a01  
87:4003 88:007f
ata2: dev 0 ATA-7, max UDMA/133, 240121728 sectors: LBA
ata2(0): applying bridge limits
ata2: dev 0 configured for UDMA/100
scsi1 : sata_sil
ata3: SATA link up 1.5 Gbps (SStatus 113)
ata3: dev 0 cfg 49:2f00 82:7c6b 83:7b09 84:4003 85:7c69 86:3a01  
87:4003 88:007f
ata3: dev 0 ATA-7, max UDMA/133, 240121728 sectors: LBA
ata3(0): applying bridge limits
ata3: dev 0 configured for UDMA/100
scsi2 : sata_sil
ata4: SATA link up 1.5 Gbps (SStatus 113)
ata4: dev 0 cfg 49:0b00 82:0210 83:1000 84:0000 85:0000 86:0000  
87:0000 88:0007
ata4: dev 0 word 62 [0]
ata4: dev 0 ATAPI, max UDMA/33
ata4: dev 0 word 62 [0]
ata4: dev 0 configured for UDMA/33
scsi3 : sata_sil
   Vendor: ATA       Model: Maxtor 6Y120P0    Rev: YAR4
   Type:   Direct-Access                      ANSI SCSI revision: 05
   Vendor: ATA       Model: Maxtor 6Y120P0    Rev: YAR4
   Type:   Direct-Access                      ANSI SCSI revision: 05
   Vendor: ATA       Model: Maxtor 6Y120P0    Rev: YAR4
   Type:   Direct-Access                      ANSI SCSI revision: 05
   Vendor: LITE-ON   Model: DVD SOHD-167T     Rev: 9S16
   Type:   CD-ROM                             ANSI SCSI revision: 05
SCSI device sda: 240121728 512-byte hdwr sectors (122942 MB)
sda: Write Protect is off
sda: Mode Sense: 00 3a 00 00
SCSI device sda: drive cache: write back
SCSI device sda: 240121728 512-byte hdwr sectors (122942 MB)
sda: Write Protect is off
sda: Mode Sense: 00 3a 00 00
SCSI device sda: drive cache: write back
sda: sda1
sd 0:0:0:0: Attached scsi disk sda
SCSI device sdb: 240121728 512-byte hdwr sectors (122942 MB)
sdb: Write Protect is off
sdb: Mode Sense: 00 3a 00 00
SCSI device sdb: drive cache: write back
SCSI device sdb: 240121728 512-byte hdwr sectors (122942 MB)
sdb: Write Protect is off
sdb: Mode Sense: 00 3a 00 00
SCSI device sdb: drive cache: write back
sdb: sdb1
sd 1:0:0:0: Attached scsi disk sdb
SCSI device sdc: 240121728 512-byte hdwr sectors (122942 MB)
sdc: Write Protect is off
sdc: Mode Sense: 00 3a 00 00
SCSI device sdc: drive cache: write back
SCSI device sdc: 240121728 512-byte hdwr sectors (122942 MB)
sdc: Write Protect is off
sdc: Mode Sense: 00 3a 00 00
SCSI device sdc: drive cache: write back
sdc: sdc1
sd 2:0:0:0: Attached scsi disk sdc
sr0: scsi3-mmc drive: 48x/48x cd/rw xa/form2 cdda tray
Uniform CD-ROM driver Revision: 3.20
sr 3:0:0:0: Attached scsi CD-ROM sr0
sd 0:0:0:0: Attached scsi generic sg0 type 0
sd 1:0:0:0: Attached scsi generic sg1 type 0
sd 2:0:0:0: Attached scsi generic sg2 type 0
sr 3:0:0:0: Attached scsi generic sg3 type 5

Workaround Disabled

libata version 1.20 loaded.
sata_sil 0000:00:09.0: version 0.9
sata_sil 0000:00:09.0: cache line size not set.  Driver may not function
sata_sil 0000:00:09.0: Applying R_ERR on DMA activate FIS errata fix
PCI: Setting latency timer of device 0000:00:09.0 to 64
ata1: SATA max UDMA/100 cmd 0xFFFFFC88093C1080 ctl 0xFFFFFC88093C108A  
bmdma 0xFFFFFC88093C1000 irq 19
ata2: SATA max UDMA/100 cmd 0xFFFFFC88093C10C0 ctl 0xFFFFFC88093C10CA  
bmdma 0xFFFFFC88093C1008 irq 19
ata3: SATA max UDMA/100 cmd 0xFFFFFC88093C1280 ctl 0xFFFFFC88093C128A  
bmdma 0xFFFFFC88093C1200 irq 19
ata4: SATA max UDMA/100 cmd 0xFFFFFC88093C12C0 ctl 0xFFFFFC88093C12CA  
bmdma 0xFFFFFC88093C1208 irq 19
ata1: SATA link up 1.5 Gbps (SStatus 113)
ata1: dev 0 cfg 49:2f00 82:7c6b 83:7b09 84:4003 85:7c68 86:3a01  
87:4003 88:007f
ata1: dev 0 ATA-7, max UDMA/133, 240121728 sectors: LBA
ata1(0): applying bridge limits
ata1: dev 0 configured for UDMA/100
scsi0 : sata_sil
ata2: SATA link up 1.5 Gbps (SStatus 113)
ata2: dev 0 cfg 49:2f00 82:7c6b 83:7b09 84:4003 85:7c69 86:3a01  
87:4003 88:007f
ata2: dev 0 ATA-7, max UDMA/133, 240121728 sectors: LBA
ata2(0): applying bridge limits
ata2: dev 0 configured for UDMA/100
scsi1 : sata_sil
ata3: SATA link up 1.5 Gbps (SStatus 113)
ata3: dev 0 cfg 49:2f00 82:7c6b 83:7b09 84:4003 85:7c69 86:3a01  
87:4003 88:007f
ata3: dev 0 ATA-7, max UDMA/133, 240121728 sectors: LBA
ata3(0): applying bridge limits
ata3: dev 0 configured for UDMA/100
scsi2 : sata_sil
ata4: SATA link up 1.5 Gbps (SStatus 113)
ata4: dev 0 cfg 49:0b00 82:0210 83:1000 84:0000 85:0000 86:0000  
87:0000 88:0007
ata4: dev 0 word 62 [0]
ata4: dev 0 ATAPI, max UDMA/33
ata4: dev 0 word 62 [0]
ata4: dev 0 configured for UDMA/33
scsi3 : sata_sil
   Vendor: ATA       Model: Maxtor 6Y120P0    Rev: YAR4
   Type:   Direct-Access                      ANSI SCSI revision: 05
   Vendor: ATA       Model: Maxtor 6Y120P0    Rev: YAR4
   Type:   Direct-Access                      ANSI SCSI revision: 05
   Vendor: ATA       Model: Maxtor 6Y120P0    Rev: YAR4
   Type:   Direct-Access                      ANSI SCSI revision: 05
CIA machine check: vector=0x670 pc=0xfffffc00005330e0 code=0x98
machine check type: processor detected hard error
pc = [<fffffc00005330e0>]  ra = [<fffffc000052d148>]  ps = 0007     
Not tainted
pc is at ata_altstatus+0x30/0x90
ra is at ata_qc_timeout+0x178/0x190
v0 = 0000000000000001  t0 = fffffc88093c12ca  t1 = fffffc88093c1208
t2 = fffffc8900000000  t3 = 0000000020010058  t4 = 0000000000000000
t5 = 00000000256d7116  t6 = 00000000fffb8c53  t7 = fffffc001fc20000
a0 = fffffc001fedec18  a1 = fffffc001fc23a48  a2 = 0000000000000318
a3 = fffffc0000610722  a4 = 0000000000000000  a5 = 0000000000000001
t8 = 000000000000001f  t9 = 000000000008efbe  t10= 0000000000000000
t11= 0000000000000000  pv = 0000000000000000  at = 0000000000000000
gp = fffffc000075f400  sp = fffffc001fc23a88
ata4: command 0xa0 timeout, stat 0xf8 host_stat 0x0
ata4: translated ATA stat/err 0xf8/00 to SCSI SK/ASC/ASCQ 0xb/47/00
SCSI device sda: 240121728 512-byte hdwr sectors (122942 MB)
sda: Write Protect is off
sda: Mode Sense: 00 3a 00 00
SCSI device sda: drive cache: write back
SCSI device sda: 240121728 512-byte hdwr sectors (122942 MB)
sda: Write Protect is off
sda: Mode Sense: 00 3a 00 00
SCSI device sda: drive cache: write back
sda: sda1
sd 0:0:0:0: Attached scsi disk sda
SCSI device sdb: 240121728 512-byte hdwr sectors (122942 MB)
sdb: Write Protect is off
sdb: Mode Sense: 00 3a 00 00
SCSI device sdb: drive cache: write back
SCSI device sdb: 240121728 512-byte hdwr sectors (122942 MB)
sdb: Write Protect is off
sdb: Mode Sense: 00 3a 00 00
SCSI device sdb: drive cache: write back
sdb: sdb1
sd 1:0:0:0: Attached scsi disk sdb
SCSI device sdc: 240121728 512-byte hdwr sectors (122942 MB)
sdc: Write Protect is off
sdc: Mode Sense: 00 3a 00 00
SCSI device sdc: drive cache: write back
SCSI device sdc: 240121728 512-byte hdwr sectors (122942 MB)
sdc: Write Protect is off
sdc: Mode Sense: 00 3a 00 00
SCSI device sdc: drive cache: write back
sdc: sdc1
sd 2:0:0:0: Attached scsi disk sdc
sd 0:0:0:0: Attached scsi generic sg0 type 0
sd 1:0:0:0: Attached scsi generic sg1 type 0
sd 2:0:0:0: Attached scsi generic sg2 type 0

Re: libata machine check on Alpha

[Albert Lee]

Jonathan Benson wrote:
> 
>>Could you please give a try to the attached patch and post the  content
> of word 62?
>>If word 62 + SiI 3611 looks good, we can turn on the DMADIR 
> automatically. Thanks!
> 
> Per Albert's request, I've tested his patch to check the status of  word
> 62 with the SiI 3611 bridge chip.  I used kernel version 2.6.17- rc1, as
> 2.6.16 reported fuzz when I applied the patch.  2.6.17-rc1  reported an
> offset, but applied without any errors.  I've tested with  the
> ATAPI_ENABLE_DMADIR workaround enabled, and disabled.  Debugging  is
> disabled.  Looks like word 62 returns 0 as its value, dmesg with  the
> relevant parts attached below.
> 
> If you need any more information, let me know and I'll be happy to 
> provide it.
> 
> Jonathan Benson
> 
> (snip)
> 
> ata2: dev 0 ATA-7, max UDMA/133, 240121728 sectors: LBA
> ata2(0): applying bridge limits
> ata2: dev 0 configured for UDMA/100
> scsi1 : sata_sil
> ata3: SATA link up 1.5 Gbps (SStatus 113)
> ata3: dev 0 cfg 49:2f00 82:7c6b 83:7b09 84:4003 85:7c69 86:3a01  87:4003
> 88:007f
> ata3: dev 0 ATA-7, max UDMA/133, 240121728 sectors: LBA
> ata3(0): applying bridge limits
> ata3: dev 0 configured for UDMA/100
> scsi2 : sata_sil
> ata4: SATA link up 1.5 Gbps (SStatus 113)
> ata4: dev 0 cfg 49:0b00 82:0210 83:1000 84:0000 85:0000 86:0000  87:0000
> 88:0007
> ata4: dev 0 word 62 [0]
> ata4: dev 0 ATAPI, max UDMA/33
> ata4: dev 0 word 62 [0]
> ata4: dev 0 configured for UDMA/33

Word 62 is also zero on my JMicron 20330 bridge.
Since JMicron doesn't need the DMA DIR bit, it looks consistent
(maybe by coincidence) with the ATA-7 spec.

For SiI 3611, it was made before the ATA-7 spec is finalized. So, it is
reasonable that SiI 3611 doesn't implement word 62 and we have to 
manually enable the DMA DIR support as work around unless SiImage provides
other solution to identify the SiI 3611 bridge.

Thanks for the test result. :)
--
albert

[PATCH/RFC] libata: turn on the ATAPI DMA DIR support per word 62

[Albert Lee]

Turn on the ATAPI DMA DIR support if word 62 indicates it.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
ATAPI DMA DIR follow-up patch to turn on the DMA_DIR support automatically
by checking identify device word 62. (Thanks for Jeff and Tejun's pointer.)

According to Jonathan's test result, SiI 3611 (the current known bridge that
requires the ATAPI DMA DIR support) doesn't implement word 62. So, the
atapi_dmadir parameter is preserved to enable the DMA DIR support manually as
work around.

Patch against upstream (c2a6585296009379e0f4eff39cdcb108b457ebf2).
For your review, thanks.

--- upstream0/include/linux/ata.h	2006-04-06 16:07:36.000000000 +0800
+++ upstream1/include/linux/ata.h	2006-04-06 16:50:54.000000000 +0800
@@ -264,6 +264,7 @@ struct ata_taskfile {
 #define ata_id_has_dma(id)	((id)[49] & (1 << 8))
 #define ata_id_removeable(id)	((id)[0] & (1 << 7))
 #define ata_id_has_dword_io(id)	((id)[50] & (1 << 0))
+#define ata_id_dmadir_needed(id) ((id)[62] & (1 << 15))
 #define ata_id_u32(id,n)	\
 	(((u32) (id)[(n) + 1] << 16) | ((u32) (id)[(n)]))
 #define ata_id_u64(id,n)	\
--- upstream0/include/linux/libata.h	2006-04-06 16:07:37.000000000 +0800
+++ upstream1/include/linux/libata.h	2006-04-06 16:53:35.000000000 +0800
@@ -121,6 +121,7 @@ enum {
 	/* struct ata_device stuff */
 	ATA_DFLAG_LBA		= (1 << 0), /* device supports LBA */
 	ATA_DFLAG_LBA48		= (1 << 1), /* device supports LBA48 */
+	ATA_DFLAG_DMADIR	= (1 << 2), /* device requires ATAPI DMADIR */
 	ATA_DFLAG_CFG_MASK	= (1 << 8) - 1,
 
 	ATA_DFLAG_PIO		= (1 << 8), /* device currently in PIO mode */
--- upstream0/drivers/scsi/libata-scsi.c	2006-04-06 16:07:32.000000000 +0800
+++ upstream1/drivers/scsi/libata-scsi.c	2006-04-07 13:54:28.000000000 +0800
@@ -2163,9 +2163,12 @@ static unsigned int atapi_xlat(struct at
 		qc->tf.protocol = ATA_PROT_ATAPI_DMA;
 		qc->tf.feature |= ATAPI_PKT_DMA;
 
-		if (atapi_dmadir && (cmd->sc_data_direction != DMA_TO_DEVICE))
-			/* some SATA bridges need us to indicate data xfer direction */
-			qc->tf.feature |= ATAPI_DMADIR;
+		if ((dev->flags & ATA_DFLAG_DMADIR) || atapi_dmadir)
+			/* some SATA bridges need us to indicate
+			 * data xfer direction
+			 */
+			if (cmd->sc_data_direction != DMA_TO_DEVICE)
+				qc->tf.feature |= ATAPI_DMADIR;
 	}
 
 	qc->nbytes = cmd->bufflen;
--- upstream0/drivers/scsi/libata-core.c	2006-04-06 16:07:32.000000000 +0800
+++ upstream1/drivers/scsi/libata-core.c	2006-04-07 13:59:01.000000000 +0800
@@ -78,7 +78,7 @@ MODULE_PARM_DESC(atapi_enabled, "Enable 
 
 int atapi_dmadir = 0;
 module_param(atapi_dmadir, int, 0444);
-MODULE_PARM_DESC(atapi_dmadir, "Enable ATAPI DMADIR bridge support (0=off, 1=on)");
+MODULE_PARM_DESC(atapi_dmadir, "Manually enable ATAPI DMADIR bridge support (0=off, 1=on)");
 
 int libata_fua = 0;
 module_param_named(fua, libata_fua, int, 0444);
@@ -1322,6 +1322,9 @@ static int ata_dev_configure(struct ata_
 		}
 		dev->cdb_len = (unsigned int) rc;
 
+		if (ata_id_dmadir_needed(id))
+			dev->flags |= ATA_DFLAG_DMADIR;
+
 		/* print device info to dmesg */
 		if (print_info)
 			printk(KERN_INFO "ata%u: dev %u ATAPI, max %s\n",

Re: [PATCH/RFC] libata: turn on the ATAPI DMA DIR support per word 62

[Jeff Garzik]

Albert Lee wrote:
> Turn on the ATAPI DMA DIR support if word 62 indicates it.
> 
> Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
> ---
> ATAPI DMA DIR follow-up patch to turn on the DMA_DIR support automatically
> by checking identify device word 62. (Thanks for Jeff and Tejun's pointer.)
> 
> According to Jonathan's test result, SiI 3611 (the current known bridge that
> requires the ATAPI DMA DIR support) doesn't implement word 62. So, the
> atapi_dmadir parameter is preserved to enable the DMA DIR support manually as
> work around.

Two comments:

* I would like to find a device that's compliant with the spec, and test 
the patch, before committing.

* DMADIR not only includes a bit flag indicating its presence, it also 
moves all the DMA capability bits from their standard places.  You'll 
have to audit every place that reads dev->id[]'s mwdma/udma masks and 
make sure they look at the DMADIR-special-case location, when DMADIR is set.

	Jeff

Re: [PATCH/RFC] libata: turn on the ATAPI DMA DIR support per word 62

[Albert Lee]

Jeff Garzik wrote:
> 
> Two comments:
> 
> * I would like to find a device that's compliant with the spec, and test
> the patch, before committing.

I'm trying to get an Acard AEC-7900A and a Sunplus 3811A (which looks like
a SiI 3811 rebrand) for test. Since the DMADIR is SiImage's idea,
hopefully the Sunplus 3811A may support DMADIR. For bridges from other
vendors, I guess the chance is low.

> 
> * DMADIR not only includes a bit flag indicating its presence, it also
> moves all the DMA capability bits from their standard places.  You'll
> have to audit every place that reads dev->id[]'s mwdma/udma masks and
> make sure they look at the DMADIR-special-case location, when DMADIR is
> set.
> 

Fortunately only ata_id_xfermask() uses the mwdma/udma bits. Revised
patch to follow. Thanks for the advice.

--
albert

[PATCH/RFC] libata: turn on the ATAPI DMADIR support per word 62 (revised)

[Albert Lee]

Turn on the ATAPI DMADIR support if word 62 indicates it.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
(Revised to add mwdma/udma mask of word 62 per Jeff's comments.
Need more testing once I get the SiI 3811, etc.)

ATAPI DMADIR follow-up patch to turn on the DMADIR support automatically
by checking identify device word 62. (Thanks for Jeff and Tejun's pointer.)

According to Jonathan's test result, SiI 3611 (the current known bridge that
requires the ATAPI DMADIR support) doesn't implement word 62. So, the
atapi_dmadir parameter is preserved to enable the DMA DIR support manually as
work around.

Patch against upstream (c2a6585296009379e0f4eff39cdcb108b457ebf2).
For your review, thanks.

--- upstream0/include/linux/ata.h	2006-04-06 16:07:36.000000000 +0800
+++ upstream1/include/linux/ata.h	2006-04-07 17:52:24.000000000 +0800
@@ -47,6 +47,7 @@ enum {
 	ATA_ID_PROD_OFS		= 27,
 	ATA_ID_OLD_PIO_MODES	= 51,
 	ATA_ID_FIELD_VALID	= 53,
+	ATA_ID_DMADIR		= 62,
 	ATA_ID_MWDMA_MODES	= 63,
 	ATA_ID_PIO_MODES	= 64,
 	ATA_ID_EIDE_DMA_MIN	= 65,
@@ -264,6 +265,7 @@ struct ata_taskfile {
 #define ata_id_has_dma(id)	((id)[49] & (1 << 8))
 #define ata_id_removeable(id)	((id)[0] & (1 << 7))
 #define ata_id_has_dword_io(id)	((id)[50] & (1 << 0))
+#define ata_id_dmadir_needed(id) ((id)[62] & (1 << 15))
 #define ata_id_u32(id,n)	\
 	(((u32) (id)[(n) + 1] << 16) | ((u32) (id)[(n)]))
 #define ata_id_u64(id,n)	\
--- upstream0/include/linux/libata.h	2006-04-06 16:07:37.000000000 +0800
+++ upstream1/include/linux/libata.h	2006-04-07 17:53:19.000000000 +0800
@@ -121,6 +121,7 @@ enum {
 	/* struct ata_device stuff */
 	ATA_DFLAG_LBA		= (1 << 0), /* device supports LBA */
 	ATA_DFLAG_LBA48		= (1 << 1), /* device supports LBA48 */
+	ATA_DFLAG_DMADIR	= (1 << 2), /* device requires ATAPI DMADIR */
 	ATA_DFLAG_CFG_MASK	= (1 << 8) - 1,
 
 	ATA_DFLAG_PIO		= (1 << 8), /* device currently in PIO mode */
--- upstream0/drivers/scsi/libata-scsi.c	2006-04-06 16:07:32.000000000 +0800
+++ upstream1/drivers/scsi/libata-scsi.c	2006-04-07 17:54:06.000000000 +0800
@@ -2163,9 +2163,12 @@ static unsigned int atapi_xlat(struct at
 		qc->tf.protocol = ATA_PROT_ATAPI_DMA;
 		qc->tf.feature |= ATAPI_PKT_DMA;
 
-		if (atapi_dmadir && (cmd->sc_data_direction != DMA_TO_DEVICE))
-			/* some SATA bridges need us to indicate data xfer direction */
-			qc->tf.feature |= ATAPI_DMADIR;
+		if ((dev->flags & ATA_DFLAG_DMADIR) || atapi_dmadir)
+			/* some SATA bridges need us to indicate
+			 * data xfer direction
+			 */
+			if (cmd->sc_data_direction != DMA_TO_DEVICE)
+				qc->tf.feature |= ATAPI_DMADIR;
 	}
 
 	qc->nbytes = cmd->bufflen;
--- upstream0/drivers/scsi/libata-core.c	2006-04-06 16:07:32.000000000 +0800
+++ upstream1/drivers/scsi/libata-core.c	2006-04-07 18:35:38.000000000 +0800
@@ -78,7 +78,7 @@ MODULE_PARM_DESC(atapi_enabled, "Enable 
 
 int atapi_dmadir = 0;
 module_param(atapi_dmadir, int, 0444);
-MODULE_PARM_DESC(atapi_dmadir, "Enable ATAPI DMADIR bridge support (0=off, 1=on)");
+MODULE_PARM_DESC(atapi_dmadir, "Manually enable ATAPI DMADIR bridge support (0=off, 1=on)");
 
 int libata_fua = 0;
 module_param_named(fua, libata_fua, int, 0444);
@@ -831,6 +831,7 @@ static inline void ata_dump_id(const u16
 /**
  *	ata_id_xfermask - Compute xfermask from the given IDENTIFY data
  *	@id: IDENTIFY data to compute xfer mask from
+ *	@class: class of attached device
  *
  *	Compute the xfermask for this device. This is not as trivial
  *	as it seems if we must consider early devices correctly.
@@ -843,7 +844,7 @@ static inline void ata_dump_id(const u16
  *	RETURNS:
  *	Computed xfermask
  */
-static unsigned int ata_id_xfermask(const u16 *id)
+static unsigned int ata_id_xfermask(const u16 *id, unsigned int class)
 {
 	unsigned int pio_mask, mwdma_mask, udma_mask;
 
@@ -873,6 +874,23 @@ static unsigned int ata_id_xfermask(cons
 	if (id[ATA_ID_FIELD_VALID] & (1 << 2))
 		udma_mask = id[ATA_ID_UDMA_MODES] & 0xff;
 
+	if (class == ATA_DEV_ATAPI && ata_id_dmadir_needed(id)) {
+		/* To prevent the pre ATA-7 host software (which is
+		 * not aware of the DMADIR word) from using ATAPI DMA
+		 * and causing trouble, the bridge mangles id[49],
+		 * id[63] and id[88] to report no DMA support.
+		 * 
+		 * We turn on DMA here if word 62 indicates it.
+		 * (Per ATA-7, DMADIR is only used for S-ATAPI
+		 * devices, therefore bits 0-10 are set to 1.
+		 * Anyway, we just use these bits as is.)
+		 */
+		WARN_ON(mwdma_mask || udma_mask);
+
+		mwdma_mask = (id[ATA_ID_DMADIR] >> 7) & 0x07;
+		udma_mask = id[ATA_ID_DMADIR] & 0x7f;
+	}
+
 	return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
 }
 
@@ -1256,7 +1274,7 @@ static int ata_dev_configure(struct ata_
 	 */
 
 	/* find max transfer mode; for printk only */
-	xfer_mask = ata_id_xfermask(id);
+	xfer_mask = ata_id_xfermask(id, dev->class);
 
 	ata_dump_id(id);
 
@@ -1322,6 +1340,9 @@ static int ata_dev_configure(struct ata_
 		}
 		dev->cdb_len = (unsigned int) rc;
 
+		if (ata_id_dmadir_needed(id))
+			dev->flags |= ATA_DFLAG_DMADIR;
+
 		/* print device info to dmesg */
 		if (print_info)
 			printk(KERN_INFO "ata%u: dev %u ATAPI, max %s\n",
@@ -2941,7 +2962,7 @@ static void ata_dev_xfermask(struct ata_
 
 		xfer_mask &= ata_pack_xfermask(d->pio_mask,
 					       d->mwdma_mask, d->udma_mask);
-		xfer_mask &= ata_id_xfermask(d->id);
+		xfer_mask &= ata_id_xfermask(d->id, d->class);
 		if (ata_dma_blacklisted(d))
 			xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
 	}

Re: [PATCH/RFC] libata: turn on the ATAPI DMADIR support per word 62 (revised)

[Jeff Garzik]

Albert Lee wrote:
> Turn on the ATAPI DMADIR support if word 62 indicates it.
> 
> Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
> ---
> (Revised to add mwdma/udma mask of word 62 per Jeff's comments.
> Need more testing once I get the SiI 3811, etc.)
> 
> ATAPI DMADIR follow-up patch to turn on the DMADIR support automatically
> by checking identify device word 62. (Thanks for Jeff and Tejun's pointer.)
> 
> According to Jonathan's test result, SiI 3611 (the current known bridge that
> requires the ATAPI DMADIR support) doesn't implement word 62. So, the
> atapi_dmadir parameter is preserved to enable the DMA DIR support manually as
> work around.
> 
> Patch against upstream (c2a6585296009379e0f4eff39cdcb108b457ebf2).
> For your review, thanks.

I ACK the patch, though like I said, I would like to see this tested 
first with some compliant devices.

	Jeff

Re: [PATCH/RFC] libata: turn on the ATAPI DMADIR support per word 62 (revised)

[Albert Lee]

Jeff Garzik wrote:
> Albert Lee wrote:
> 
>> Turn on the ATAPI DMADIR support if word 62 indicates it.
>>
>> Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
>> ---
>> (Revised to add mwdma/udma mask of word 62 per Jeff's comments.
>> Need more testing once I get the SiI 3811, etc.)
>>
>> ATAPI DMADIR follow-up patch to turn on the DMADIR support automatically
>> by checking identify device word 62. (Thanks for Jeff and Tejun's
>> pointer.)
>>
>> According to Jonathan's test result, SiI 3611 (the current known
>> bridge that
>> requires the ATAPI DMADIR support) doesn't implement word 62. So, the
>> atapi_dmadir parameter is preserved to enable the DMA DIR support
>> manually as
>> work around.
>>
>> Patch against upstream (c2a6585296009379e0f4eff39cdcb108b457ebf2).
>> For your review, thanks.
> 
> 
> I ACK the patch, though like I said, I would like to see this tested
> first with some compliant devices.
> 

I've collected/tested most of the PATA-to-SATA bridges, none of them 
implements the word 62 behavior as defined in ATA-7.
So, maybe we don't need the word 62 patch at this moment.
(And I guess few vendor is going to implement it in the future:
depending on host software for the DMA direction isn't too backward
compatible with old host software.)

The following are test results:
(Tested with SiI 3112 SATA adapter + LITEON SOHR-5238S CD-RW drive.)

1. SiI 3611:
   - ATAPI DMA supported (DMADIR needed)
   - id[62] == 0
   - IDENTIFY PACKET DEVICE unchanged

2. SiI 3811:
   There is a jumper on board. When the jumper installed (default),
   DMADIR is not needed.
   - ATAPI DMA supported (DMADIR not needed)
   - id[62] == 0
   - IDENTIFY PACKET DEVICE unchanged

   When the jumper is removed, SiI 3811 behaves like SiI 3611
   and needs the DMADIR bit for ATAPI DMA to work.

3. Marvell 88i8030:
   - ATAPI not supported. Device timeout on probe.

4. Marvell 88SA8040:
   - ATAPI DMA supported (DMADIR not needed)
   - id[62] == 0
   - IDENTIFY PACKET DEVICE unchanged

5. Marvell 88SA8050:
   - Not tested

6. Acard ARC770:
   - ATAPI DMA supported (DMADIR not needed)
   - id[62] == 0
   - IDENTIFY PACKET DEVICE mangled to report "no MWDMA support".
     (UDMA is supported.)

7. JMicron JM20330:
   - ATAPI DMA supported (DMADIR not needed)
   - id[62] == 0
   - IDENTIFY PACKET DEVICE mangled. id[76] == 0x0202 id[78] == 0008.
     (Don't know why yet. Maybe device signature?)

Summary:
  SiI 3611 is currently the only known chip that needs DMADIR.
Since SiI 3611 doesn't implement the ATA-7 word 62 or mangle identify
packet device data, it's hard to check the chip and turn on the DMADIR
support automatically. (Currently the libata DMADIR support can be 
turned on manually by module parameter.)

To turn on the DMADIR support automatically, maybe we can check whether
the ATAPI device is bridged (by ata_dev_knobble()). If bridged, we can
try to issue a ATAPI DMA read command to check if ATAPI DMA works.
If not work, then try turning on DMADIR or turn off ATAPI DMA.
--
albert
     

Re: [PATCH/RFC] libata: turn on the ATAPI DMADIR support per word 62 (revised)

[Jeff Garzik]

Albert Lee wrote:
> Summary:
>   SiI 3611 is currently the only known chip that needs DMADIR.
> Since SiI 3611 doesn't implement the ATA-7 word 62 or mangle identify
> packet device data, it's hard to check the chip and turn on the DMADIR
> support automatically. (Currently the libata DMADIR support can be 
> turned on manually by module parameter.)

Wonderful test report, thanks a _bunch_ for investigating this.


> To turn on the DMADIR support automatically, maybe we can check whether
> the ATAPI device is bridged (by ata_dev_knobble()). If bridged, we can
> try to issue a ATAPI DMA read command to check if ATAPI DMA works.
> If not work, then try turning on DMADIR or turn off ATAPI DMA.

Sounds like more complexity than its worth, for one non-spec-compliant 
bridge chip.  The module option gets the 3611 going, so IMO the current 
level of support is adequate.

BTW, I found a 3611 here, and put it into my primary fileserver, 
attaching to a DMA-capable PATA CD-RW drive.

	Jeff