Skip to contentSkip to conversation

libata: honor the transfer cycle time speficied by the EIDE device

PATCH/RFC Applied Albert Lee 2 participants 2 messages Nov 16, 2005

Subsystems (2)
lore
Share & tools
lore

Albert Lee

author
#0 lore raw

Jeff,

The following code segment is not functional because the transfer cycle time speficied by the EIDE device is later overwritten by ata_timing_quantize():

	/*
	 * If the drive is an EIDE drive, it can tell us it needs extended
	 * PIO/MW_DMA cycle timing.
	 */
	if (adev->id[ATA_ID_FIELD_VALID] & 2) {	/* EIDE drive */
		memset(&p, 0, sizeof(p));
		(snip)
		ata_timing_merge(&p, t, t, ATA_TIMING_CYCLE | ATA_TIMING_CYC8B);
               <== uninitialized "t" is used here
	}

	/*
	 * Convert the timing to bus clock counts.
	 */
	ata_timing_quantize(s, t, T, UT);  <== t is overwritten by quantized s

The patch has been submitted for ide-timing.h before:
http://marc.theaimsgroup.com/?l=linux-ide&m=110820013425454&w=2
Resubmitted for libata.

Changes:
- Minor fix to honor the following transfer cycle time speficied by the device
- id[65]: Minimum Multiword DMA transfer cycle time per word
- id[67]: Minimum PIO transfer cycle time without flow control
- id[68]: Minimum PIO transfer cycle time with IORDY

Patch against the mainline tree
(f6ff56cd56b83d8edf4b3cffc5c53c56b37a5081)
For your review, thanks.

Albert

=======

drivers/scsi/libata-core.c +4 -2
@@ -1570,11 +1570,13 @@ int ata_timing_compute(struct ata_device
 
 	/*
 	 * Find the mode. 
-	*/
+	 */
 
 	if (!(s = ata_timing_find_mode(speed)))
 		return -EINVAL;
 
+	memcpy(t, s, sizeof(*s));
+
 	/*
 	 * If the drive is an EIDE drive, it can tell us it needs extended
 	 * PIO/MW_DMA cycle timing.
@@ -1595,7 +1597,7 @@ int ata_timing_compute(struct ata_device
 	 * Convert the timing to bus clock counts.
 	 */
 
-	ata_timing_quantize(s, t, T, UT);
+	ata_timing_quantize(t, t, T, UT);
 
 	/*
 	 * Even in DMA/UDMA modes we still use PIO access for IDENTIFY, S.M.A.R.T
Applied
#1 lore raw
Quoted 58 lines · Albert Lee

Jeff,

The following code segment is not functional because the transfer cycle time speficied by the EIDE device is later overwritten by ata_timing_quantize():

	/*
	 * If the drive is an EIDE drive, it can tell us it needs extended
	 * PIO/MW_DMA cycle timing.
	 */
	if (adev->id[ATA_ID_FIELD_VALID] & 2) {	/* EIDE drive */
		memset(&p, 0, sizeof(p));
		(snip)
		ata_timing_merge(&p, t, t, ATA_TIMING_CYCLE | ATA_TIMING_CYC8B);
               <== uninitialized "t" is used here
	}

	/*
	 * Convert the timing to bus clock counts.
	 */
	ata_timing_quantize(s, t, T, UT);  <== t is overwritten by quantized s

The patch has been submitted for ide-timing.h before:
http://marc.theaimsgroup.com/?l=linux-ide&m=110820013425454&w=2
Resubmitted for libata.

Changes:
- Minor fix to honor the following transfer cycle time speficied by the device
- id[65]: Minimum Multiword DMA transfer cycle time per word
- id[67]: Minimum PIO transfer cycle time without flow control
- id[68]: Minimum PIO transfer cycle time with IORDY

Patch against the mainline tree
(f6ff56cd56b83d8edf4b3cffc5c53c56b37a5081)
For your review, thanks.

Albert

=======

drivers/scsi/libata-core.c +3 -1
@@ -1570,11 +1570,13 @@ int ata_timing_compute(struct ata_device
 
 	/*
 	 * Find the mode. 
-	*/
+	 */
 
 	if (!(s = ata_timing_find_mode(speed)))
 		return -EINVAL;
 
+	memcpy(t, s, sizeof(*s));
+
 	/*
 	 * If the drive is an EIDE drive, it can tell us it needs extended
 	 * PIO/MW_DMA cycle timing.

drivers/scsi/libata-core.c int ata_timing_compute(struct ata_device · line 1600

Quoted 5 lines
drivers/scsi/libata-core.c +1 -1
@@ -1595,7 +1597,7 @@ int ata_timing_compute(struct ata_device
 	 * Convert the timing to bus clock counts.
 	 */
 
-	ata_timing_quantize(s, t, T, UT);
+	ata_timing_quantize(t, t, T, UT);

Applied to upstream-fixes branch. Let's see if Alan screams... :)

Jeff