Re: [PATCH 2/4] if condition fix for __atapi_pio_bytes()

[Albert Lee <albertcc@tw.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]



Bartlomiej Zolnierkiewicz wrote:

> 
> Another idea - move rounding just before ata_data_xfer() so 'count' will stay 
> rounded only for the duration of ata_data_xfer() ('count' is reset for
> every sg)
> and we won't have to worry about sg->length and 'bytes'.
> 
> Or just leave it as it is and reorder patches... :).
> 
> 

Hi Bart,

How about moving the odd length rounding up code to ata_mmio_data_xfer() and ata_pio_data_xfer()?
It seems this can make the code of patch #4 simplified.
Also we can change the rounding in the trailing data handling code
from 'words = (bytes + 1)>> 1 ' to 'words = bytes >> 1'.

Ex. If bytes is 52, and buffer length is 51,
     ata_data_xfer() will be called with 51.
     ata_data_xfer() will actually transfer 52 bytes.

     1 byte left for the trailing data handling. Since
we know ata_data_xfer() round up one more byte, so in the trailing data handling code,
we can safely round down one less byte by 'words = bytes >> 1'.


Attached please find the revised patch #3 and #4 for your review.


Albert

[-- Attachment #2: 04_odd_data.diff --]
[-- Type: text/plain, Size: 741 bytes --]

--- 13_atapi_pio_trailing_data_handling/drivers/scsi/libata-core.c	2005-06-09 11:05:10.000000000 +0800
+++ 14_atapi_pio_odd_handling/drivers/scsi/libata-core.c	2005-06-09 11:04:09.000000000 +0800
@@ -2491,7 +2491,7 @@
 			       unsigned int buflen, int write_data)
 {
 	unsigned int i;
-	unsigned int words = buflen >> 1;
+	unsigned int words = (buflen+1) >> 1;
 	u16 *buf16 = (u16 *) buf;
 	void __iomem *mmio = (void __iomem *)ap->ioaddr.data_addr;
 
@@ -2507,7 +2507,7 @@
 static void ata_pio_data_xfer(struct ata_port *ap, unsigned char *buf,
 			      unsigned int buflen, int write_data)
 {
-	unsigned int dwords = buflen >> 1;
+	unsigned int dwords = (buflen+1) >> 1;
 
 	if (write_data)
 		outsw(ap->ioaddr.data_addr, buf, dwords);

[-- Attachment #3: 03_trailing_data.diff --]
[-- Type: text/plain, Size: 738 bytes --]

--- 12_atapi_pio_if_condition_fix/drivers/scsi/libata-core.c	2005-06-06 13:52:11.000000000 +0800
+++ 13_atapi_pio_trailing_data_handling/drivers/scsi/libata-core.c	2005-06-09 11:05:10.000000000 +0800
@@ -2575,6 +2575,23 @@
 		ap->pio_task_state = PIO_ST_LAST;
 
 next_sg:
+	/* check whether qc->sg is full */
+	if (unlikely(qc->cursg >= qc->n_elem)) {
+		unsigned char pad_buf[2];
+		unsigned int words = bytes >> 1;
+		unsigned int i;
+
+		DPRINTK("ata%u: padding %u bytes\n", ap->id, bytes);
+
+		memset(&pad_buf, 0, sizeof(pad_buf));
+		for (i = 0; i < words; i++) {
+			ata_data_xfer(ap, pad_buf, sizeof(pad_buf), do_write);
+		}
+
+		ap->pio_task_state = PIO_ST_LAST;
+		return;
+	}
+
 	sg = &qc->sg[qc->cursg];
 
 	page = sg->page;