* [PATCH 1/4] libata: reorder HSM_ST_FIRST for easier decoding
2007-03-31 7:07 [PATCH 0/4] libata: Workaround/fixes for ATAPI devices Albert Lee
@ 2007-03-31 7:13 ` Albert Lee
2007-03-31 7:27 ` Tejun Heo
2007-03-31 7:17 ` [PATCH 2/4] libata: Clear tf before doing request sense Albert Lee
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Albert Lee @ 2007-03-31 7:13 UTC (permalink / raw)
To: Jeff Garzik
Cc: Alan Cox, Tejun Heo, Bartlomiej Zolnierkiewicz, Sergei Shtylyov,
YUP, Vlad Codrea, IDE Linux
patch 1/4:
Reorder HSM_ST_FIRST, such that the task state transition is easier decoded with human eyes.
Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 00_libata-dev.ori/include/linux/libata.h 01_hsm_st/include/linux/libata.h
--- 00_libata-dev.ori/include/linux/libata.h 2007-03-23 16:56:24.000000000 +0800
+++ 01_hsm_st/include/linux/libata.h 2007-03-26 12:33:28.000000000 +0800
@@ -315,11 +315,11 @@ enum {
enum hsm_task_states {
HSM_ST_IDLE, /* no command on going */
+ HSM_ST_FIRST, /* (waiting the device to)
+ write CDB or first data block */
HSM_ST, /* (waiting the device to) transfer data */
HSM_ST_LAST, /* (waiting the device to) complete command */
HSM_ST_ERR, /* error */
- HSM_ST_FIRST, /* (waiting the device to)
- write CDB or first data block */
};
enum ata_completion_errors {
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 2/4] libata: Clear tf before doing request sense
2007-03-31 7:07 [PATCH 0/4] libata: Workaround/fixes for ATAPI devices Albert Lee
2007-03-31 7:13 ` [PATCH 1/4] libata: reorder HSM_ST_FIRST for easier decoding Albert Lee
@ 2007-03-31 7:17 ` Albert Lee
2007-03-31 7:27 ` Tejun Heo
2007-03-31 7:21 ` [PATCH 3/4] libata: Limit max sector to 240 for TORiSAN DVD drives Albert Lee
2007-03-31 7:27 ` [PATCH 4/4] libata: Limit ATAPI DMA to R/W commands only " Albert Lee
3 siblings, 1 reply; 11+ messages in thread
From: Albert Lee @ 2007-03-31 7:17 UTC (permalink / raw)
To: Jeff Garzik
Cc: Alan Cox, Tejun Heo, Bartlomiej Zolnierkiewicz, Sergei Shtylyov,
YUP, Vlad Codrea, IDE Linux
patch 2/4:
Clear tf before doing request sense.
This fixes the AOpen 56X/AKH timeout problem.
(http://bugzilla.kernel.org/show_bug.cgi?id=8244)
Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 01_hsm_st/drivers/ata/libata-eh.c 02_aopen_rs/drivers/ata/libata-eh.c
--- 01_hsm_st/drivers/ata/libata-eh.c 2007-03-23 16:56:13.000000000 +0800
+++ 02_aopen_rs/drivers/ata/libata-eh.c 2007-03-31 01:11:01.000000000 +0800
@@ -991,18 +991,19 @@ static unsigned int atapi_eh_request_sen
DPRINTK("ATAPI request sense\n");
- ata_tf_init(dev, &tf);
-
/* FIXME: is this needed? */
memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);
- /* XXX: why tf_read here? */
+ /* read error register to initialize sense_buf */
ap->ops->tf_read(ap, &tf);
/* fill these in, for the case where they are -not- overwritten */
sense_buf[0] = 0x70;
sense_buf[2] = tf.feature >> 4;
+ /* some devices time out if garbage left in tf */
+ ata_tf_init(dev, &tf);
+
memset(cdb, 0, ATAPI_CDB_LEN);
cdb[0] = REQUEST_SENSE;
cdb[4] = SCSI_SENSE_BUFFERSIZE;
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] libata: Clear tf before doing request sense
2007-03-31 7:17 ` [PATCH 2/4] libata: Clear tf before doing request sense Albert Lee
@ 2007-03-31 7:27 ` Tejun Heo
2007-03-31 7:44 ` Albert Lee
0 siblings, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2007-03-31 7:27 UTC (permalink / raw)
To: albertl
Cc: Jeff Garzik, Alan Cox, Bartlomiej Zolnierkiewicz, Sergei Shtylyov,
YUP, Vlad Codrea, IDE Linux
Albert Lee wrote:
> diff -Nrup 01_hsm_st/drivers/ata/libata-eh.c 02_aopen_rs/drivers/ata/libata-eh.c
> --- 01_hsm_st/drivers/ata/libata-eh.c 2007-03-23 16:56:13.000000000 +0800
> +++ 02_aopen_rs/drivers/ata/libata-eh.c 2007-03-31 01:11:01.000000000 +0800
> @@ -991,18 +991,19 @@ static unsigned int atapi_eh_request_sen
>
> DPRINTK("ATAPI request sense\n");
>
> - ata_tf_init(dev, &tf);
> -
> /* FIXME: is this needed? */
> memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);
>
> - /* XXX: why tf_read here? */
> + /* read error register to initialize sense_buf */
> ap->ops->tf_read(ap, &tf);
>
> /* fill these in, for the case where they are -not- overwritten */
> sense_buf[0] = 0x70;
> sense_buf[2] = tf.feature >> 4;
Oh, now I see why it's there. Thanks for spotting this. We don't need
tf_read here, you can simply use the value in qc->result_tf.feature for
this purpose.
--
tejun
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] libata: Clear tf before doing request sense
2007-03-31 7:27 ` Tejun Heo
@ 2007-03-31 7:44 ` Albert Lee
0 siblings, 0 replies; 11+ messages in thread
From: Albert Lee @ 2007-03-31 7:44 UTC (permalink / raw)
To: Tejun Heo
Cc: albertl, Jeff Garzik, Alan Cox, Bartlomiej Zolnierkiewicz,
Sergei Shtylyov, YUP, Vlad Codrea, IDE Linux
Tejun Heo wrote:
> Albert Lee wrote:
>
>> diff -Nrup 01_hsm_st/drivers/ata/libata-eh.c
>> 02_aopen_rs/drivers/ata/libata-eh.c
>> --- 01_hsm_st/drivers/ata/libata-eh.c 2007-03-23 16:56:13.000000000
>> +0800
>> +++ 02_aopen_rs/drivers/ata/libata-eh.c 2007-03-31
>> 01:11:01.000000000 +0800
>> @@ -991,18 +991,19 @@ static unsigned int atapi_eh_request_sen
>>
>> DPRINTK("ATAPI request sense\n");
>>
>> - ata_tf_init(dev, &tf);
>> -
>> /* FIXME: is this needed? */
>> memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);
>>
>> - /* XXX: why tf_read here? */
>> + /* read error register to initialize sense_buf */
>> ap->ops->tf_read(ap, &tf);
>>
>> /* fill these in, for the case where they are -not- overwritten */
>> sense_buf[0] = 0x70;
>> sense_buf[2] = tf.feature >> 4;
>
>
> Oh, now I see why it's there. Thanks for spotting this. We don't need
> tf_read here, you can simply use the value in qc->result_tf.feature for
> this purpose.
>
Thanks for the advice. Will revise this patch.
--
albert
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] libata: Limit max sector to 240 for TORiSAN DVD drives
2007-03-31 7:07 [PATCH 0/4] libata: Workaround/fixes for ATAPI devices Albert Lee
2007-03-31 7:13 ` [PATCH 1/4] libata: reorder HSM_ST_FIRST for easier decoding Albert Lee
2007-03-31 7:17 ` [PATCH 2/4] libata: Clear tf before doing request sense Albert Lee
@ 2007-03-31 7:21 ` Albert Lee
2007-03-31 7:28 ` Tejun Heo
2007-03-31 7:27 ` [PATCH 4/4] libata: Limit ATAPI DMA to R/W commands only " Albert Lee
3 siblings, 1 reply; 11+ messages in thread
From: Albert Lee @ 2007-03-31 7:21 UTC (permalink / raw)
To: Jeff Garzik
Cc: Alan Cox, Tejun Heo, Bartlomiej Zolnierkiewicz, Sergei Shtylyov,
YUP, Vlad Codrea, IDE Linux
patch 3/4:
The TORiSAN drive locks up when max sector == 256.
Limit max sector to 240 for TORiSAN DRD-N216 drives.
(http://bugzilla.kernel.org/show_bug.cgi?id=6710)
Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 02_aopen_rs/drivers/ata/libata-core.c 03_max_sect/drivers/ata/libata-core.c
--- 02_aopen_rs/drivers/ata/libata-core.c 2007-03-23 16:56:13.000000000 +0800
+++ 03_max_sect/drivers/ata/libata-core.c 2007-03-31 14:11:33.000000000 +0800
@@ -1784,6 +1784,9 @@ int ata_dev_configure(struct ata_device
dev->max_sectors = ATA_MAX_SECTORS;
}
+ if (ata_device_blacklisted(dev) & ATA_HORKAGE_MAX_SEC_240)
+ dev->max_sectors = min(ATA_MAX_SECTORS_240, dev->max_sectors);
+
if (ap->ops->dev_config)
ap->ops->dev_config(ap, dev);
@@ -3352,6 +3355,9 @@ static const struct ata_blacklist_entry
{ "_NEC DV5800A", NULL, ATA_HORKAGE_NODMA },
{ "SAMSUNG CD-ROM SN-124","N001", ATA_HORKAGE_NODMA },
+ /* Weird ATAPI devices */
+ { "TORiSAN DVD-ROM DRD-N216", NULL, ATA_HORKAGE_MAX_SEC_240 },
+
/* Devices we expect to fail diagnostics */
/* Devices where NCQ should be avoided */
diff -Nrup 02_aopen_rs/include/linux/ata.h 03_max_sect/include/linux/ata.h
--- 02_aopen_rs/include/linux/ata.h 2007-03-23 16:56:24.000000000 +0800
+++ 03_max_sect/include/linux/ata.h 2007-03-31 10:06:53.000000000 +0800
@@ -40,6 +40,7 @@ enum {
ATA_MAX_DEVICES = 2, /* per bus/port */
ATA_MAX_PRD = 256, /* we could make these 256/256 */
ATA_SECT_SIZE = 512,
+ ATA_MAX_SECTORS_240 = 240,
ATA_MAX_SECTORS = 256,
ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */
diff -Nrup 02_aopen_rs/include/linux/libata.h 03_max_sect/include/linux/libata.h
--- 02_aopen_rs/include/linux/libata.h 2007-03-26 12:33:28.000000000 +0800
+++ 03_max_sect/include/linux/libata.h 2007-03-31 14:07:49.000000000 +0800
@@ -311,6 +311,7 @@ enum {
ATA_HORKAGE_DIAGNOSTIC = (1 << 0), /* Failed boot diag */
ATA_HORKAGE_NODMA = (1 << 1), /* DMA problems */
ATA_HORKAGE_NONCQ = (1 << 2), /* Don't use NCQ */
+ ATA_HORKAGE_MAX_SEC_240 = (1 << 3), /* Limit max sects below 256 */
};
enum hsm_task_states {
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 4/4] libata: Limit ATAPI DMA to R/W commands only for TORiSAN DVD drives
2007-03-31 7:07 [PATCH 0/4] libata: Workaround/fixes for ATAPI devices Albert Lee
` (2 preceding siblings ...)
2007-03-31 7:21 ` [PATCH 3/4] libata: Limit max sector to 240 for TORiSAN DVD drives Albert Lee
@ 2007-03-31 7:27 ` Albert Lee
2007-03-31 7:30 ` Tejun Heo
3 siblings, 1 reply; 11+ messages in thread
From: Albert Lee @ 2007-03-31 7:27 UTC (permalink / raw)
To: Jeff Garzik
Cc: Alan Cox, Tejun Heo, Bartlomiej Zolnierkiewicz, Sergei Shtylyov,
YUP, Vlad Codrea, IDE Linux
patch 4/4:
Limit ATAPI DMA to R/W commands only for TORiSAN DRD-N216 DVD-ROM drives
(http://bugzilla.kernel.org/show_bug.cgi?id=6710)
Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
I guess maybe other commands like READ_DVD_STRUCTURE and READ_CD also
work under ATAPI DMA, but not confirmed by Vlad yet. So, allow ATAPI DMA to
only for Read/Write here for safty.
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 03_max_sect/drivers/ata/libata-core.c 04_dma_rw_only/drivers/ata/libata-core.c
--- 03_max_sect/drivers/ata/libata-core.c 2007-03-31 14:11:33.000000000 +0800
+++ 04_dma_rw_only/drivers/ata/libata-core.c 2007-03-31 14:27:47.000000000 +0800
@@ -1787,6 +1787,10 @@ int ata_dev_configure(struct ata_device
if (ata_device_blacklisted(dev) & ATA_HORKAGE_MAX_SEC_240)
dev->max_sectors = min(ATA_MAX_SECTORS_240, dev->max_sectors);
+ /* limit ATAPI DMA to R/W commands only */
+ if (ata_device_blacklisted(dev) & ATA_HORKAGE_DMA_RW_ONLY)
+ dev->horkage |= ATA_HORKAGE_DMA_RW_ONLY;
+
if (ap->ops->dev_config)
ap->ops->dev_config(ap, dev);
@@ -3356,7 +3360,8 @@ static const struct ata_blacklist_entry
{ "SAMSUNG CD-ROM SN-124","N001", ATA_HORKAGE_NODMA },
/* Weird ATAPI devices */
- { "TORiSAN DVD-ROM DRD-N216", NULL, ATA_HORKAGE_MAX_SEC_240 },
+ { "TORiSAN DVD-ROM DRD-N216", NULL, ATA_HORKAGE_MAX_SEC_240 |
+ ATA_HORKAGE_DMA_RW_ONLY },
/* Devices we expect to fail diagnostics */
@@ -3676,6 +3681,26 @@ int ata_check_atapi_dma(struct ata_queue
struct ata_port *ap = qc->ap;
int rc = 0; /* Assume ATAPI DMA is OK by default */
+ /* some drives can only do ATAPI DMA on read/write */
+ if (unlikely(qc->dev->horkage & ATA_HORKAGE_DMA_RW_ONLY)) {
+ struct scsi_cmnd *cmd = qc->scsicmd;
+ u8 *scsicmd = cmd->cmnd;
+
+ switch (scsicmd[0]) {
+ case READ_10:
+ case WRITE_10:
+ case READ_12:
+ case WRITE_12:
+ case READ_6:
+ case WRITE_6:
+ /* atapi dma maybe ok */
+ break;
+ default:
+ /* turn off atapi dma */
+ return 1;
+ }
+ }
+
if (ap->ops->check_atapi_dma)
rc = ap->ops->check_atapi_dma(qc);
diff -Nrup 03_max_sect/include/linux/libata.h 04_dma_rw_only/include/linux/libata.h
--- 03_max_sect/include/linux/libata.h 2007-03-31 14:07:49.000000000 +0800
+++ 04_dma_rw_only/include/linux/libata.h 2007-03-31 14:08:31.000000000 +0800
@@ -312,6 +312,7 @@ enum {
ATA_HORKAGE_NODMA = (1 << 1), /* DMA problems */
ATA_HORKAGE_NONCQ = (1 << 2), /* Don't use NCQ */
ATA_HORKAGE_MAX_SEC_240 = (1 << 3), /* Limit max sects below 256 */
+ ATA_HORKAGE_DMA_RW_ONLY = (1 << 4), /* ATAPI DMA for RW only */
};
enum hsm_task_states {
^ permalink raw reply [flat|nested] 11+ messages in thread