Tejun Heo wrote:
> Add qc->result_tf and ATA_QCFLAG_RESULT_TF. This moves the
> responsibility of loading result TF from post-compltion path to qc
> execution path. qc->result_tf is loaded if explicitly requested or
> the qc failsa. This allows more efficient completion implementation
> and correct handling of result TF for controllers which don't have
> global TF representation such as sil3124/32.
>
> Signed-off-by: Tejun Heo <htejun@xxxxxxxxx>
>
> ---
>
> drivers/scsi/libata-core.c | 4 ++--
> drivers/scsi/libata-scsi.c | 18 +++++++-----------
> include/linux/libata.h | 16 ++++++++++++++--
> 3 files changed, 23 insertions(+), 15 deletions(-)
>
> 46a93ec2cb1124f507e0cad7dcb65794a15af844
> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> index 464fd46..4cb7828 100644
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> @@ -955,7 +955,6 @@ void ata_qc_complete_internal(struct ata
> {
> struct completion *waiting = qc->private_data;
>
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> complete(waiting);
> }
>
> @@ -997,6 +996,7 @@ unsigned ata_exec_internal(struct ata_po
> qc->tf = *tf;
> if (cdb)
> memcpy(qc->cdb, cdb, ATAPI_CDB_LEN);
> + qc->flags |= ATA_QCFLAG_RESULT_TF;
> qc->dma_dir = dma_dir;
> if (dma_dir != DMA_NONE) {
> ata_sg_init_one(qc, buf, buflen);
> @@ -1034,7 +1034,7 @@ unsigned ata_exec_internal(struct ata_po
> /* finish up */
> spin_lock_irqsave(&ap->host_set->lock, flags);
>
> - *tf = qc->tf;
> + *tf = qc->result_tf;
> err_mask = qc->err_mask;
>
> ata_qc_free(qc);
> diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
> index b0c83c2..ce90b63 100644
> --- a/drivers/scsi/libata-scsi.c
> +++ b/drivers/scsi/libata-scsi.c
> @@ -537,7 +537,7 @@ void ata_to_sense_error(unsigned id, u8
> void ata_gen_ata_desc_sense(struct ata_queued_cmd *qc)
> {
> struct scsi_cmnd *cmd = qc->scsicmd;
> - struct ata_taskfile *tf = &qc->tf;
> + struct ata_taskfile *tf = &qc->result_tf;
> unsigned char *sb = cmd->sense_buffer;
> unsigned char *desc = sb + 8;
>
> @@ -608,7 +608,7 @@ void ata_gen_ata_desc_sense(struct ata_q
> void ata_gen_fixed_sense(struct ata_queued_cmd *qc)
> {
> struct scsi_cmnd *cmd = qc->scsicmd;
> - struct ata_taskfile *tf = &qc->tf;
> + struct ata_taskfile *tf = &qc->result_tf;
> unsigned char *sb = cmd->sense_buffer;
>
> memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
> @@ -1199,14 +1199,11 @@ static void ata_scsi_qc_complete(struct
> */
> if (((cdb[0] == ATA_16) || (cdb[0] == ATA_12)) &&
> ((cdb[2] & 0x20) || need_sense)) {
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> ata_gen_ata_desc_sense(qc);
> } else {
> if (!need_sense) {
> cmd->result = SAM_STAT_GOOD;
> } else {
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> -
> /* TODO: decide which descriptor format to use
> * for 48b LBA devices and call that here
> * instead of the fixed desc, which is only
> @@ -1217,10 +1214,8 @@ static void ata_scsi_qc_complete(struct
> }
> }
>
> - if (need_sense) {
> - /* The ata_gen_..._sense routines fill in tf */
> - ata_dump_status(qc->ap->id, &qc->tf);
> - }
> + if (need_sense)
> + ata_dump_status(qc->ap->id, &qc->result_tf);
>
> qc->scsidone(cmd);
>
> @@ -2004,7 +1999,6 @@ static void atapi_sense_complete(struct
> * a sense descriptors, since that's only
> * correct for ATA, not ATAPI
> */
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> ata_gen_ata_desc_sense(qc);
> }
>
> @@ -2080,7 +2074,6 @@ static void atapi_qc_complete(struct ata
> * a sense descriptors, since that's only
> * correct for ATA, not ATAPI
> */
> - qc->ap->ops->tf_read(qc->ap, &qc->tf);
> ata_gen_ata_desc_sense(qc);
> } else {
> u8 *scsicmd = cmd->cmnd;
> @@ -2361,6 +2354,9 @@ ata_scsi_pass_thru(struct ata_queued_cmd
> */
> qc->nsect = cmd->bufflen / ATA_SECT_SIZE;
>
> + /* request result TF */
> + qc->flags |= ATA_QCFLAG_RESULT_TF;
> +
> return 0;
>
> invalid_fld:
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index a117ca2..3d15c00 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -162,7 +162,9 @@ enum {
> ATA_QCFLAG_SINGLE = (1 << 2), /* no s/g, just a single buffer */
> ATA_QCFLAG_DMAMAP = ATA_QCFLAG_SG | ATA_QCFLAG_SINGLE,
> ATA_QCFLAG_IO = (1 << 3), /* standard IO command */
> - ATA_QCFLAG_EH_SCHEDULED = (1 << 4), /* EH scheduled */
> + ATA_QCFLAG_RESULT_TF = (1 << 4), /* result TF requested */
> +
> + ATA_QCFLAG_EH_SCHEDULED = (1 << 16), /* EH scheduled */
>
> /* host set flags */
> ATA_HOST_SIMPLEX = (1 << 0), /* Host is simplex, one DMA channel per host_set only */
> @@ -343,7 +345,7 @@ struct ata_queued_cmd {
> struct scatterlist *__sg;
>
> unsigned int err_mask;
> -
> + struct ata_taskfile result_tf;
> ata_qc_cb_t complete_fn;
>
> void *private_data;
> @@ -824,6 +826,10 @@ static inline void ata_qc_reinit(struct
> qc->err_mask = 0;
>
> ata_tf_init(qc->ap, &qc->tf, qc->dev->devno);
> +
> + /* init result_tf such that it indicates normal completion */
> + qc->result_tf.command = ATA_DRDY;
> + qc->result_tf.feature = 0;
> }
>
> /**
> @@ -839,9 +845,15 @@ static inline void ata_qc_reinit(struct
> */
> static inline void ata_qc_complete(struct ata_queued_cmd *qc)
> {
> + struct ata_port *ap = qc->ap;
> +
> if (unlikely(qc->flags & ATA_QCFLAG_EH_SCHEDULED))
> return;
>
> + /* read result TF if failed or requested */
> + if (qc->err_mask || qc->flags & ATA_QCFLAG_RESULT_TF)
> + ap->ops->tf_read(ap, &qc->result_tf);
> +
> __ata_qc_complete(qc);
> }
>
This patch makes qc->tf well preserved, except we still have
qc->tf got overwritten by ata_pio_bytes().
Maybe we also need the attached follow-up patch?
--
albert
--- 02_atapi_pio_bytes/drivers/scsi/libata-core.c 2006-05-18 14:20:32.000000000 +0800
+++ 03_atapi_pio_bytes/drivers/scsi/libata-core.c 2006-05-18 14:25:41.000000000 +0800
@@ -3861,10 +3861,10 @@ static void atapi_pio_bytes(struct ata_q
unsigned int ireason, bc_lo, bc_hi, bytes;
int i_write, do_write = (qc->tf.flags & ATA_TFLAG_WRITE) ? 1 : 0;
- ap->ops->tf_read(ap, &qc->tf);
- ireason = qc->tf.nsect;
- bc_lo = qc->tf.lbam;
- bc_hi = qc->tf.lbah;
+ ap->ops->tf_read(ap, &qc->result_tf);
+ ireason = qc->result_tf.nsect;
+ bc_lo = qc->result_tf.lbam;
+ bc_hi = qc->result_tf.lbah;
bytes = (bc_hi << 8) | bc_lo;
/* shall be cleared to zero, indicating xfer of data */
-
: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
