* [Qemu-devel] [PATCH] FDC: rework status0, status1, status2 handling
@ 2008-06-25 21:25 Hervé Poussineau
2008-06-26 7:53 ` J. Mayer
0 siblings, 1 reply; 2+ messages in thread
From: Hervé Poussineau @ 2008-06-25 21:25 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 366 bytes --]
Hi,
Attached patch fixes status0, status1 and status2 handling, which were
broken in read/write commands and in some subtle ways. Status values are
now calculated during command execution and not at the very end.
This allows removing of one hack in fdctrl_handle_sense_interrupt_status().
This also removes the FD_STATE_SEEK/FD_DID_SEEK stuff.
Hervé
[-- Attachment #2: fdc_status012.diff --]
[-- Type: text/plain, Size: 20326 bytes --]
Index: fdc.c
===================================================================
--- fdc.c (revision 4789)
+++ fdc.c (working copy)
@@ -116,54 +116,6 @@
return _fd_sector(drv->head, drv->track, drv->sect, drv->last_sect);
}
-/* Seek to a new position:
- * returns 0 if already on right track
- * returns 1 if track changed
- * returns 2 if track is invalid
- * returns 3 if sector is invalid
- * returns 4 if seek is disabled
- */
-static int fd_seek (fdrive_t *drv, uint8_t head, uint8_t track, uint8_t sect,
- int enable_seek)
-{
- uint32_t sector;
- int ret;
-
- if (track > drv->max_track ||
- (head != 0 && (drv->flags & FDISK_DBL_SIDES) == 0)) {
- FLOPPY_DPRINTF("try to read %d %02x %02x (max=%d %d %02x %02x)\n",
- head, track, sect, 1,
- (drv->flags & FDISK_DBL_SIDES) == 0 ? 0 : 1,
- drv->max_track, drv->last_sect);
- return 2;
- }
- if (sect > drv->last_sect) {
- FLOPPY_DPRINTF("try to read %d %02x %02x (max=%d %d %02x %02x)\n",
- head, track, sect, 1,
- (drv->flags & FDISK_DBL_SIDES) == 0 ? 0 : 1,
- drv->max_track, drv->last_sect);
- return 3;
- }
- sector = _fd_sector(head, track, sect, drv->last_sect);
- ret = 0;
- if (sector != fd_sector(drv)) {
-#if 0
- if (!enable_seek) {
- FLOPPY_ERROR("no implicit seek %d %02x %02x (max=%d %02x %02x)\n",
- head, track, sect, 1, drv->max_track, drv->last_sect);
- return 4;
- }
-#endif
- drv->head = head;
- if (drv->track != track)
- ret = 1;
- drv->track = track;
- drv->sect = sect;
- }
-
- return ret;
-}
-
/* Set drive back to track 0 */
static void fd_recalibrate (fdrive_t *drv)
{
@@ -302,7 +254,7 @@
static void fdctrl_reset_fifo (fdctrl_t *fdctrl);
static int fdctrl_transfer_handler (void *opaque, int nchan,
int dma_pos, int dma_len);
-static void fdctrl_raise_irq (fdctrl_t *fdctrl, uint8_t status0);
+static void fdctrl_raise_irq (fdctrl_t *fdctrl);
static uint32_t fdctrl_read_statusA (fdctrl_t *fdctrl);
static uint32_t fdctrl_read_statusB (fdctrl_t *fdctrl);
@@ -327,7 +279,6 @@
enum {
FD_STATE_MULTI = 0x01, /* multi track flag */
FD_STATE_FORMAT = 0x02, /* format flag */
- FD_STATE_SEEK = 0x04, /* seek flag */
};
enum {
@@ -463,7 +414,6 @@
};
#define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
-#define FD_DID_SEEK(state) ((state) & FD_STATE_SEEK)
#define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
struct fdctrl_t {
@@ -508,6 +458,61 @@
fdrive_t drives[MAX_FD];
};
+/* Seek to a new position:
+ * returns 0 if already on right track
+ * returns 1 if track changed
+ * returns 2 if track is invalid
+ * returns 3 if sector is invalid
+ * returns 4 if seek is disabled
+ */
+static int fd_seek (fdctrl_t *fdctrl, fdrive_t *drv,
+ uint8_t head, uint8_t track, uint8_t sect)
+{
+ uint32_t sector;
+ int ret;
+
+ if (track > drv->max_track ||
+ (head != 0 && (drv->flags & FDISK_DBL_SIDES) == 0)) {
+ FLOPPY_DPRINTF("try to read %d %02x %02x (max=%d %d %02x %02x)\n",
+ head, track, sect, 1,
+ (drv->flags & FDISK_DBL_SIDES) == 0 ? 0 : 1,
+ drv->max_track, drv->last_sect);
+ fdctrl->status0 |= FD_SR0_ABNTERM;
+ return 2;
+ }
+ if (sect > drv->last_sect) {
+ FLOPPY_DPRINTF("try to read %d %02x %02x (max=%d %d %02x %02x)\n",
+ head, track, sect, 1,
+ (drv->flags & FDISK_DBL_SIDES) == 0 ? 0 : 1,
+ drv->max_track, drv->last_sect);
+ fdctrl->status0 |= FD_SR0_ABNTERM;
+ fdctrl->status1 |= FD_SR1_EC;
+ return 3;
+ }
+ sector = _fd_sector(head, track, sect, drv->last_sect);
+ ret = 0;
+#if 0
+ if (!(fdctrl->config & FD_CONFIG_EIS) && (head != drv->head || track != drv->track)) {
+ FLOPPY_ERROR("no implicit seek %d %02x %02x (current=%d %02x %02x, max=%d %02x %02x)\n",
+ head, track, sect,
+ drv->head, drv->track, drv->sect,
+ 1, drv->max_track, drv->last_sect);
+ fdctrl->status0 |= FD_SR0_ABNTERM;
+ return 4;
+ }
+#endif
+ if (sector != fd_sector(drv)) {
+ drv->head = head;
+ if (drv->track != track)
+ ret = 1;
+ drv->track = track;
+ drv->sect = sect;
+ }
+
+ fdctrl->status0 |= FD_SR0_SEEK;
+ return ret;
+}
+
static uint32_t fdctrl_read (void *opaque, uint32_t reg)
{
fdctrl_t *fdctrl = opaque;
@@ -739,21 +744,19 @@
fdctrl->sra &= ~FD_SRA_INTPEND;
}
-static void fdctrl_raise_irq (fdctrl_t *fdctrl, uint8_t status0)
+static void fdctrl_raise_irq (fdctrl_t *fdctrl)
{
/* Sparc mutation */
if (fdctrl->sun4m && (fdctrl->msr & FD_MSR_CMDBUSY)) {
/* XXX: not sure */
fdctrl->msr &= ~FD_MSR_CMDBUSY;
fdctrl->msr |= FD_MSR_RQM | FD_MSR_DIO;
- fdctrl->status0 = status0;
return;
}
if (!(fdctrl->sra & FD_SRA_INTPEND)) {
qemu_set_irq(fdctrl->irq, 1);
fdctrl->sra |= FD_SRA_INTPEND;
}
- fdctrl->status0 = status0;
FLOPPY_DPRINTF("Set interrupt status to 0x%02x\n", fdctrl->status0);
}
@@ -782,7 +785,8 @@
fd_recalibrate(&fdctrl->drives[i]);
fdctrl_reset_fifo(fdctrl);
if (do_irq) {
- fdctrl_raise_irq(fdctrl, FD_SR0_RDYCHG);
+ fdctrl->status0 |= FD_SR0_RDYCHG;
+ fdctrl_raise_irq(fdctrl);
}
}
@@ -819,7 +823,7 @@
static fdrive_t *get_cur_drv (fdctrl_t *fdctrl)
{
- switch (fdctrl->cur_drv) {
+ switch (GET_CUR_DRV(fdctrl)) {
case 0: return drv0(fdctrl);
case 1: return drv1(fdctrl);
#if MAX_FD == 4
@@ -1005,7 +1009,7 @@
fdctrl->data_pos = 0;
fdctrl->msr |= FD_MSR_CMDBUSY | FD_MSR_RQM | FD_MSR_DIO;
if (do_irq)
- fdctrl_raise_irq(fdctrl, 0x00);
+ fdctrl_raise_irq(fdctrl);
}
/* Set an error: unimplemented/unknown command */
@@ -1034,11 +1038,13 @@
} else {
cur_drv->head = 0;
cur_drv->track++;
+ fdctrl->status0 |= FD_SR0_SEEK;
if ((cur_drv->flags & FDISK_DBL_SIDES) == 0)
return 0;
}
} else {
cur_drv->track++;
+ fdctrl->status0 |= FD_SR0_SEEK;
return 0;
}
FLOPPY_DPRINTF("seek to next track (%d %02x %02x => %d)\n",
@@ -1051,18 +1057,21 @@
}
/* Callback for transfer end (stop or abort) */
-static void fdctrl_stop_transfer (fdctrl_t *fdctrl, uint8_t status0,
- uint8_t status1, uint8_t status2)
+static void fdctrl_stop_transfer (fdctrl_t *fdctrl)
{
fdrive_t *cur_drv;
+ uint8_t status0 = fdctrl->status0;
cur_drv = get_cur_drv(fdctrl);
+ /* Add head and drive to status0 */
+ status0 |= (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
+
FLOPPY_DPRINTF("transfer status: %02x %02x %02x (%02x)\n",
- status0, status1, status2,
- status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl));
- fdctrl->fifo[0] = status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
- fdctrl->fifo[1] = status1;
- fdctrl->fifo[2] = status2;
+ fdctrl->status0, fdctrl->status1, fdctrl->status2,
+ status0);
+ fdctrl->fifo[0] = status0;
+ fdctrl->fifo[1] = fdctrl->status1;
+ fdctrl->fifo[2] = fdctrl->status2;
fdctrl->fifo[3] = cur_drv->track;
fdctrl->fifo[4] = cur_drv->head;
fdctrl->fifo[5] = cur_drv->sect;
@@ -1081,7 +1090,6 @@
{
fdrive_t *cur_drv;
uint8_t kh, kt, ks;
- int did_seek = 0;
SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
cur_drv = get_cur_drv(fdctrl);
@@ -1091,31 +1099,20 @@
FLOPPY_DPRINTF("Start transfer at %d %d %02x %02x (%d)\n",
GET_CUR_DRV(fdctrl), kh, kt, ks,
_fd_sector(kh, kt, ks, cur_drv->last_sect));
- switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) {
+ switch (fd_seek(fdctrl, cur_drv, kh, kt, ks)) {
+ case 0:
+ case 1:
+ /* success */
+ break;
case 2:
- /* sect too big */
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00);
- fdctrl->fifo[3] = kt;
- fdctrl->fifo[4] = kh;
- fdctrl->fifo[5] = ks;
- return;
case 3:
- /* track too big */
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00);
- fdctrl->fifo[3] = kt;
- fdctrl->fifo[4] = kh;
- fdctrl->fifo[5] = ks;
- return;
case 4:
- /* No seek enabled */
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00);
+ /* error */
+ fdctrl_stop_transfer(fdctrl);
fdctrl->fifo[3] = kt;
fdctrl->fifo[4] = kh;
fdctrl->fifo[5] = ks;
return;
- case 1:
- did_seek = 1;
- break;
default:
break;
}
@@ -1128,10 +1125,6 @@
fdctrl->data_state |= FD_STATE_MULTI;
else
fdctrl->data_state &= ~FD_STATE_MULTI;
- if (did_seek)
- fdctrl->data_state |= FD_STATE_SEEK;
- else
- fdctrl->data_state &= ~FD_STATE_SEEK;
if (fdctrl->fifo[5] == 00) {
fdctrl->data_len = fdctrl->fifo[8];
} else {
@@ -1173,7 +1166,7 @@
if (direction != FD_DIR_WRITE)
fdctrl->msr |= FD_MSR_DIO;
/* IO based transfer: calculate len */
- fdctrl_raise_irq(fdctrl, 0x00);
+ fdctrl_raise_irq(fdctrl);
return;
}
@@ -1186,7 +1179,8 @@
/* We don't handle deleted data,
* so we don't return *ANYTHING*
*/
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
+ fdctrl->status0 |= FD_SR0_ABNTERM;
+ fdctrl_stop_transfer(fdctrl);
}
/* handlers for DMA transfers */
@@ -1196,7 +1190,6 @@
fdctrl_t *fdctrl;
fdrive_t *cur_drv;
int len, start_pos, rel_pos;
- uint8_t status0 = 0x00, status1 = 0x00, status2 = 0x00;
fdctrl = opaque;
if (fdctrl->msr & FD_MSR_RQM) {
@@ -1204,16 +1197,11 @@
return 0;
}
cur_drv = get_cur_drv(fdctrl);
- if (fdctrl->data_dir == FD_DIR_SCANE || fdctrl->data_dir == FD_DIR_SCANL ||
- fdctrl->data_dir == FD_DIR_SCANH)
- status2 = FD_SR2_SNS;
if (dma_len > fdctrl->data_len)
dma_len = fdctrl->data_len;
if (cur_drv->bs == NULL) {
- if (fdctrl->data_dir == FD_DIR_WRITE)
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
- else
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00);
+ fdctrl->status0 |= FD_SR0_ABNTERM;
+ fdctrl_stop_transfer(fdctrl);
len = 0;
goto transfer_error;
}
@@ -1251,7 +1239,8 @@
if (bdrv_write(cur_drv->bs, fd_sector(cur_drv),
fdctrl->fifo, 1) < 0) {
FLOPPY_ERROR("writing sector %d\n", fd_sector(cur_drv));
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
+ fdctrl->status0 |= FD_SR0_ABNTERM;
+ fdctrl_stop_transfer(fdctrl);
goto transfer_error;
}
break;
@@ -1263,14 +1252,16 @@
DMA_read_memory (nchan, tmpbuf, fdctrl->data_pos, len);
ret = memcmp(tmpbuf, fdctrl->fifo + rel_pos, len);
if (ret == 0) {
- status2 = FD_SR2_SEH;
+ fdctrl->status2 |= FD_SR2_SEH;
+ fdctrl->status2 &= ~FD_SR2_SNS;
goto end_transfer;
}
if ((ret < 0 && fdctrl->data_dir == FD_DIR_SCANL) ||
(ret > 0 && fdctrl->data_dir == FD_DIR_SCANH)) {
- status2 = 0x00;
+ fdctrl->status2 &= ~FD_SR2_SNS;
goto end_transfer;
}
+ fdctrl->status2 |= FD_SR2_SNS;
}
break;
}
@@ -1286,14 +1277,8 @@
len = fdctrl->data_pos - start_pos;
FLOPPY_DPRINTF("end transfer %d %d %d\n",
fdctrl->data_pos, len, fdctrl->data_len);
- if (fdctrl->data_dir == FD_DIR_SCANE ||
- fdctrl->data_dir == FD_DIR_SCANL ||
- fdctrl->data_dir == FD_DIR_SCANH)
- status2 = FD_SR2_SEH;
- if (FD_DID_SEEK(fdctrl->data_state))
- status0 |= FD_SR0_SEEK;
fdctrl->data_len -= len;
- fdctrl_stop_transfer(fdctrl, status0, status1, status2);
+ fdctrl_stop_transfer(fdctrl);
transfer_error:
return len;
@@ -1322,7 +1307,8 @@
fd_sector(cur_drv));
return 0;
}
- if (bdrv_read(cur_drv->bs, fd_sector(cur_drv), fdctrl->fifo, 1) < 0) {
+ if (cur_drv->bs == NULL ||
+ bdrv_read(cur_drv->bs, fd_sector(cur_drv), fdctrl->fifo, 1) < 0) {
FLOPPY_DPRINTF("error getting sector %d\n",
fd_sector(cur_drv));
/* Sure, image size is too small... */
@@ -1337,7 +1323,7 @@
* then from status mode to command mode
*/
if (fdctrl->msr & FD_MSR_NONDMA) {
- fdctrl_stop_transfer(fdctrl, FD_SR0_SEEK, 0x00, 0x00);
+ fdctrl_stop_transfer(fdctrl);
} else {
fdctrl_reset_fifo(fdctrl);
fdctrl_reset_irq(fdctrl);
@@ -1361,31 +1347,20 @@
FLOPPY_DPRINTF("format sector at %d %d %02x %02x (%d)\n",
GET_CUR_DRV(fdctrl), kh, kt, ks,
_fd_sector(kh, kt, ks, cur_drv->last_sect));
- switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) {
+ switch (fd_seek(fdctrl, cur_drv, kh, kt, ks)) {
+ case 0:
+ case 1:
+ /* success */
+ break;
case 2:
- /* sect too big */
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00);
- fdctrl->fifo[3] = kt;
- fdctrl->fifo[4] = kh;
- fdctrl->fifo[5] = ks;
- return;
case 3:
- /* track too big */
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00);
- fdctrl->fifo[3] = kt;
- fdctrl->fifo[4] = kh;
- fdctrl->fifo[5] = ks;
- return;
case 4:
- /* No seek enabled */
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00);
+ /* error */
+ fdctrl_stop_transfer(fdctrl);
fdctrl->fifo[3] = kt;
fdctrl->fifo[4] = kh;
fdctrl->fifo[5] = ks;
return;
- case 1:
- fdctrl->data_state |= FD_STATE_SEEK;
- break;
default:
break;
}
@@ -1393,15 +1368,13 @@
if (cur_drv->bs == NULL ||
bdrv_write(cur_drv->bs, fd_sector(cur_drv), fdctrl->fifo, 1) < 0) {
FLOPPY_ERROR("formatting sector %d\n", fd_sector(cur_drv));
- fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
+ fdctrl->status0 |= FD_SR0_ABNTERM;
+ fdctrl_stop_transfer(fdctrl);
} else {
if (cur_drv->sect == cur_drv->last_sect) {
fdctrl->data_state &= ~FD_STATE_FORMAT;
/* Last sector done */
- if (FD_DID_SEEK(fdctrl->data_state))
- fdctrl_stop_transfer(fdctrl, FD_SR0_SEEK, 0x00, 0x00);
- else
- fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
+ fdctrl_stop_transfer(fdctrl);
} else {
/* More to do */
fdctrl->data_pos = 0;
@@ -1512,7 +1485,9 @@
{
fdrive_t *cur_drv = get_cur_drv(fdctrl);
- /* XXX: should set main status register to busy */
+ /* Set main status register to busy */
+ fdctrl->msr &= ~FD_MSR_RQM;
+
cur_drv->head = (fdctrl->fifo[1] >> 2) & 1;
qemu_mod_timer(fdctrl->result_timer,
qemu_get_clock(vm_clock) + (ticks_per_sec / 50));
@@ -1529,7 +1504,6 @@
fdctrl->data_state |= FD_STATE_MULTI;
else
fdctrl->data_state &= ~FD_STATE_MULTI;
- fdctrl->data_state &= ~FD_STATE_SEEK;
cur_drv->bps =
fdctrl->fifo[2] > 7 ? 16384 : 128 << fdctrl->fifo[2];
#if 0
@@ -1544,7 +1518,7 @@
* the sector with the specified fill byte
*/
fdctrl->data_state &= ~FD_STATE_FORMAT;
- fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
+ fdctrl_stop_transfer(fdctrl);
}
static void fdctrl_handle_specify (fdctrl_t *fdctrl, int direction)
@@ -1584,23 +1558,16 @@
fd_recalibrate(cur_drv);
fdctrl_reset_fifo(fdctrl);
/* Raise Interrupt */
- fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
+ fdctrl->status0 |= FD_SR0_SEEK;
+ fdctrl_raise_irq(fdctrl);
}
static void fdctrl_handle_sense_interrupt_status (fdctrl_t *fdctrl, int direction)
{
fdrive_t *cur_drv = get_cur_drv(fdctrl);
-#if 0
fdctrl->fifo[0] =
fdctrl->status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
-#else
- /* XXX: status0 handling is broken for read/write
- commands, so we do this hack. It should be suppressed
- ASAP */
- fdctrl->fifo[0] =
- FD_SR0_SEEK | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
-#endif
fdctrl->fifo[1] = cur_drv->track;
fdctrl_set_fifo(fdctrl, 2, 0);
fdctrl_reset_irq(fdctrl);
@@ -1614,13 +1581,13 @@
SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
cur_drv = get_cur_drv(fdctrl);
fdctrl_reset_fifo(fdctrl);
- if (fdctrl->fifo[2] > cur_drv->max_track) {
- fdctrl_raise_irq(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK);
- } else {
+ fdctrl->status0 |= FD_SR0_SEEK;
+ if (fdctrl->fifo[2] > cur_drv->max_track)
+ fdctrl->status0 |= FD_SR0_ABNTERM;
+ else
cur_drv->track = fdctrl->fifo[2];
- /* Raise Interrupt */
- fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
- }
+ /* Raise Interrupt */
+ fdctrl_raise_irq(fdctrl);
}
static void fdctrl_handle_perpendicular_mode (fdctrl_t *fdctrl, int direction)
@@ -1689,7 +1656,8 @@
}
fdctrl_reset_fifo(fdctrl);
/* Raise Interrupt */
- fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
+ fdctrl->status0 |= FD_SR0_SEEK;
+ fdctrl_raise_irq(fdctrl);
}
static void fdctrl_handle_relative_seek_in (fdctrl_t *fdctrl, int direction)
@@ -1705,7 +1673,8 @@
}
fdctrl_reset_fifo(fdctrl);
/* Raise Interrupt */
- fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
+ fdctrl->status0 |= FD_SR0_SEEK;
+ fdctrl_raise_irq(fdctrl);
}
static const struct {
@@ -1776,7 +1745,8 @@
if (pos == FD_SECTOR_LEN - 1 ||
fdctrl->data_pos == fdctrl->data_len) {
cur_drv = get_cur_drv(fdctrl);
- if (bdrv_write(cur_drv->bs, fd_sector(cur_drv), fdctrl->fifo, 1) < 0) {
+ if (cur_drv->bs == NULL ||
+ bdrv_write(cur_drv->bs, fd_sector(cur_drv), fdctrl->fifo, 1) < 0) {
FLOPPY_ERROR("writing sector %d\n", fd_sector(cur_drv));
return;
}
@@ -1790,7 +1760,7 @@
* then from status mode to command mode
*/
if (fdctrl->data_pos == fdctrl->data_len)
- fdctrl_stop_transfer(fdctrl, FD_SR0_SEEK, 0x00, 0x00);
+ fdctrl_stop_transfer(fdctrl);
return;
}
if (fdctrl->data_pos == 0) {
@@ -1811,6 +1781,15 @@
return;
}
+ if (fdctrl->fifo[0] != FD_CMD_SENSE_INTERRUPT_STATUS)
+ {
+ /* Reset status0, except for SENSE_INTERRUPT_STATUS
+ * which returns it */
+ fdctrl->status0 = 0;
+ }
+ fdctrl->status1 = 0;
+ fdctrl->status2 = 0;
+
pos = command_to_handler[fdctrl->fifo[0] & 0xff];
FLOPPY_DPRINTF("treat %s command\n", handlers[pos].name);
(*handlers[pos].handler)(fdctrl, handlers[pos].direction);
@@ -1829,7 +1808,7 @@
if (cur_drv->last_sect != 0) {
cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
}
- fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
+ fdctrl_stop_transfer(fdctrl);
}
/* Init functions */
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] FDC: rework status0, status1, status2 handling
2008-06-25 21:25 [Qemu-devel] [PATCH] FDC: rework status0, status1, status2 handling Hervé Poussineau
@ 2008-06-26 7:53 ` J. Mayer
0 siblings, 0 replies; 2+ messages in thread
From: J. Mayer @ 2008-06-26 7:53 UTC (permalink / raw)
To: qemu-devel
On Wed, 2008-06-25 at 23:25 +0200, Hervé Poussineau wrote:
> Hi,
>
> Attached patch fixes status0, status1 and status2 handling, which were
> broken in read/write commands and in some subtle ways. Status values are
> now calculated during command execution and not at the very end.
> This allows removing of one hack in fdctrl_handle_sense_interrupt_status().
Hi,
this way of proceeding seem very strange, as actual hardware do compute
the status bytes in the FIFO when entering the status phase. This is
quite logical as those bytes are supposed to reflect the state of the
hardware at the end of the command completion and cannot be computed
before, apart from a few bits, like the DID_SEEK bit.
Furthermore, doing this way, you have to introduce weird hack in the
main access routine :
@@ -1811,6 +1781,15 @@
> return;
> }
>
> + if (fdctrl->fifo[0] != FD_CMD_SENSE_INTERRUPT_STATUS)
> + {
> + /* Reset status0, except for SENSE_INTERRUPT_STATUS
> + * which returns it */
> + fdctrl->status0 = 0;
> + }
> + fdctrl->status1 = 0;
> + fdctrl->status2 = 0;
> +
what is the exact bug and why isn't it possible to try to going on doing
things like actual hardware do ?
Another remark: moving code, like you do in this patch make very
difficult to figure what the real changes are and to track the
regressions if any. Could you please avoid introducing such artificial
diffs ?
--
J. Mayer <l_indien@magic.fr>
Never organized
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2008-06-26 7:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-25 21:25 [Qemu-devel] [PATCH] FDC: rework status0, status1, status2 handling Hervé Poussineau
2008-06-26 7:53 ` J. Mayer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).