Fix the DMA problem that most severely hit on the DS3112a SATA chip

in connection with Marvell based SATA->PATA dongles.

The problem was caused by a combination of things working
together to make it hard to spot...

The ATA driver has always started the ATA command, then build
the SG list for DMA and then finally started the DMA engine.
While this is according to specs, it poses a potential
problem as some controllers apparently do not allow for unlimitted
time between starting the ATA command and starting the DMA engine.

At about the same time as ATAng was committed there were lots
of other changes applied, some of which was locking in parts
that causes the busdma load functions to take significantly
longer to load the SG list.

This pushed the time spent between starting the ATA command and
starting the DMA engine over the hill for some controllers
(especially the Silicon Image DS3112a) and caused what looked
like lost interrupts.

The solution is to get all the SG list work or rather all
busdma related stuff done before we even try to start anything.

This has the nice side effect of seperating busdma out the
way it should be, so the working of the ATA machinery is not
cluttered up with busdma droppings, making the code easier
to read and understand.
This commit is contained in:
sos 2003-10-21 19:20:37 +00:00
parent 3c2f2b8b31
commit 08d97161dc
5 changed files with 59 additions and 66 deletions

View File

@ -247,14 +247,16 @@ struct ata_dma {
bus_addr_t mdmatab; /* bus address of dmatab */
u_int32_t alignment; /* DMA engine alignment */
u_int32_t max_iosize; /* DMA engine max IO size */
u_int32_t cur_iosize; /* DMA engine current IO size */
int flags;
#define ATA_DMA_ACTIVE 0x01 /* DMA transfer in progress */
#define ATA_DMA_READ 0x02 /* transaction is a read */
void (*alloc)(struct ata_channel *ch);
void (*free)(struct ata_channel *ch);
int (*setup)(struct ata_device *atadev, caddr_t data, int32_t count);
int (*start)(struct ata_channel *ch, caddr_t data, int32_t count, int dir);
int (*load)(struct ata_device *atadev, caddr_t data, int32_t count,int dir);
int (*unload)(struct ata_channel *ch);
int (*start)(struct ata_channel *ch);
int (*stop)(struct ata_channel *ch);
};

View File

@ -90,10 +90,10 @@ static void ata_promise_tx2_intr(void *);
static void ata_promise_mio_intr(void *);
static void ata_promise_setmode(struct ata_device *, int);
static void ata_promise_new_dmainit(struct ata_channel *);
static int ata_promise_new_dmastart(struct ata_channel *, caddr_t, int32_t,int);
static int ata_promise_new_dmastart(struct ata_channel *);
static int ata_promise_new_dmastop(struct ata_channel *);
static void ata_promise_mio_dmainit(struct ata_channel *);
static int ata_promise_mio_dmastart(struct ata_channel *, caddr_t, int32_t,int);
static int ata_promise_mio_dmastart(struct ata_channel *);
static int ata_promise_mio_dmastop(struct ata_channel *);
static int ata_serverworks_chipinit(device_t);
static void ata_serverworks_setmode(struct ata_device *, int);
@ -1360,27 +1360,25 @@ ata_promise_new_dmainit(struct ata_channel *ch)
}
static int
ata_promise_new_dmastart(struct ata_channel *ch,
caddr_t data, int32_t count, int dir)
ata_promise_new_dmastart(struct ata_channel *ch)
{
struct ata_pci_controller *ctlr =
device_get_softc(device_get_parent(ch->dev));
int error;
if ((error = ata_dmastart(ch, data, count, dir)))
return error;
if (ch->flags & ATA_48BIT_ACTIVE) {
ATA_OUTB(ctlr->r_io1, 0x11,
ATA_INB(ctlr->r_io1, 0x11) | (ch->unit ? 0x08 : 0x02));
ATA_OUTL(ctlr->r_io1, 0x20,
(dir ? 0x05000000 : 0x06000000) | (count >> 1));
((ch->dma->flags & ATA_DMA_READ) ? 0x05000000 : 0x06000000) |
(ch->dma->cur_iosize >> 1));
}
ATA_IDX_OUTB(ch, ATA_BMSTAT_PORT, (ATA_IDX_INB(ch, ATA_BMSTAT_PORT) |
(ATA_BMSTAT_INTERRUPT | ATA_BMSTAT_ERROR)));
ATA_IDX_OUTL(ch, ATA_BMDTP_PORT, ch->dma->mdmatab);
ATA_IDX_OUTB(ch, ATA_BMCMD_PORT,
(dir ? ATA_BMCMD_WRITE_READ : 0) | ATA_BMCMD_START_STOP);
return error;
((ch->dma->flags & ATA_DMA_READ) ? ATA_BMCMD_WRITE_READ : 0) |
ATA_BMCMD_START_STOP);
return 0;
}
static int
@ -1399,7 +1397,6 @@ ata_promise_new_dmastop(struct ata_channel *ch)
ATA_IDX_OUTB(ch, ATA_BMCMD_PORT,
ATA_IDX_INB(ch, ATA_BMCMD_PORT) & ~ATA_BMCMD_START_STOP);
ATA_IDX_OUTB(ch, ATA_BMSTAT_PORT, ATA_BMSTAT_INTERRUPT | ATA_BMSTAT_ERROR);
ata_dmastop(ch);
return error;
}
@ -1414,19 +1411,13 @@ ata_promise_mio_dmainit(struct ata_channel *ch)
}
static int
ata_promise_mio_dmastart(struct ata_channel *ch,
caddr_t data, int32_t count, int dir)
ata_promise_mio_dmastart(struct ata_channel *ch)
{
int error;
if ((error = ata_dmastart(ch, data, count, dir)))
return error;
ATA_IDX_OUTL(ch, ATA_BMDTP_PORT, ch->dma->mdmatab);
ATA_IDX_OUTL(ch, ATA_BMCTL_PORT,
(ATA_IDX_INL(ch, ATA_BMCTL_PORT) & ~0x000000c0) |
((dir) ? 0x00000080 : 0x000000c0));
return error;
((ch->dma->flags & ATA_DMA_READ) ? 0x00000080 : 0x000000c0));
return 0;
}
static int
@ -1434,7 +1425,7 @@ ata_promise_mio_dmastop(struct ata_channel *ch)
{
ATA_IDX_OUTL(ch, ATA_BMCTL_PORT,
ATA_IDX_INL(ch, ATA_BMCTL_PORT) & ~0x00000080);
return ata_dmastop(ch);
return 0;
}
/*

View File

@ -49,7 +49,8 @@ __FBSDID("$FreeBSD$");
static void ata_dmaalloc(struct ata_channel *);
static void ata_dmafree(struct ata_channel *);
static void ata_dmasetupd_cb(void *, bus_dma_segment_t *, int, int);
static int ata_dmasetup(struct ata_device *, caddr_t, int32_t);
static int ata_dmaload(struct ata_device *, caddr_t, int32_t, int);
static int ata_dmaunload(struct ata_channel *);
/* local vars */
static MALLOC_DEFINE(M_ATADMA, "ATA DMA", "ATA driver DMA");
@ -70,9 +71,8 @@ ata_dmainit(struct ata_channel *ch)
if ((ch->dma = malloc(sizeof(struct ata_dma), M_ATADMA, M_NOWAIT|M_ZERO))) {
ch->dma->alloc = ata_dmaalloc;
ch->dma->free = ata_dmafree;
ch->dma->setup = ata_dmasetup;
ch->dma->start = ata_dmastart;
ch->dma->stop = ata_dmastop;
ch->dma->load = ata_dmaload;
ch->dma->unload = ata_dmaunload;
ch->dma->alignment = 2;
ch->dma->max_iosize = 64*1024;
}
@ -194,38 +194,35 @@ ata_dmasetupd_cb(void *xsc, bus_dma_segment_t *segs, int nsegs, int error)
}
static int
ata_dmasetup(struct ata_device *atadev, caddr_t data, int32_t count)
ata_dmaload(struct ata_device *atadev, caddr_t data, int32_t count, int dir)
{
struct ata_channel *ch = atadev->channel;
struct ata_dmasetup_data_cb_args cba;
if (((uintptr_t)data & (ch->dma->alignment - 1)) ||
(count & (ch->dma->alignment - 1))) {
ata_prtdev(atadev, "FAILURE - non aligned DMA transfer attempted\n");
if (ch->dma->flags & ATA_DMA_ACTIVE) {
ata_prtdev(atadev, "FAILURE - already active DMA on this device\n");
return -1;
}
if (!count) {
ata_prtdev(atadev, "FAILURE - zero length DMA transfer attempted\n");
return -1;
}
if (((uintptr_t)data & (ch->dma->alignment - 1)) ||
(count & (ch->dma->alignment - 1))) {
ata_prtdev(atadev, "FAILURE - non aligned DMA transfer attempted\n");
return -1;
}
if (count > ch->dma->max_iosize) {
ata_prtdev(atadev,
"FAILURE - oversized DMA transfer attempted %d > %d\n",
count, ch->dma->max_iosize);
return -1;
}
return 0;
}
int
ata_dmastart(struct ata_channel *ch, caddr_t data, int32_t count, int dir)
{
struct ata_dmasetup_data_cb_args cba;
if (ch->dma->flags & ATA_DMA_ACTIVE)
panic("ata_dmasetup: transfer active on this device!");
cba.dmatab = ch->dma->dmatab;
bus_dmamap_sync(ch->dma->cdmatag, ch->dma->cdmamap, BUS_DMASYNC_PREWRITE);
if (bus_dmamap_load(ch->dma->ddmatag, ch->dma->ddmamap, data, count,
ata_dmasetupd_cb, &cba, 0) || cba.error)
return -1;
@ -233,12 +230,14 @@ ata_dmastart(struct ata_channel *ch, caddr_t data, int32_t count, int dir)
bus_dmamap_sync(ch->dma->ddmatag, ch->dma->ddmamap,
dir ? BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE);
ch->dma->cur_iosize = count;
ch->dma->flags = dir ? (ATA_DMA_ACTIVE | ATA_DMA_READ) : ATA_DMA_ACTIVE;
return 0;
}
int
ata_dmastop(struct ata_channel *ch)
ata_dmaunload(struct ata_channel *ch)
{
bus_dmamap_sync(ch->dma->cdmatag, ch->dma->cdmamap, BUS_DMASYNC_POSTWRITE);
@ -247,6 +246,8 @@ ata_dmastop(struct ata_channel *ch)
BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE);
bus_dmamap_unload(ch->dma->ddmatag, ch->dma->ddmamap);
ch->dma->cur_iosize = 0;
ch->dma->flags = 0;
return 0;
}

View File

@ -111,10 +111,11 @@ ata_transaction(struct ata_request *request)
/* ATA DMA data transfer commands */
case ATA_R_DMA:
/* check sanity and setup DMA engine */
if (request->device->channel->dma->setup(request->device,
request->data,
request->bytecount)) {
/* check sanity, setup SG list and DMA engine */
if (request->device->channel->dma->load(request->device,
request->data,
request->bytecount,
request->flags & ATA_R_READ)) {
ata_prtdev(request->device, "setting up DMA failed\n");
request->result = EIO;
break;
@ -130,10 +131,7 @@ ata_transaction(struct ata_request *request)
}
/* start DMA engine */
if (request->device->channel->dma->start(request->device->channel,
request->data,
request->bytecount,
request->flags & ATA_R_READ)) {
if (request->device->channel->dma->start(request->device->channel)) {
ata_prtdev(request->device, "error starting DMA\n");
request->result = EIO;
break;
@ -208,10 +206,11 @@ ata_transaction(struct ata_request *request)
break;
}
/* check sanity and setup DMA engine */
if (request->device->channel->dma->setup(request->device,
request->data,
request->bytecount)) {
/* check sanity, setup SG list and DMA engine */
if (request->device->channel->dma->load(request->device,
request->data,
request->bytecount,
request->flags & ATA_R_READ)) {
ata_prtdev(request->device, "setting up DMA failed\n");
request->result = EIO;
break;
@ -253,10 +252,7 @@ ata_transaction(struct ata_request *request)
ATA_PROTO_ATAPI_12 ? 6 : 8);
/* start DMA engine */
if (request->device->channel->dma->start(request->device->channel,
request->data,
request->bytecount,
request->flags & ATA_R_READ)) {
if (request->device->channel->dma->start(request->device->channel)) {
request->result = EIO;
break;
}
@ -373,6 +369,9 @@ ata_interrupt(void *data)
else
request->donecount = request->bytecount;
/* release SG list etc */
ch->dma->unload(ch);
/* done with HW */
break;
@ -458,6 +457,7 @@ ata_interrupt(void *data)
ata_prtdev(request->device, "unknown transfer phase\n");
request->status = ATA_S_ERROR;
}
/* done with HW */
break;
@ -474,6 +474,9 @@ ata_interrupt(void *data)
request->status |= ATA_S_ERROR;
else
request->donecount = request->bytecount;
/* release SG list etc */
ch->dma->unload(ch);
/* done with HW */
break;
@ -481,7 +484,7 @@ ata_interrupt(void *data)
ata_finish(request);
/* unlock the ATA HW for new work */
/* unlock the ATA channel for new work */
ch->running = NULL;
ATA_UNLOCK_CH(ch);
ch->locking(ch, ATA_LF_UNLOCK);

View File

@ -411,17 +411,14 @@ ata_pci_allocate(device_t dev, struct ata_channel *ch)
}
static int
ata_pci_dmastart(struct ata_channel *ch, caddr_t data, int32_t count, int dir)
ata_pci_dmastart(struct ata_channel *ch)
{
int error;
if ((error = ata_dmastart(ch, data, count, dir)))
return error;
ATA_IDX_OUTB(ch, ATA_BMSTAT_PORT, (ATA_IDX_INB(ch, ATA_BMSTAT_PORT) |
(ATA_BMSTAT_INTERRUPT | ATA_BMSTAT_ERROR)));
ATA_IDX_OUTL(ch, ATA_BMDTP_PORT, ch->dma->mdmatab);
ATA_IDX_OUTB(ch, ATA_BMCMD_PORT,
(dir ? ATA_BMCMD_WRITE_READ : 0) | ATA_BMCMD_START_STOP);
((ch->dma->flags & ATA_DMA_READ) ? ATA_BMCMD_WRITE_READ : 0) |
ATA_BMCMD_START_STOP);
return 0;
}
@ -434,7 +431,6 @@ ata_pci_dmastop(struct ata_channel *ch)
ATA_IDX_OUTB(ch, ATA_BMCMD_PORT,
ATA_IDX_INB(ch, ATA_BMCMD_PORT) & ~ATA_BMCMD_START_STOP);
ATA_IDX_OUTB(ch, ATA_BMSTAT_PORT, ATA_BMSTAT_INTERRUPT | ATA_BMSTAT_ERROR);
ata_dmastop(ch);
return error;
}