2012-05-10 04:20:46

by Thang Q. Nguyen

[permalink] [raw]
Subject: [PATCH 3/5] sata_dwc: Add support ATAPI transfer


Signed-off-by: Thang Q. Nguyen <[email protected]>
---
drivers/ata/sata_dwc_460ex.c | 64 ++++++++++++++++++++++++++++++++++++++++--
1 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 937aeb3..6175e01 100755
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -1035,7 +1035,8 @@ DRVSTILLBUSY:

if ((host_pvt.dma_interrupt_count % 2) == 0)
sata_dwc_dma_xfer_complete(ap, 1);
- } else if (ata_is_pio(qc->tf.protocol)) {
+ } else if (ata_is_pio(qc->tf.protocol) ||
+ ata_is_nodata(qc->tf.protocol)) {
ata_sff_hsm_move(ap, qc, status, 0);
handled = 1;
goto DONE;
@@ -1235,7 +1236,10 @@ static int sata_dwc_qc_complete(struct ata_port *ap, struct ata_queued_cmd *qc,
& mask;
host_pvt.sata_dwc_sactive_issued = (host_pvt.sata_dwc_sactive_issued) \
& mask;
- ata_qc_complete(qc);
+ if (ata_is_atapi(qc->tf.protocol))
+ ata_sff_hsm_move(ap, qc, status, 0);
+ else
+ ata_qc_complete(qc);
return 0;
}

@@ -1385,6 +1389,58 @@ static void sata_dwc_port_stop(struct ata_port *ap)
ap->private_data = NULL;
}

+/**
+ * sata_dwc_check_atapi_dma - Filter ATAPI cmds which are unsuitable for DMA.
+ * @qc: queued command to check for chipset/DMA compatibility.
+ *
+ * The bmdma engines cannot handle speculative data sizes (bytecount
+ * under/over flow). So only allow DMA for data transfer commands with
+ * known data sizes.
+ */
+static int sata_dwc_check_atapi_dma(struct ata_queued_cmd *qc)
+{
+ struct scsi_cmnd *scmd = qc->scsicmd;
+ int pio = 1; /* atapi dma off by default */
+ unsigned int lba;
+
+ if (scmd) {
+ switch (scmd->cmnd[0]) {
+ case WRITE_6:
+ case WRITE_10:
+ case WRITE_12:
+ case READ_6:
+ case READ_10:
+ case READ_12:
+ pio = 0; /* DMA is safe */
+ break;
+ }
+
+ /* -45150 (FFFF4FA2) to -1 (FFFFFFFF) shall use PIO mode */
+ if (scmd->cmnd[0] == WRITE_10) {
+ lba = (scmd->cmnd[2] << 24) |
+ (scmd->cmnd[3] << 16) |
+ (scmd->cmnd[4] << 8) |
+ scmd->cmnd[5];
+ if (lba >= 0xFFFF4FA2)
+ pio = 1;
+ }
+ /*
+ * WORK AROUND: Fix DMA issue when blank CD/DVD disc in the drive
+ * and user use the 'fdisk -l' command. No DMA data returned so
+ * we can not complete the QC.
+ */
+ else if (scmd->cmnd[0] == READ_10) {
+ lba = (scmd->cmnd[2] << 24) |
+ (scmd->cmnd[3] << 16) |
+ (scmd->cmnd[4] << 8) |
+ scmd->cmnd[5];
+ if (lba < 0x20)
+ pio = 1;
+ }
+ }
+ return pio;
+}
+
/*
* Function : sata_dwc_exec_command_by_tag
* arguments : ata_port *ap, ata_taskfile *tf, u8 tag, u32 cmd_issued
@@ -1572,7 +1628,8 @@ static unsigned int sata_dwc_qc_issue(struct ata_queued_cmd *qc)

static void sata_dwc_qc_prep(struct ata_queued_cmd *qc)
{
- if ((qc->dma_dir == DMA_NONE) || (qc->tf.protocol == ATA_PROT_PIO))
+ if ((qc->dma_dir == DMA_NONE) || (qc->tf.protocol == ATA_PROT_PIO) ||
+ (qc->tf.protocol == ATAPI_PROT_PIO))
return;

#ifdef DEBUG_NCQ
@@ -1641,6 +1698,7 @@ static struct ata_port_operations sata_dwc_ops = {
.port_start = sata_dwc_port_start,
.port_stop = sata_dwc_port_stop,

+ .check_atapi_dma = sata_dwc_check_atapi_dma,
.bmdma_setup = sata_dwc_bmdma_setup,
.bmdma_start = sata_dwc_bmdma_start,
};
--
1.7.3.4


2012-05-10 09:50:37

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 3/5] sata_dwc: Add support ATAPI transfer

Hello.

On 10-05-2012 8:17, Thang Q. Nguyen wrote:

> Signed-off-by: Thang Q. Nguyen<[email protected]>
> ---
> drivers/ata/sata_dwc_460ex.c | 64 ++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 61 insertions(+), 3 deletions(-)

> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> index 937aeb3..6175e01 100755
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
[...]
> @@ -1385,6 +1389,58 @@ static void sata_dwc_port_stop(struct ata_port *ap)
> ap->private_data = NULL;
> }
>
> +/**
> + * sata_dwc_check_atapi_dma - Filter ATAPI cmds which are unsuitable for DMA.
> + * @qc: queued command to check for chipset/DMA compatibility.
> + *
> + * The bmdma engines cannot handle speculative data sizes (bytecount
> + * under/over flow). So only allow DMA for data transfer commands with
> + * known data sizes.
> + */
> +static int sata_dwc_check_atapi_dma(struct ata_queued_cmd *qc)
> +{
[...]
> +
> + /* -45150 (FFFF4FA2) to -1 (FFFFFFFF) shall use PIO mode */

Sector numbers are unsigned entities.

> + if (scmd->cmnd[0] == WRITE_10) {
> + lba = (scmd->cmnd[2] << 24) |
> + (scmd->cmnd[3] << 16) |
> + (scmd->cmnd[4] << 8) |
> + scmd->cmnd[5];
> + if (lba>= 0xFFFF4FA2)
> + pio = 1;
> + }
> + /*
> + * WORK AROUND: Fix DMA issue when blank CD/DVD disc in the drive
> + * and user use the 'fdisk -l' command. No DMA data returned so
> + * we can not complete the QC.

The user shouldn't use 'fdisk' command on CD/DVD disks. On the other
hand, he could use something like 'hdparm -t' od 'dd' with offset in which
case I'm not sure this heuristics will work.

> + */
> + else if (scmd->cmnd[0] == READ_10) {
> + lba = (scmd->cmnd[2] << 24) |
> + (scmd->cmnd[3] << 16) |
> + (scmd->cmnd[4] << 8) |
> + scmd->cmnd[5];
> + if (lba < 0x20)
> + pio = 1;
> + }
> + }
> + return pio;
> +}
> +
> /*
> * Function : sata_dwc_exec_command_by_tag
> * arguments : ata_port *ap, ata_taskfile *tf, u8 tag, u32 cmd_issued

WBR, Sergei

2012-05-10 10:53:23

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 3/5] sata_dwc: Add support ATAPI transfer

Hello.

On 10-05-2012 14:12, Thang Q. Nguyen wrote:

> Thanks for quick response,
> I do not mean user use fdisk command on CD/DVD. However, whenever using 'fdisk
> -l' command, the system will access each first sector on each SCSI device
> (including CD/DVD on it) and the DMA transfer on blank CD/DVD will not return
> any data.

Shouldn't the drive return error status in this case?

> The timeout is detected and error handler will be called.

But user may also run 'hdparm -t' or even 'dd' with offset on a drive.
Have you tried your workaround with those?

> Using PIO on some first sectors will avoid the issue.

> Regards,
> Thang Nguyen -

MBR, Sergei