Date: Tue, 4 Jan 2000 02:52:07 -0500 (EST) From: Brian Fundakowski Feldman <green@FreeBSD.org> To: Soren Schmidt <sos@freebsd.dk> Cc: current@FreeBSD.org Subject: Re: ATA atapi-all.c problems/fixes/cleanups Message-ID: <Pine.BSF.4.10.10001040243220.951-100000@green.dyndns.org> In-Reply-To: <200001040722.IAA72101@freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 4 Jan 2000, Soren Schmidt wrote:
> Either the patch is very short, or you forgot to include it :)
*LOL* I can't believe I did that :) It'll be on this one!!
>
> The ATA_16BIT_ONLY thing is to only do 16bit wide inb/outb instructions
> as old ISA HW don't allow 32bit wide access. This option is now deprecated
> as it is switched on automagically for ISA cards.
Is all data a multiple of 4 bytes, though?
>
> You _should_ _always_ have dd (or whatever you use) pad the output to the
> given blocksize, or you will run into problems.
IMHO, it should be better to use a larger multiple of block size (2352)
to not perform so many operations in userland (since the kernel does a
great job of doing writes in the right size in order). Then, it would
be pessimal to use "conv=osync" because you'd lose more room from the
media. But anyway, the padding should work properly, no matter what :)
Thanks for the prompt reply! Now I'll remember that patch...
> -Søren
--
Brian Fundakowski Feldman \ FreeBSD: The Power to Serve! /
green@FreeBSD.org `------------------------------'
cvs diff: Diffing .
Index: atapi-all.c
===================================================================
RCS file: /usr2/ncvs/src/sys/dev/ata/atapi-all.c,v
retrieving revision 1.28
diff -u -r1.28 atapi-all.c
--- atapi-all.c 1999/12/21 20:18:55 1.28
+++ atapi-all.c 2000/01/04 06:26:31
@@ -71,6 +71,10 @@
/* defines */
#define ATAPI_MAX_RETRIES 5
+/* This is temporary until I can read up more on the specs WRT packet length. */
+#undef ATA_16BIT_ONLY
+#define ATA_16BIT_ONLY
+
static __inline int
apiomode(struct atapi_params *ap)
{
@@ -546,58 +550,70 @@
atapi_read(struct atapi_request *request, int32_t length)
{
int8_t **buffer = (int8_t **)&request->data;
+ int32_t minlen = min(request->bytecount, length);
int32_t resid;
if (request->ccb[0] == ATAPI_REQUEST_SENSE)
*buffer = (int8_t *)&request->sense;
- if (request->bytecount < length) {
- printf("%s: read data overrun %d/%d\n",
- request->device->devname, length, request->bytecount);
+ if (minlen < length) {
+ printf("%s: read data overrun (%d < %d); some data discarded.\n",
+ request->device->devname, minlen, length);
#ifdef ATA_16BIT_ONLY
insw(request->device->controller->ioaddr + ATA_DATA,
- (void *)((uintptr_t)*buffer), length / sizeof(int16_t));
+ (void *)((uintptr_t)*buffer), minlen / sizeof(int16_t));
#else
insl(request->device->controller->ioaddr + ATA_DATA,
- (void *)((uintptr_t)*buffer), length / sizeof(int32_t));
+ (void *)((uintptr_t)*buffer), minlen / sizeof(int32_t));
#endif
- for (resid=request->bytecount; resid<length; resid+=sizeof(int16_t))
- inw(request->device->controller->ioaddr + ATA_DATA);
- }
+ for (resid = minlen; resid < length; resid += sizeof(int16_t))
+ (void)inw(request->device->controller->ioaddr + ATA_DATA);
+ }
else
+#ifdef ATA_16BIT_ONLY
insw(request->device->controller->ioaddr + ATA_DATA,
- (void *)((uintptr_t)*buffer), length / sizeof(int16_t));
- request->bytecount -= length;
- *buffer += length;
+ (void *)((uintptr_t)*buffer), minlen / sizeof(int16_t));
+#else
+ insl(request->device->controller->ioaddr + ATA_DATA,
+ (void *)((uintptr_t)*buffer), minlen / sizeof(int32_t));
+#endif
+ request->bytecount -= minlen;
+ *buffer += minlen;
}
static void
atapi_write(struct atapi_request *request, int32_t length)
{
int8_t **buffer = (int8_t **)&request->data;
+ int32_t minlen = min(request->bytecount, length);
int32_t resid;
if (request->ccb[0] == ATAPI_REQUEST_SENSE)
*buffer = (int8_t *)&request->sense;
- if (request->bytecount < length) {
- printf("%s: write data underrun %d/%d\n",
- request->device->devname, length, request->bytecount);
+ if (minlen < length) {
+ printf("%s: write data underrun (%d < %d); padding to full length.\n",
+ request->device->devname, minlen, length);
#ifdef ATA_16BIT_ONLY
outsw(request->device->controller->ioaddr + ATA_DATA,
- (void *)((uintptr_t)*buffer), length / sizeof(int16_t));
+ (void *)((uintptr_t)*buffer), minlen / sizeof(int16_t));
#else
outsl(request->device->controller->ioaddr + ATA_DATA,
- (void *)((uintptr_t)*buffer), length / sizeof(int32_t));
+ (void *)((uintptr_t)*buffer), minlen / sizeof(int32_t));
#endif
- for (resid=request->bytecount; resid<length; resid+=sizeof(int16_t))
+ for (resid = minlen; resid < length; resid += sizeof(int16_t))
outw(request->device->controller->ioaddr + ATA_DATA, 0);
}
else
+#ifdef ATA_16BIT_ONLY
outsw(request->device->controller->ioaddr + ATA_DATA,
- (void *)((uintptr_t)*buffer), length / sizeof(int16_t));
- request->bytecount -= length;
- *buffer += length;
+ (void *)((uintptr_t)*buffer), minlen / sizeof(int16_t));
+#else
+ outsl(request->device->controller->ioaddr + ATA_DATA,
+ (void *)((uintptr_t)*buffer), minlen / sizeof(int32_t));
+#endif
+ request->bytecount -= minlen;
+ *buffer += minlen;
}
static void
@@ -619,7 +635,7 @@
/* retries all used up, return error */
request->result = ATAPI_SK_RESERVED | ATAPI_E_ABRT;
wakeup((caddr_t)request);
- }
+ }
ata_reinit(atp->controller);
}
@@ -737,7 +753,7 @@
(atp->controller->status & ATA_S_READY))
break;
DELAY (10);
- }
+ }
if (timeout <= 0)
return -1;
if (!mask)
@@ -749,9 +765,9 @@
if ((atp->controller->status & mask) == mask)
return (atp->controller->status & ATA_S_ERROR);
DELAY (10);
- }
+ }
return -1;
-}
+}
static void
atapi_init(void)
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.10.10001040243220.951-100000>