From 25c765768f9502725e2b9a782c16491c125d6c55 Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 26 Dec 2024 16:46:17 +0100 Subject: [PATCH 01/52] feat(floppy): implemented floppy writing --- src/fdc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/floppy.c | 64 +++++++++++++++++++++++-- src/floppy.h | 5 +- 3 files changed, 195 insertions(+), 6 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 59fe19b..ae98dc6 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -109,16 +109,23 @@ typedef struct rw_args_t { /* Command callbacks prototypes */ static void pre_exec_specify(void); +static void pre_exec_write_data(void); +static uint8_t exec_write_data(uint8_t value); +static void post_exec_write_data(void); static void pre_exec_read_data(void); static uint8_t exec_read_data(uint8_t value); static void post_exec_read_data(void); static void pre_exec_recalibrate(void); static void post_exec_sense_interrupt(void); +static void pre_exec_format_track(void); +static uint8_t exec_format_track(uint8_t value); +static void post_exec_format_track(void); static void pre_exec_seek(void); /* Utility routines prototypes */ static void fdc_compute_next_status(void); // Update read buffer with the data from current ths static void buffer_update(void); +static void buffer_write_size(void); /* Local variables */ // The command descriptors @@ -131,6 +138,14 @@ static const fdc_operation_t fdc_operations[] = { .exec = NULL, .post_exec = NULL, }, + { + .cmd = WRITE_DATA, + .args_len = 8, + .result_len = 7, + .pre_exec = pre_exec_write_data, + .exec = exec_write_data, + .post_exec = post_exec_write_data, + }, { .cmd = READ_DATA, .args_len = 8, @@ -155,6 +170,14 @@ static const fdc_operation_t fdc_operations[] = { .exec = NULL, .post_exec = post_exec_sense_interrupt, }, + { + .cmd = FORMAT_TRACK, + .args_len = 5, + .result_len = 7, + .pre_exec = pre_exec_format_track, + .exec = exec_format_track, + .post_exec = post_exec_format_track, + }, { .cmd = SEEK, .args_len = 2, @@ -202,6 +225,78 @@ static void pre_exec_specify(void) { LOG_DEBUG("HLT: %d\n", args[1] >> 1); } +// Write data: +static void pre_exec_write_data(void) { + // TODO(giuliof): eventually: use the appropriate structure + LOG_DEBUG("FDC Write Data\n"); + LOG_DEBUG("MF: %d\n", (command_args >> 6) & 0x01); + LOG_DEBUG("MT: %d\n", (command_args >> 7) & 0x01); + LOG_DEBUG("Drive: %d\n", args[0] & 0x3); + LOG_DEBUG("HD: %d\n", (args[0] >> 2) & 0x1); + LOG_DEBUG("Cyl: %d\n", args[1]); + LOG_DEBUG("Head: %d\n", args[2]); + LOG_DEBUG("Record: %d\n", args[3]); + LOG_DEBUG("N: %d\n", args[4]); + LOG_DEBUG("EOT: %d\n", args[5]); + LOG_DEBUG("GPL: %d\n", args[6]); + LOG_DEBUG("DTL: %d\n", args[7]); + + // Set DIO to read for Execution phase + status_register.dio = 1; + + buffer_write_size(); +} + +static uint8_t exec_write_data(uint8_t value) { + rw_args_t *rw_args = (rw_args_t *)args; + + exec_buffer[rwcount++] = value; + + if (rwcount == rwcount_max) { + uint8_t sector = rw_args->record; + // FDC counts sectors from 1 + assert(sector != 0); + // But all other routines counts sectors from 0 + sector--; + int ret = floppy_write_buffer(exec_buffer, rw_args->unit_select, + rw_args->head_address, + track[rw_args->unit_select], + rw_args->head, rw_args->cylinder, sector); + assert(ret > 0); + // TODO(giuliof): At the moment we do not support error codes, we assume + // the image is always loaded and valid + assert((size_t)ret <= sizeof(exec_buffer)); + + // Multi-sector mode (enabled by default). + // If read is not interrupted at the end of the sector, the next logical + // sector is loaded + rw_args->record++; + + // Last sector of the track + if (rw_args->record > rw_args->eot) { + // Multi track mode, if enabled the read operation go on on the next + // side of the same track + if (command_args & CMD_ARGS_MT_bm) { + rw_args->head_address = !rw_args->head_address; + rw_args->head = !rw_args->head; + }; + + // In any case, reached the end of track we start back from sector 1 + rw_args->record = 1; + } + + buffer_write_size(); + } + return 0; +} + +static void post_exec_write_data(void) { + LOG_DEBUG("Write has ended\n"); + // TODO(giulio): populate result (which is pretty the same for read, + // write, ...) + memset(result, 0x00, sizeof(result)); +} + // Read data: static void pre_exec_read_data(void) { // TODO(giuliof): eventually: use the appropriate structure @@ -297,6 +392,19 @@ static void post_exec_sense_interrupt(void) { result[1] = track[drive]; } +// Format track +static void pre_exec_format_track(void) { + LOG_DEBUG("FDC Format track\n"); +} + +static uint8_t exec_format_track(uint8_t value) { + return value; +} + +static void post_exec_format_track(void) { + LOG_DEBUG("FDC end Format track\n"); +} + // Seek static void pre_exec_seek(void) { uint8_t drive = args[0] & 0x03; @@ -393,6 +501,30 @@ static void buffer_update(void) { rwcount_max = (size_t)ret; } +static void buffer_write_size(void) { + rw_args_t *rw_args = (rw_args_t *)args; + + uint8_t sector = rw_args->record; + + // FDC counts sectors from 1 + assert(sector != 0); + + // But all other routines counts sectors from 0 + sector--; + + int ret = floppy_write_buffer( + NULL, rw_args->unit_select, rw_args->head_address, + track[rw_args->unit_select], rw_args->head, rw_args->cylinder, sector); + assert(ret > 0); + // TODO(giuliof): At the moment we do not support error codes, we assume the + // image is always loaded and valid + assert((size_t)ret <= sizeof(exec_buffer)); + + rwcount = 0; + // TODO(giuliof) rwcount_max = min(DTL, ret) + rwcount_max = (size_t)ret; +} + /* * * * * * * * * * * * * * * Public routines * * * * * * * * * * * * * * */ void fdc_init(void) { diff --git a/src/floppy.c b/src/floppy.c index 2bd8f4c..b67f4a1 100644 --- a/src/floppy.c +++ b/src/floppy.c @@ -21,7 +21,7 @@ ssize_t floppy_load_image(const char *filename, unsigned int unit_number) { floppy_unload_image(unit_number); // TODO(giuliof): if extension is ..., then image format is ... - FILE *fd = fopen(filename, "rb"); + FILE *fd = fopen(filename, "rb+"); if (fd == NULL) return -1; @@ -66,10 +66,6 @@ ssize_t floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, if (fd == NULL) return -1; - // TODO(giuliof): at the moment we support only drive 0 - if (unit_number != 0) - return -1; - // Due to its structure, with Ceda File Format the physical head and track // must be same as their logical counterpart. if (phy_head != head) @@ -114,3 +110,61 @@ ssize_t floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, return (int)len; } + +int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, + uint8_t phy_track, bool head, uint8_t track, + uint8_t sector) { + assert(unit_number < ARRAY_SIZE(floppy_units)); + + FILE *fd = floppy_units[unit_number].fd; + size_t len = 256; + long offset; + + // No disk loaded, raise error + if (fd == NULL) + return -1; + + // Due to its structure, with Ceda File Format the physical head and track + // must be same as their logical counterpart. + if (phy_head != head) + return -1; + if (phy_track != track) + return -1; + + // CFF has up to 80 tracks + if (track > 79) + return -1; + + // Locate sector start + if (track == 0 && head == 0) { + // First track has max 16 sectors + if (sector > 15) + return -1; + // Compute byte offset to sector start + offset = (long)sector * 256; + } else { + if (sector > 5) + return -1; + + // Compute byte offset to sector start + offset = (long)track * 1024 * 5 * 2; + offset += (long)head * 1024 * 5; + offset += (long)sector * 1024; + + // First track has a different format + offset -= (long)1024 * 5; + offset += (long)256 * 16; + + len = 1024; + } + + // If requested, load sector into buffer + if (buffer) { + if (fseek(fd, offset, SEEK_SET) < 0) + return -1; + + len = fwrite(buffer, sizeof(uint8_t), len, fd); + } + + return (int)len; +} \ No newline at end of file diff --git a/src/floppy.h b/src/floppy.h index d877de5..c109590 100644 --- a/src/floppy.h +++ b/src/floppy.h @@ -44,4 +44,7 @@ ssize_t floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, uint8_t phy_track, bool head, uint8_t track, uint8_t sector); -#endif \ No newline at end of file +int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, + uint8_t phy_track, bool head, uint8_t track, + uint8_t sector); +#endif From 4b3688670047d41f925a6ec4bd9b4a0ff2ccb94b Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 26 Dec 2024 16:47:01 +0100 Subject: [PATCH 02/52] style: newlines --- src/fdc.c | 3 ++- src/floppy.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index ae98dc6..2b4d582 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -185,7 +185,8 @@ static const fdc_operation_t fdc_operations[] = { .pre_exec = pre_exec_seek, .exec = NULL, .post_exec = NULL, - }}; + }, +}; // Current FDC status static fdc_status_t fdc_status = CMD; // Currently selected operation diff --git a/src/floppy.c b/src/floppy.c index b67f4a1..2d3a6dc 100644 --- a/src/floppy.c +++ b/src/floppy.c @@ -167,4 +167,4 @@ int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, } return (int)len; -} \ No newline at end of file +} From 5afcb4e9fc7c276592cb5ffae195a61cf9ba2e4a Mon Sep 17 00:00:00 2001 From: giuliof Date: Mon, 6 Jan 2025 00:11:57 +0100 Subject: [PATCH 03/52] fix: missing include in macro.h --- src/macro.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/macro.h b/src/macro.h index f92afbb..f8fd9c9 100644 --- a/src/macro.h +++ b/src/macro.h @@ -2,6 +2,7 @@ #define CEDA_MACRO_H #include +#include // TODO(giomba): maybe print a stack trace? #define CEDA_STRONG_ASSERT_NEQ(a, b) \ From fb4ddc263158d0a2650e7ca0383f9942bd818cfe Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 4 Jan 2025 00:38:30 +0100 Subject: [PATCH 04/52] feat(fdc): preliminary implementation of INT signal INT is used to notify asynchronous events to the MCU (i.e. seek command is sent, INT rises when the head has moved onto the requested track). This is used too to notify if new data burst is available during execution phase. If no disk is present and read command is requested, INT=0. In this implementation, a disk inserted after the read command, will trigger INT and it should unlock the reading process. Now we can handle at least one error from floppy.c. Other errors will raise an assertion and will be implemented later. --- src/fdc.c | 69 ++++++++++++++++++++++++++++++++++++++++++--------- src/fdc.h | 3 +++ src/floppy.c | 7 ++++-- src/upd8255.c | 8 +++--- 4 files changed, 68 insertions(+), 19 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 2b4d582..36d1198 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -4,6 +4,7 @@ #include #include "floppy.h" +#include "macro.h" #define LOG_LEVEL LOG_LVL_DEBUG #include "log.h" @@ -205,6 +206,7 @@ static uint8_t args[8]; static uint8_t exec_buffer[1024]; // Result buffer. Each command has maximum 7 bytes as argument. static uint8_t result[7]; +static bool isReady = false; /* FDC internal registers */ // Main Status Register @@ -263,10 +265,11 @@ static uint8_t exec_write_data(uint8_t value) { rw_args->head_address, track[rw_args->unit_select], rw_args->head, rw_args->cylinder, sector); - assert(ret > 0); // TODO(giuliof): At the moment we do not support error codes, we assume // the image is always loaded and valid - assert((size_t)ret <= sizeof(exec_buffer)); + CEDA_STRONG_ASSERT_TRUE(ret > 0); + // Buffer is statically allocated, be sure that the data can fit it + CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); // Multi-sector mode (enabled by default). // If read is not interrupted at the end of the sector, the next logical @@ -374,6 +377,9 @@ static void pre_exec_recalibrate(void) { LOG_DEBUG("Drive: %d\n", drive); track[drive] = 0; + + // We don't have to actually move the head. The drive is immediately ready + isReady = true; } // Sense interrupt: @@ -381,6 +387,9 @@ static void post_exec_sense_interrupt(void) { // TODO(giuliof): last accessed drive uint8_t drive = 0; + // After reading interrupt status, ready can be deasserted + isReady = false; + LOG_DEBUG("FDC Sense Interrupt\n"); /* Status Register 0 */ // Drive number @@ -415,6 +424,9 @@ static void pre_exec_seek(void) { LOG_DEBUG("Drive: %d\n", drive); LOG_DEBUG("HD: %d\n", (result[0] >> 2) & 0x01); LOG_DEBUG("NCN: %d\n", track[drive]); + + // We don't have to actually move the head. The drive is immediately ready + isReady = true; } /* * * * * * * * * * * * * * * Utility routines * * * * * * * * * * * * * * */ @@ -487,15 +499,29 @@ static void buffer_update(void) { ssize_t ret = floppy_read_buffer( NULL, rw_args->unit_select, rw_args->head_address, track[rw_args->unit_select], rw_args->head, rw_args->cylinder, sector); - assert(ret > 0); - // TODO(giuliof): At the moment we do not support error codes, we assume the - // image is always loaded and valid - assert((size_t)ret <= sizeof(exec_buffer)); - ret = floppy_read_buffer(exec_buffer, rw_args->unit_select, - rw_args->head_address, track[rw_args->unit_select], - rw_args->head, rw_args->cylinder, sector); - assert(ret > 0); + if (ret != 0) { + // TODO(giuliof): At the moment we do not support error codes, we assume + // the image is always loaded and valid + CEDA_STRONG_ASSERT_TRUE(ret > 0); + // Buffer is statically allocated, be sure that the data can fit it + CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); + + ret = floppy_read_buffer(exec_buffer, rw_args->unit_select, + rw_args->head_address, + track[rw_args->unit_select], rw_args->head, + rw_args->cylinder, sector); + // TODO(giuliof): At the moment we do not support error codes, we assume + // the image is always loaded and valid + CEDA_STRONG_ASSERT_TRUE(ret >= 0); + // Ready to serve data + isReady = true; + } + // TODO(giuliof): add proper error code, this is no mounted image + else { + // Not ready to serve data + isReady = false; + } rwcount = 0; // TODO(giuliof) rwcount_max = min(DTL, ret) @@ -516,10 +542,12 @@ static void buffer_write_size(void) { int ret = floppy_write_buffer( NULL, rw_args->unit_select, rw_args->head_address, track[rw_args->unit_select], rw_args->head, rw_args->cylinder, sector); - assert(ret > 0); + // TODO(giuliof): At the moment we do not support error codes, we assume the // image is always loaded and valid - assert((size_t)ret <= sizeof(exec_buffer)); + CEDA_STRONG_ASSERT_TRUE(ret > 0); + // Buffer is statically allocated, be sure that the data can fit it + CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); rwcount = 0; // TODO(giuliof) rwcount_max = min(DTL, ret) @@ -627,3 +655,20 @@ void fdc_tc_out(ceda_ioaddr_t address, uint8_t value) { fdc_compute_next_status(); } } + +// TODO(giuliof): After Execution Phase or EOR sector read, INT=1 +// (beginning of result phase). When first byte of result phase data +// is read, INT=0. +bool fdc_getIntStatus(void) { + return isReady; +} + +void fdc_kickDiskImage(void) { + if (fdc_status == EXEC && fdc_currop->cmd == READ_DATA) { + buffer_update(); + } + + if (fdc_status == EXEC && fdc_currop->cmd == WRITE_DATA) { + buffer_write_size(); + } +} diff --git a/src/fdc.h b/src/fdc.h index 7983353..1fdd5c8 100644 --- a/src/fdc.h +++ b/src/fdc.h @@ -4,11 +4,14 @@ #include "type.h" #include +#include void fdc_init(void); uint8_t fdc_in(ceda_ioaddr_t address); void fdc_out(ceda_ioaddr_t address, uint8_t value); void fdc_tc_out(ceda_ioaddr_t address, uint8_t value); +bool fdc_getIntStatus(void); +void fdc_kickDiskImage(void); #endif // CEDA_FDC_H diff --git a/src/floppy.c b/src/floppy.c index 2d3a6dc..ced4e9a 100644 --- a/src/floppy.c +++ b/src/floppy.c @@ -3,6 +3,7 @@ #include #include +#include "fdc.h" #include "macro.h" // TODO(giuliof): this structure will contain the type of image loaded. @@ -28,6 +29,8 @@ ssize_t floppy_load_image(const char *filename, unsigned int unit_number) { floppy_units[unit_number].fd = fd; + fdc_kickDiskImage(); + return 0; } @@ -64,7 +67,7 @@ ssize_t floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, // No disk loaded, raise error if (fd == NULL) - return -1; + return 0; // TODO(giuliof): no disk loaded error // Due to its structure, with Ceda File Format the physical head and track // must be same as their logical counterpart. @@ -122,7 +125,7 @@ int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, // No disk loaded, raise error if (fd == NULL) - return -1; + return 0; // TODO(giuliof): no disk loaded error // Due to its structure, with Ceda File Format the physical head and track // must be same as their logical counterpart. diff --git a/src/upd8255.c b/src/upd8255.c index f0b7c2f..823614d 100644 --- a/src/upd8255.c +++ b/src/upd8255.c @@ -1,6 +1,7 @@ #include "upd8255.h" #include "bus.h" +#include "fdc.h" #include "video.h" #include @@ -37,11 +38,8 @@ uint8_t upd8255_in(ceda_ioaddr_t address) { // C1: CRTC frame sync port_c |= (!!video_frameSync()) << 1; - // C2: FDC INT pin. - // TODO(giulio): After Execution Phase or EOR sector read, INT=1 - // (beginning of result phase). When first byte of result phase data - // is read, INT=0. - port_c |= 1U << 2; + // C2: FDC INT pin + port_c |= (!!fdc_getIntStatus()) << 2; // Cx -- to be implemented From 75734d60e69c23a77cdd56a9525231d189863a69 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 4 Jan 2025 01:16:29 +0100 Subject: [PATCH 05/52] refactor(fdc): dependency break between floppy and fdc modules Floppy.h now exposes only frontend methods and fdc doesn't have to include floppy.h. Access to the floppy disk image is provided through callbacks. This should help with fdc module testability. --- src/fdc.c | 77 ++++++++++++++++++++++++++++++++++++---------------- src/fdc.h | 6 +++- src/floppy.c | 43 ++++++++++++++++++++++++----- src/floppy.h | 22 --------------- 4 files changed, 94 insertions(+), 54 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 36d1198..fc30791 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -1,9 +1,9 @@ #include "fdc.h" #include +#include #include -#include "floppy.h" #include "macro.h" #define LOG_LEVEL LOG_LVL_DEBUG @@ -216,6 +216,15 @@ static main_status_register_t status_register; // Current track position static uint8_t track[4]; +/* Callbacks to handle floppy read and write */ +static int (*read_buffer_cb)(uint8_t *buffer, uint8_t unit_number, + bool phy_head, uint8_t phy_track, bool head, + uint8_t track, uint8_t sector) = NULL; + +static int (*write_buffer_cb)(uint8_t *buffer, uint8_t unit_number, + bool phy_head, uint8_t phy_track, bool head, + uint8_t track, uint8_t sector) = NULL; + /* * * * * * * * * * * * * * * Command routines * * * * * * * * * * * * * * */ // Specify: @@ -253,6 +262,9 @@ static void pre_exec_write_data(void) { static uint8_t exec_write_data(uint8_t value) { rw_args_t *rw_args = (rw_args_t *)args; + if (write_buffer_cb == NULL) + return 0; + exec_buffer[rwcount++] = value; if (rwcount == rwcount_max) { @@ -261,11 +273,10 @@ static uint8_t exec_write_data(uint8_t value) { assert(sector != 0); // But all other routines counts sectors from 0 sector--; - int ret = floppy_write_buffer(exec_buffer, rw_args->unit_select, - rw_args->head_address, - track[rw_args->unit_select], - rw_args->head, rw_args->cylinder, sector); - // TODO(giuliof): At the moment we do not support error codes, we assume + int ret = + write_buffer_cb(exec_buffer, rw_args->unit_select, + rw_args->head_address, track[rw_args->unit_select], + rw_args->head, rw_args->cylinder, sector); // the image is always loaded and valid CEDA_STRONG_ASSERT_TRUE(ret > 0); // Buffer is statically allocated, be sure that the data can fit it @@ -323,7 +334,7 @@ static void pre_exec_read_data(void) { // TODO(giuliof) create handles to manage more than one floppy image at a // time - // floppy_read_buffer(exec_buffer, track_size, head, track, sector); + // read_buffer_cb(exec_buffer, track_size, head, track, sector); // TODO(giuliof): may be a good idea to pass a sort of "floppy context" buffer_update(); } @@ -490,15 +501,23 @@ static void buffer_update(void) { uint8_t sector = rw_args->record; + // Default: no data ready to be served + isReady = false; + rwcount_max = 0; + // FDC counts sectors from 1 assert(sector != 0); // But all other routines counts sectors from 0 sector--; - ssize_t ret = floppy_read_buffer( - NULL, rw_args->unit_select, rw_args->head_address, - track[rw_args->unit_select], rw_args->head, rw_args->cylinder, sector); + if (read_buffer_cb == NULL) + return; + + // TODO(giuliof): add proper error code, zero is no mounted image + int ret = read_buffer_cb(NULL, rw_args->unit_select, rw_args->head_address, + track[rw_args->unit_select], rw_args->head, + rw_args->cylinder, sector); if (ret != 0) { // TODO(giuliof): At the moment we do not support error codes, we assume @@ -507,21 +526,16 @@ static void buffer_update(void) { // Buffer is statically allocated, be sure that the data can fit it CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); - ret = floppy_read_buffer(exec_buffer, rw_args->unit_select, - rw_args->head_address, - track[rw_args->unit_select], rw_args->head, - rw_args->cylinder, sector); + ret = read_buffer_cb(exec_buffer, rw_args->unit_select, + rw_args->head_address, track[rw_args->unit_select], + rw_args->head, rw_args->cylinder, sector); // TODO(giuliof): At the moment we do not support error codes, we assume // the image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret >= 0); + CEDA_STRONG_ASSERT_TRUE(ret > 0); + // Ready to serve data isReady = true; } - // TODO(giuliof): add proper error code, this is no mounted image - else { - // Not ready to serve data - isReady = false; - } rwcount = 0; // TODO(giuliof) rwcount_max = min(DTL, ret) @@ -533,15 +547,22 @@ static void buffer_write_size(void) { uint8_t sector = rw_args->record; + // Default: no data ready to be served + isReady = false; + rwcount_max = 0; + // FDC counts sectors from 1 assert(sector != 0); // But all other routines counts sectors from 0 sector--; - int ret = floppy_write_buffer( - NULL, rw_args->unit_select, rw_args->head_address, - track[rw_args->unit_select], rw_args->head, rw_args->cylinder, sector); + if (write_buffer_cb == NULL) + return; + + int ret = write_buffer_cb(NULL, rw_args->unit_select, rw_args->head_address, + track[rw_args->unit_select], rw_args->head, + rw_args->cylinder, sector); // TODO(giuliof): At the moment we do not support error codes, we assume the // image is always loaded and valid @@ -663,7 +684,15 @@ bool fdc_getIntStatus(void) { return isReady; } -void fdc_kickDiskImage(void) { +// TODO(giuliof): describe better this function +// Fast notes: if an image is loaded at runtime, check if the code is stuck in +// a read or write loop that was waiting for interrupt. +// In that case, remove the lock (buffer_update will do it, for the writing it +// has to be implemented) +void fdc_kickDiskImage(p_rwBuffer read_callback, p_rwBuffer write_callback) { + read_buffer_cb = read_callback; + write_buffer_cb = write_callback; + if (fdc_status == EXEC && fdc_currop->cmd == READ_DATA) { buffer_update(); } diff --git a/src/fdc.h b/src/fdc.h index 1fdd5c8..1b20360 100644 --- a/src/fdc.h +++ b/src/fdc.h @@ -6,12 +6,16 @@ #include #include +typedef int (*p_rwBuffer)(uint8_t *buffer, uint8_t unit_number, bool phy_head, + uint8_t phy_track, bool head, uint8_t track, + uint8_t sector); + void fdc_init(void); uint8_t fdc_in(ceda_ioaddr_t address); void fdc_out(ceda_ioaddr_t address, uint8_t value); void fdc_tc_out(ceda_ioaddr_t address, uint8_t value); bool fdc_getIntStatus(void); -void fdc_kickDiskImage(void); +void fdc_kickDiskImage(p_rwBuffer read_callback, p_rwBuffer write_callback); #endif // CEDA_FDC_H diff --git a/src/floppy.c b/src/floppy.c index ced4e9a..8293a2f 100644 --- a/src/floppy.c +++ b/src/floppy.c @@ -6,6 +6,33 @@ #include "fdc.h" #include "macro.h" +#define CEDA_FORMAT_MAXIMUM_TRACKS (80) +#define CEDA_FORMAT_MAXIMUM_SECTORS (80) +#define CEDA_FORMAT_FIRST_TRACK_SECTOR (80) + +/** + * @brief Read a sector from a certain drive + * + * @param buffer pointer to byte buffer where to load the sector. Buffer may be + * NULL to only fetch sector size. + * @param unit_number drive number where to unload the image + * @param phy_head physical head, from which disk side will the disk be read + * @param phy_track physical track, where the track is currently placed on the + * drive + * @param head logical head, must match with sector descriptor + * @param track logical track, must match with sector descriptor + * @param sector logical sector, which sector has to be read + * @return is 0 when successful, -1 for any kind of error (no image loaded, + * invalid parameters, ...) + */ +static int floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, + bool phy_head, uint8_t phy_track, bool head, + uint8_t track, uint8_t sector); + +static int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, + bool phy_head, uint8_t phy_track, bool head, + uint8_t track, uint8_t sector); + // TODO(giuliof): this structure will contain the type of image loaded. // At the moment, only Ceda File Format is supported (linearized binary disk // image) @@ -29,7 +56,7 @@ ssize_t floppy_load_image(const char *filename, unsigned int unit_number) { floppy_units[unit_number].fd = fd; - fdc_kickDiskImage(); + fdc_kickDiskImage(floppy_read_buffer, floppy_write_buffer); return 0; } @@ -42,6 +69,8 @@ ssize_t floppy_unload_image(unsigned int unit_number) { floppy_units[unit_number].fd = NULL; + fdc_kickDiskImage(NULL, NULL); + if (fclose(fd) < 0) return -1; @@ -56,9 +85,9 @@ ssize_t floppy_unload_image(unsigned int unit_number) { * first track with 256 bytes per sector and 16 sectors per track, others with * 1024 bps and 5 spt. The Ceda File Format reflects this formatting layout. */ -ssize_t floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, - uint8_t phy_track, bool head, uint8_t track, - uint8_t sector) { +static int floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, + bool phy_head, uint8_t phy_track, bool head, + uint8_t track, uint8_t sector) { assert(unit_number < ARRAY_SIZE(floppy_units)); FILE *fd = floppy_units[unit_number].fd; @@ -114,9 +143,9 @@ ssize_t floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, return (int)len; } -int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, - uint8_t phy_track, bool head, uint8_t track, - uint8_t sector) { +static int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, + bool phy_head, uint8_t phy_track, bool head, + uint8_t track, uint8_t sector) { assert(unit_number < ARRAY_SIZE(floppy_units)); FILE *fd = floppy_units[unit_number].fd; diff --git a/src/floppy.h b/src/floppy.h index c109590..f6611e2 100644 --- a/src/floppy.h +++ b/src/floppy.h @@ -25,26 +25,4 @@ ssize_t floppy_load_image(const char *filename, unsigned int unit_number); */ ssize_t floppy_unload_image(unsigned int unit_number); -/** - * @brief Read a sector from a certain drive - * - * @param buffer pointer to byte buffer where to load the sector. Buffer may be - * NULL to only fetch sector size. - * @param unit_number drive number where to unload the image - * @param phy_head physical head, from which disk side will the disk be read - * @param phy_track physical track, where the track is currently placed on the - * drive - * @param head logical head, must match with sector descriptor - * @param track logical track, must match with sector descriptor - * @param sector logical sector, which sector has to be read - * @return is 0 when successful, -1 for any kind of error (no image loaded, - * invalid parameters, ...) - */ -ssize_t floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, - uint8_t phy_track, bool head, uint8_t track, - uint8_t sector); - -int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, - uint8_t phy_track, bool head, uint8_t track, - uint8_t sector); #endif From cad18e76c22cec071f03f6ffc0728f023aa65c92 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 4 Jan 2025 01:19:19 +0100 Subject: [PATCH 06/52] fix(fdc): avoid read/write execution phases if no data available --- src/fdc.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/fdc.c b/src/fdc.c index fc30791..3a546d8 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -265,6 +265,11 @@ static uint8_t exec_write_data(uint8_t value) { if (write_buffer_cb == NULL) return 0; + if (rwcount_max == 0) { + LOG_WARN("Write execution happened when no data can be written"); + return 0; + } + exec_buffer[rwcount++] = value; if (rwcount == rwcount_max) { @@ -342,6 +347,11 @@ static void pre_exec_read_data(void) { static uint8_t exec_read_data(uint8_t value) { rw_args_t *rw_args = (rw_args_t *)args; + if (rwcount_max == 0) { + LOG_WARN("Read execution happened when no data can be read"); + return 0; + } + // read doesn't care of in value (void)value; uint8_t ret = exec_buffer[rwcount++]; From 49c0d3757c316aea30db91da5c7bf54fca289b64 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 4 Jan 2025 01:24:44 +0100 Subject: [PATCH 07/52] refactor(fdc): moved variable --- src/fdc.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 3a546d8..05d2d7b 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -9,8 +9,6 @@ #define LOG_LEVEL LOG_LVL_DEBUG #include "log.h" -uint8_t tc_status = 0; - // The FDC virtually expose two registers, which can be both read or written #define ADDR_STATUS_REGISTER (0x00) #define ADDR_DATA_REGISTER (0x01) @@ -206,6 +204,7 @@ static uint8_t args[8]; static uint8_t exec_buffer[1024]; // Result buffer. Each command has maximum 7 bytes as argument. static uint8_t result[7]; +static bool tc_status = false; static bool isReady = false; /* FDC internal registers */ @@ -480,7 +479,7 @@ static void fdc_compute_next_status(void) { } if (fdc_status == EXEC && (tc_status || fdc_currop->exec == NULL)) { - tc_status = 0; + tc_status = false; // Set DIO to read for RESULT phase status_register.dio = 1; @@ -682,7 +681,7 @@ void fdc_tc_out(ceda_ioaddr_t address, uint8_t value) { if (fdc_status == EXEC) { // TODO(giuliof) tc may be an argument to the fdc_compute_next_status, // since it is just a trigger. - tc_status = 1; + tc_status = true; fdc_compute_next_status(); } } From 4c59e0be6021ac8010192c84a4ba208205c05731 Mon Sep 17 00:00:00 2001 From: giuliof Date: Tue, 7 Jan 2025 23:33:03 +0100 Subject: [PATCH 08/52] test(fdc): implemented some preliminary tests for FDC --- src/fdc.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/src/fdc.c b/src/fdc.c index 05d2d7b..fee3bb7 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -710,3 +710,108 @@ void fdc_kickDiskImage(p_rwBuffer read_callback, p_rwBuffer write_callback) { buffer_write_size(); } } + +#ifdef CEDA_TEST + +#include + +Test(ceda_fdc, mainStatusRegisterWhenIdle) { + fdc_init(); + + // Assure that drive is waiting for command + cr_assert_eq(fdc_status, CMD); + + // Try to read status register and check that it is idle + uint8_t sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7)); +} + +Test(ceda_fdc, specifyCommand) { + fdc_init(); + + uint8_t sr; + + // Try to read status register and check that it is idle + fdc_out(ADDR_DATA_REGISTER, SPECIFY); + + // Now read status register to check that FDC is ready to receive arguments + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 4)); + + // Pass dummy arguments + fdc_out(ADDR_DATA_REGISTER, 0x00); + fdc_out(ADDR_DATA_REGISTER, 0x00); + + // FDC is no more busy + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7)); +} + +Test(ceda_fdc, senseInterruptStatusCommand) { + fdc_init(); + + uint8_t sr; + + fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); + + // This command has no arguments + // FDC should be ready to give response + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + + // First response byte is SR0 with interrupt code = 0 and Seek End = 1 + sr = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(sr, (1 << 5)); + + // FDC has another byte of response + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + + // Second response byte is current cylinder, which should be zero at reset + sr = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(sr, 0); +} + +Test(ceda_fdc, seekCommand) { + fdc_init(); + + uint8_t sr; + + fdc_out(ADDR_DATA_REGISTER, SEEK); + + // Now read status register to check that FDC is ready to receive arguments + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 4)); + + // First argument is number of drive + fdc_out(ADDR_DATA_REGISTER, 0x00); + // Second argument is cylinder position + fdc_out(ADDR_DATA_REGISTER, 5); + + // FDC is no more busy + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7)); + + // A sense interrupt command is expected after SEEK + fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); + + // This command has no arguments + // FDC should be ready to give response + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + + // First response byte is SR0 with interrupt code = 0 and Seek End = 1 + sr = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(sr, (1 << 5)); + + // FDC has another byte of response + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + + // Second response byte is current cylinder, which should be the one + // specified by the seek argument + sr = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(sr, 5); +} + +#endif From 569e4dd696364270bb13d8764d7bb03a0805583b Mon Sep 17 00:00:00 2001 From: giuliof Date: Wed, 8 Jan 2025 18:40:23 +0100 Subject: [PATCH 09/52] refactor(fdc): carried out defines and commands which represents fdc interface for tests --- src/fdc.c | 33 +-------------------------------- src/fdc_registers.h | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 32 deletions(-) create mode 100644 src/fdc_registers.h diff --git a/src/fdc.c b/src/fdc.c index fee3bb7..0f8cee0 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -4,43 +4,12 @@ #include #include +#include "fdc_registers.h" #include "macro.h" #define LOG_LEVEL LOG_LVL_DEBUG #include "log.h" -// The FDC virtually expose two registers, which can be both read or written -#define ADDR_STATUS_REGISTER (0x00) -#define ADDR_DATA_REGISTER (0x01) - -// The FDC can perform 15 different commands. -// Don't ask me why they didn't use a single nibble to represent all the -// commands. Please refer to the datasheet for more information. -typedef enum cmd_t { - READ_TRACK = 0x02, - SPECIFY = 0x03, - SENSE_DRIVE = 0x04, - WRITE_DATA = 0x05, - READ_DATA = 0x06, - RECALIBRATE = 0x07, - SENSE_INTERRUPT = 0x08, - WRITE_DELETED_DATA = 0x09, - READ_ID = 0x0A, - READ_DELETED_DATA = 0x0C, - FORMAT_TRACK = 0x0D, - SEEK = 0x0F, - SCAN_EQUAL = 0x11, - SCAN_LOW_EQUAL = 0x19, - SCAN_HIGH_EQUAL = 0x1D -} cmd_t; - -// Some commands carry argument bits in MSb -#define CMD_COMMAND_bm (0x1F) -#define CMD_ARGS_bm (0xE0) -#define CMD_ARGS_MT_bm (0x80) -#define CMD_ARGS_MF_bm (0x40) -#define CMD_ARGS_SK_bm (0x20) - // Each FDC command sequence can be split in four phases. // The single-byte command must be always sent (CMD). // Optional arguments may follow (ARGS). diff --git a/src/fdc_registers.h b/src/fdc_registers.h new file mode 100644 index 0000000..44d0bcf --- /dev/null +++ b/src/fdc_registers.h @@ -0,0 +1,36 @@ +#ifndef CEDA_FDC_REGISTERS_H_ +#define CEDA_FDC_REGISTERS_H_ + +// The FDC virtually expose two registers, which can be both read or written +#define ADDR_STATUS_REGISTER (0x00) +#define ADDR_DATA_REGISTER (0x01) + +// The FDC can perform 15 different commands. +// Don't ask me why they didn't use a single nibble to represent all the +// commands. Please refer to the datasheet for more information. +typedef enum cmd_t { + READ_TRACK = 0x02, + SPECIFY = 0x03, + SENSE_DRIVE = 0x04, + WRITE_DATA = 0x05, + READ_DATA = 0x06, + RECALIBRATE = 0x07, + SENSE_INTERRUPT = 0x08, + WRITE_DELETED_DATA = 0x09, + READ_ID = 0x0A, + READ_DELETED_DATA = 0x0C, + FORMAT_TRACK = 0x0D, + SEEK = 0x0F, + SCAN_EQUAL = 0x11, + SCAN_LOW_EQUAL = 0x19, + SCAN_HIGH_EQUAL = 0x1D +} cmd_t; + +// Some commands carry argument bits in MSb +#define CMD_COMMAND_bm (0x1F) +#define CMD_ARGS_bm (0xE0) +#define CMD_ARGS_MT_bm (0x80) +#define CMD_ARGS_MF_bm (0x40) +#define CMD_ARGS_SK_bm (0x20) + +#endif // CEDA_FDC_REGISTERS_H_ \ No newline at end of file From 383d3056995ca94e25768f08094afc2172213306 Mon Sep 17 00:00:00 2001 From: giuliof Date: Wed, 8 Jan 2025 18:46:07 +0100 Subject: [PATCH 10/52] refactor(fdc): moved tests in a separate file --- CMakeLists.txt | 1 + src/fdc.c | 105 ------------------------------------------- src/tests/test_fdc.c | 102 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 105 deletions(-) create mode 100644 src/tests/test_fdc.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 683d63a..98f0a35 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,6 +28,7 @@ add_executable(ceda src/ram/dynamic.c src/bios.c src/3rd/disassembler.c + src/tests/test_fdc.c ) target_compile_options(ceda PRIVATE -W -Wformat -Wall -Wundef -Wpointer-arith -Wcast-qual -Wwrite-strings -Wsign-compare -Wmissing-noreturn -Wextra -Wconversion) diff --git a/src/fdc.c b/src/fdc.c index 0f8cee0..caf8131 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -679,108 +679,3 @@ void fdc_kickDiskImage(p_rwBuffer read_callback, p_rwBuffer write_callback) { buffer_write_size(); } } - -#ifdef CEDA_TEST - -#include - -Test(ceda_fdc, mainStatusRegisterWhenIdle) { - fdc_init(); - - // Assure that drive is waiting for command - cr_assert_eq(fdc_status, CMD); - - // Try to read status register and check that it is idle - uint8_t sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7)); -} - -Test(ceda_fdc, specifyCommand) { - fdc_init(); - - uint8_t sr; - - // Try to read status register and check that it is idle - fdc_out(ADDR_DATA_REGISTER, SPECIFY); - - // Now read status register to check that FDC is ready to receive arguments - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 4)); - - // Pass dummy arguments - fdc_out(ADDR_DATA_REGISTER, 0x00); - fdc_out(ADDR_DATA_REGISTER, 0x00); - - // FDC is no more busy - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7)); -} - -Test(ceda_fdc, senseInterruptStatusCommand) { - fdc_init(); - - uint8_t sr; - - fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); - - // This command has no arguments - // FDC should be ready to give response - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); - - // First response byte is SR0 with interrupt code = 0 and Seek End = 1 - sr = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(sr, (1 << 5)); - - // FDC has another byte of response - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); - - // Second response byte is current cylinder, which should be zero at reset - sr = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(sr, 0); -} - -Test(ceda_fdc, seekCommand) { - fdc_init(); - - uint8_t sr; - - fdc_out(ADDR_DATA_REGISTER, SEEK); - - // Now read status register to check that FDC is ready to receive arguments - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 4)); - - // First argument is number of drive - fdc_out(ADDR_DATA_REGISTER, 0x00); - // Second argument is cylinder position - fdc_out(ADDR_DATA_REGISTER, 5); - - // FDC is no more busy - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7)); - - // A sense interrupt command is expected after SEEK - fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); - - // This command has no arguments - // FDC should be ready to give response - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); - - // First response byte is SR0 with interrupt code = 0 and Seek End = 1 - sr = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(sr, (1 << 5)); - - // FDC has another byte of response - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); - - // Second response byte is current cylinder, which should be the one - // specified by the seek argument - sr = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(sr, 5); -} - -#endif diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c new file mode 100644 index 0000000..fd35de9 --- /dev/null +++ b/src/tests/test_fdc.c @@ -0,0 +1,102 @@ + +#include + +// TODO source path is src! +#include "../fdc.h" +#include "../fdc_registers.h" + +Test(ceda_fdc, mainStatusRegisterWhenIdle) { + fdc_init(); + + // Try to read status register and check that it is idle + uint8_t sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7)); +} + +Test(ceda_fdc, specifyCommand) { + fdc_init(); + + uint8_t sr; + + // Try to read status register and check that it is idle + fdc_out(ADDR_DATA_REGISTER, SPECIFY); + + // Now read status register to check that FDC is ready to receive arguments + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 4)); + + // Pass dummy arguments + fdc_out(ADDR_DATA_REGISTER, 0x00); + fdc_out(ADDR_DATA_REGISTER, 0x00); + + // FDC is no more busy + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7)); +} + +Test(ceda_fdc, senseInterruptStatusCommand) { + fdc_init(); + + uint8_t sr; + + fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); + + // This command has no arguments + // FDC should be ready to give response + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + + // First response byte is SR0 with interrupt code = 0 and Seek End = 1 + sr = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(sr, (1 << 5)); + + // FDC has another byte of response + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + + // Second response byte is current cylinder, which should be zero at reset + sr = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(sr, 0); +} + +Test(ceda_fdc, seekCommand) { + fdc_init(); + + uint8_t sr; + + fdc_out(ADDR_DATA_REGISTER, SEEK); + + // Now read status register to check that FDC is ready to receive arguments + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 4)); + + // First argument is number of drive + fdc_out(ADDR_DATA_REGISTER, 0x00); + // Second argument is cylinder position + fdc_out(ADDR_DATA_REGISTER, 5); + + // FDC is no more busy + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7)); + + // A sense interrupt command is expected after SEEK + fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); + + // This command has no arguments + // FDC should be ready to give response + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + + // First response byte is SR0 with interrupt code = 0 and Seek End = 1 + sr = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(sr, (1 << 5)); + + // FDC has another byte of response + sr = fdc_in(ADDR_STATUS_REGISTER); + cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + + // Second response byte is current cylinder, which should be the one + // specified by the seek argument + sr = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(sr, 5); +} From b7df99166af50d9b81578bb44eb71c3230f000c3 Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 16 Jan 2025 20:41:34 +0100 Subject: [PATCH 11/52] refactor: just some renaming --- src/fdc.c | 28 ++++++++++++---------------- src/fdc.h | 9 +++++---- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index caf8131..c15fd6c 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -174,7 +174,7 @@ static uint8_t exec_buffer[1024]; // Result buffer. Each command has maximum 7 bytes as argument. static uint8_t result[7]; static bool tc_status = false; -static bool isReady = false; +static bool is_ready = false; /* FDC internal registers */ // Main Status Register @@ -185,13 +185,8 @@ static main_status_register_t status_register; static uint8_t track[4]; /* Callbacks to handle floppy read and write */ -static int (*read_buffer_cb)(uint8_t *buffer, uint8_t unit_number, - bool phy_head, uint8_t phy_track, bool head, - uint8_t track, uint8_t sector) = NULL; - -static int (*write_buffer_cb)(uint8_t *buffer, uint8_t unit_number, - bool phy_head, uint8_t phy_track, bool head, - uint8_t track, uint8_t sector) = NULL; +static fdc_read_write_t read_buffer_cb = NULL; +static fdc_read_write_t write_buffer_cb = NULL; /* * * * * * * * * * * * * * * Command routines * * * * * * * * * * * * * * */ @@ -368,7 +363,7 @@ static void pre_exec_recalibrate(void) { track[drive] = 0; // We don't have to actually move the head. The drive is immediately ready - isReady = true; + is_ready = true; } // Sense interrupt: @@ -377,7 +372,7 @@ static void post_exec_sense_interrupt(void) { uint8_t drive = 0; // After reading interrupt status, ready can be deasserted - isReady = false; + is_ready = false; LOG_DEBUG("FDC Sense Interrupt\n"); /* Status Register 0 */ @@ -415,7 +410,7 @@ static void pre_exec_seek(void) { LOG_DEBUG("NCN: %d\n", track[drive]); // We don't have to actually move the head. The drive is immediately ready - isReady = true; + is_ready = true; } /* * * * * * * * * * * * * * * Utility routines * * * * * * * * * * * * * * */ @@ -480,7 +475,7 @@ static void buffer_update(void) { uint8_t sector = rw_args->record; // Default: no data ready to be served - isReady = false; + is_ready = false; rwcount_max = 0; // FDC counts sectors from 1 @@ -512,7 +507,7 @@ static void buffer_update(void) { CEDA_STRONG_ASSERT_TRUE(ret > 0); // Ready to serve data - isReady = true; + is_ready = true; } rwcount = 0; @@ -526,7 +521,7 @@ static void buffer_write_size(void) { uint8_t sector = rw_args->record; // Default: no data ready to be served - isReady = false; + is_ready = false; rwcount_max = 0; // FDC counts sectors from 1 @@ -659,7 +654,7 @@ void fdc_tc_out(ceda_ioaddr_t address, uint8_t value) { // (beginning of result phase). When first byte of result phase data // is read, INT=0. bool fdc_getIntStatus(void) { - return isReady; + return is_ready; } // TODO(giuliof): describe better this function @@ -667,7 +662,8 @@ bool fdc_getIntStatus(void) { // a read or write loop that was waiting for interrupt. // In that case, remove the lock (buffer_update will do it, for the writing it // has to be implemented) -void fdc_kickDiskImage(p_rwBuffer read_callback, p_rwBuffer write_callback) { +void fdc_kickDiskImage(fdc_read_write_t read_callback, + fdc_read_write_t write_callback) { read_buffer_cb = read_callback; write_buffer_cb = write_callback; diff --git a/src/fdc.h b/src/fdc.h index 1b20360..36ef2e5 100644 --- a/src/fdc.h +++ b/src/fdc.h @@ -6,9 +6,9 @@ #include #include -typedef int (*p_rwBuffer)(uint8_t *buffer, uint8_t unit_number, bool phy_head, - uint8_t phy_track, bool head, uint8_t track, - uint8_t sector); +typedef int (*fdc_read_write_t)(uint8_t *buffer, uint8_t unit_number, + bool phy_head, uint8_t phy_track, bool head, + uint8_t track, uint8_t sector); void fdc_init(void); @@ -16,6 +16,7 @@ uint8_t fdc_in(ceda_ioaddr_t address); void fdc_out(ceda_ioaddr_t address, uint8_t value); void fdc_tc_out(ceda_ioaddr_t address, uint8_t value); bool fdc_getIntStatus(void); -void fdc_kickDiskImage(p_rwBuffer read_callback, p_rwBuffer write_callback); +void fdc_kickDiskImage(fdc_read_write_t read_callback, + fdc_read_write_t write_callback); #endif // CEDA_FDC_H From 47adc7372566b5c5b1dda386c93fdbc39743364f Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 16 Jan 2025 21:07:41 +0100 Subject: [PATCH 12/52] refactor(fdc): code cleanup and comments in read/write routines --- src/fdc.c | 116 +++++++++++++++++++++++++++++------------------------- 1 file changed, 62 insertions(+), 54 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index c15fd6c..aaaf262 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -235,41 +235,44 @@ static uint8_t exec_write_data(uint8_t value) { exec_buffer[rwcount++] = value; - if (rwcount == rwcount_max) { - uint8_t sector = rw_args->record; - // FDC counts sectors from 1 - assert(sector != 0); - // But all other routines counts sectors from 0 - sector--; - int ret = - write_buffer_cb(exec_buffer, rw_args->unit_select, - rw_args->head_address, track[rw_args->unit_select], - rw_args->head, rw_args->cylinder, sector); - // the image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret > 0); - // Buffer is statically allocated, be sure that the data can fit it - CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); + // More data can be written, just go on with the current buffer + if (rwcount != rwcount_max) + return 0; - // Multi-sector mode (enabled by default). - // If read is not interrupted at the end of the sector, the next logical - // sector is loaded - rw_args->record++; - - // Last sector of the track - if (rw_args->record > rw_args->eot) { - // Multi track mode, if enabled the read operation go on on the next - // side of the same track - if (command_args & CMD_ARGS_MT_bm) { - rw_args->head_address = !rw_args->head_address; - rw_args->head = !rw_args->head; - }; - - // In any case, reached the end of track we start back from sector 1 - rw_args->record = 1; - } + /* Commit the current buffer and prepare the next one to be written */ + uint8_t sector = rw_args->record; + // FDC counts sectors from 1 + CEDA_STRONG_ASSERT_TRUE(sector != 0); + // But all other routines counts sectors from 0 + sector--; + int ret = write_buffer_cb( + exec_buffer, rw_args->unit_select, rw_args->head_address, + track[rw_args->unit_select], rw_args->head, rw_args->cylinder, sector); + // the image is always loaded and valid + CEDA_STRONG_ASSERT_TRUE(ret > 0); + // Buffer is statically allocated, be sure that the data can fit it + CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); - buffer_write_size(); + // Multi-sector mode (enabled by default). + // If read is not interrupted at the end of the sector, the next logical + // sector is loaded + rw_args->record++; + + // Last sector of the track + if (rw_args->record > rw_args->eot) { + // Multi track mode, if enabled the read operation go on on the next + // side of the same track + if (command_args & CMD_ARGS_MT_bm) { + rw_args->head_address = !rw_args->head_address; + rw_args->head = !rw_args->head; + }; + + // In any case, reached the end of track we start back from sector 1 + rw_args->record = 1; } + + buffer_write_size(); + return 0; } @@ -308,6 +311,9 @@ static void pre_exec_read_data(void) { } static uint8_t exec_read_data(uint8_t value) { + // read doesn't care of in value + (void)value; + rw_args_t *rw_args = (rw_args_t *)args; if (rwcount_max == 0) { @@ -315,31 +321,33 @@ static uint8_t exec_read_data(uint8_t value) { return 0; } - // read doesn't care of in value - (void)value; uint8_t ret = exec_buffer[rwcount++]; - if (rwcount == rwcount_max) { - // Multi-sector mode (enabled by default). - // If read is not interrupted at the end of the sector, the next logical - // sector is loaded - rw_args->record++; - - // Last sector of the track - if (rw_args->record > rw_args->eot) { - // Multi track mode, if enabled the read operation go on on the next - // side of the same track - if (command_args & CMD_ARGS_MT_bm) { - rw_args->head_address = !rw_args->head_address; - rw_args->head = !rw_args->head; - }; - - // In any case, reached the end of track we start back from sector 1 - rw_args->record = 1; - } - - buffer_update(); + // More data can be read, just go on with the current buffer + if (rwcount != rwcount_max) + return ret; + + /* Prepare the next buffer to be read */ + // Multi-sector mode (enabled by default). + // If read is not interrupted at the end of the sector, the next logical + // sector is loaded + rw_args->record++; + + // Last sector of the track + if (rw_args->record > rw_args->eot) { + // Multi track mode, if enabled the read operation go on on the next + // side of the same track + if (command_args & CMD_ARGS_MT_bm) { + rw_args->head_address = !rw_args->head_address; + rw_args->head = !rw_args->head; + }; + + // In any case, reached the end of track we start back from sector 1 + rw_args->record = 1; } + + buffer_update(); + return ret; } From 5167e130509d0b694be33b842f22a4a91e678b44 Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 16 Jan 2025 22:43:04 +0100 Subject: [PATCH 13/52] test(fdc): added preliminary reading test Just check if read command can accept arguments and puts the FDC in the right status, even if media is not loaded and then loaded at runtime --- src/tests/test_fdc.c | 74 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index fd35de9..c87cbdd 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -5,6 +5,38 @@ #include "../fdc.h" #include "../fdc_registers.h" +static void assert_fdc_sr(uint8_t expected_sr); +static int fake_read(uint8_t *buffer, uint8_t unit_number, bool phy_head, + uint8_t phy_track, bool head, uint8_t track, + uint8_t sector); + +/** + * @brief Helper function to check the current status of the FDC main status + * register + * + * @param expected_sr the expected value of the FDC main status register + */ +static void assert_fdc_sr(uint8_t expected_sr) { + uint8_t sr; + sr = fdc_in(ADDR_STATUS_REGISTER); + // cr_log_info("%x != %x", sr, expected_sr); + cr_expect_eq(sr, expected_sr); +} + +static int fake_read(uint8_t *buffer, uint8_t unit_number, bool phy_head, + uint8_t phy_track, bool head, uint8_t track, + uint8_t sector) { + (void)buffer; + (void)unit_number; + (void)phy_head; + (void)phy_track; + (void)head; + (void)track; + (void)sector; + + return 4; +} + Test(ceda_fdc, mainStatusRegisterWhenIdle) { fdc_init(); @@ -100,3 +132,45 @@ Test(ceda_fdc, seekCommand) { sr = fdc_in(ADDR_DATA_REGISTER); cr_expect_eq(sr, 5); } + +Test(ceda_fdc, readCommandNoMedium) { + fdc_init(); + + fdc_out(ADDR_DATA_REGISTER, READ_DATA); + + /* Provide the argument, dummy ones! */ + assert_fdc_sr((1 << 7) | (1 << 4)); + // 1st argument is number of drive + fdc_out(ADDR_DATA_REGISTER, 0x00); + assert_fdc_sr((1 << 7) | (1 << 4)); + // 2nd argument is cylinder number + fdc_out(ADDR_DATA_REGISTER, 1); + assert_fdc_sr((1 << 7) | (1 << 4)); + // 3rd argument is head number + fdc_out(ADDR_DATA_REGISTER, 1); + assert_fdc_sr((1 << 7) | (1 << 4)); + // 4th argument is record number + fdc_out(ADDR_DATA_REGISTER, 1); + assert_fdc_sr((1 << 7) | (1 << 4)); + // 5th argument is bytes per sector factor + fdc_out(ADDR_DATA_REGISTER, 0); + assert_fdc_sr((1 << 7) | (1 << 4)); + // 6th argument is EOT + fdc_out(ADDR_DATA_REGISTER, 2); + assert_fdc_sr((1 << 7) | (1 << 4)); + // 7th argument is GPL + fdc_out(ADDR_DATA_REGISTER, 0); + assert_fdc_sr((1 << 7) | (1 << 4)); + // 8th argument is DTL + fdc_out(ADDR_DATA_REGISTER, 4); + + // FDC switches IO mode, but... + assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 5) | (1 << 4)); + // ... is not ready since no medium is loaded + cr_assert_eq(fdc_getIntStatus(), false); + + // Kick medium in... + fdc_kickDiskImage(fake_read, NULL); + // ... now FDC is ready + cr_assert_eq(fdc_getIntStatus(), true); +} From 886e0c350f3ab41ad796a2aceed2bc50f6136465 Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 16 Jan 2025 22:48:51 +0100 Subject: [PATCH 14/52] refactor(fdc): replaced status register read and check with helper function --- src/tests/test_fdc.c | 59 ++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 35 deletions(-) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index c87cbdd..c9df79c 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -41,66 +41,58 @@ Test(ceda_fdc, mainStatusRegisterWhenIdle) { fdc_init(); // Try to read status register and check that it is idle - uint8_t sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7)); + assert_fdc_sr(1 << 7); } Test(ceda_fdc, specifyCommand) { fdc_init(); - uint8_t sr; - // Try to read status register and check that it is idle fdc_out(ADDR_DATA_REGISTER, SPECIFY); // Now read status register to check that FDC is ready to receive arguments - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 4)); + assert_fdc_sr((1 << 7) | (1 << 4)); // Pass dummy arguments fdc_out(ADDR_DATA_REGISTER, 0x00); fdc_out(ADDR_DATA_REGISTER, 0x00); // FDC is no more busy - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7)); + assert_fdc_sr((1 << 7)); } Test(ceda_fdc, senseInterruptStatusCommand) { fdc_init(); - uint8_t sr; + uint8_t data; fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); // This command has no arguments // FDC should be ready to give response - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 4)); // First response byte is SR0 with interrupt code = 0 and Seek End = 1 - sr = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(sr, (1 << 5)); + data = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(data, (1 << 5)); // FDC has another byte of response - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 4)); // Second response byte is current cylinder, which should be zero at reset - sr = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(sr, 0); + data = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(data, 0); } Test(ceda_fdc, seekCommand) { fdc_init(); - uint8_t sr; + uint8_t data; fdc_out(ADDR_DATA_REGISTER, SEEK); // Now read status register to check that FDC is ready to receive arguments - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 4)); + assert_fdc_sr((1 << 7) | (1 << 4)); // First argument is number of drive fdc_out(ADDR_DATA_REGISTER, 0x00); @@ -108,29 +100,26 @@ Test(ceda_fdc, seekCommand) { fdc_out(ADDR_DATA_REGISTER, 5); // FDC is no more busy - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7)); + assert_fdc_sr((1 << 7)); // A sense interrupt command is expected after SEEK fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); // This command has no arguments // FDC should be ready to give response - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 4)); // First response byte is SR0 with interrupt code = 0 and Seek End = 1 - sr = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(sr, (1 << 5)); + data = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(data, (1 << 5)); // FDC has another byte of response - sr = fdc_in(ADDR_STATUS_REGISTER); - cr_expect_eq(sr, (1 << 7) | (1 << 6) | (1 << 4)); + assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 4)); // Second response byte is current cylinder, which should be the one // specified by the seek argument - sr = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(sr, 5); + data = fdc_in(ADDR_DATA_REGISTER); + cr_expect_eq(data, 5); } Test(ceda_fdc, readCommandNoMedium) { @@ -141,28 +130,28 @@ Test(ceda_fdc, readCommandNoMedium) { /* Provide the argument, dummy ones! */ assert_fdc_sr((1 << 7) | (1 << 4)); // 1st argument is number of drive - fdc_out(ADDR_DATA_REGISTER, 0x00); + fdc_out(ADDR_DATA_REGISTER, 0); assert_fdc_sr((1 << 7) | (1 << 4)); // 2nd argument is cylinder number fdc_out(ADDR_DATA_REGISTER, 1); assert_fdc_sr((1 << 7) | (1 << 4)); // 3rd argument is head number - fdc_out(ADDR_DATA_REGISTER, 1); + fdc_out(ADDR_DATA_REGISTER, 0); assert_fdc_sr((1 << 7) | (1 << 4)); // 4th argument is record number fdc_out(ADDR_DATA_REGISTER, 1); assert_fdc_sr((1 << 7) | (1 << 4)); // 5th argument is bytes per sector factor - fdc_out(ADDR_DATA_REGISTER, 0); + fdc_out(ADDR_DATA_REGISTER, 1); assert_fdc_sr((1 << 7) | (1 << 4)); // 6th argument is EOT - fdc_out(ADDR_DATA_REGISTER, 2); + fdc_out(ADDR_DATA_REGISTER, 5); assert_fdc_sr((1 << 7) | (1 << 4)); // 7th argument is GPL fdc_out(ADDR_DATA_REGISTER, 0); assert_fdc_sr((1 << 7) | (1 << 4)); // 8th argument is DTL - fdc_out(ADDR_DATA_REGISTER, 4); + fdc_out(ADDR_DATA_REGISTER, 0); // FDC switches IO mode, but... assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 5) | (1 << 4)); From 9f7ae83254b7e1e88263da6ad6e2f87e2e97db00 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 18 Jan 2025 16:14:38 +0100 Subject: [PATCH 15/52] fix(fdc): corrected initialization --- src/fdc.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/fdc.c b/src/fdc.c index aaaf262..f25ee23 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -559,11 +559,27 @@ static void buffer_write_size(void) { /* * * * * * * * * * * * * * * Public routines * * * * * * * * * * * * * * */ void fdc_init(void) { + // Reset current command status fdc_status = CMD; + fdc_currop = NULL; + + // Reset any internal status rwcount_max = 0; + memset(result, 0, sizeof(result)); + tc_status = false; + is_ready = false; + + // Reset main status register, but keep RQM active since FDC is always ready + // to receive requests status_register.value = 0x00; - // FDC is always ready status_register.rqm = 1; + + // Reset track positions + memset(track, 0, sizeof(track)); + + // Detach any read/write callback + read_buffer_cb = NULL; + write_buffer_cb = NULL; } uint8_t fdc_in(ceda_ioaddr_t address) { From d5c5e0f3957ef7f64b7457afca9e21c162c0bc9f Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 18 Jan 2025 17:43:40 +0100 Subject: [PATCH 16/52] refactor(fdc): replaced magic number with defined constants in tests TODO: remove bitfields and use these constants --- src/fdc_registers.h | 54 ++++++++++++++++++++++++++++++++++++++++++++ src/tests/test_fdc.c | 40 ++++++++++++++++---------------- 2 files changed, 74 insertions(+), 20 deletions(-) diff --git a/src/fdc_registers.h b/src/fdc_registers.h index 44d0bcf..1718972 100644 --- a/src/fdc_registers.h +++ b/src/fdc_registers.h @@ -33,4 +33,58 @@ typedef enum cmd_t { #define CMD_ARGS_MF_bm (0x40) #define CMD_ARGS_SK_bm (0x20) +/* Main status register bitfield */ +// Request From Master, set if FDC is ready to receive or send data +#define FDC_ST_RQM (1 << 7) +// Data I/O, set if FDC is read from CPU, clear otherwise +#define FDC_ST_DIO (1 << 6) +// Execution mode +#define FDC_ST_EXM (1 << 5) +// Controller has already accepted a command +#define FDC_ST_CB (1 << 4) +// Drive x is in Seek mode +#define FDC_ST_D3B (1 << 3) +#define FDC_ST_D2B (1 << 2) +#define FDC_ST_D1B (1 << 1) +#define FDC_ST_D0B (1 << 0) + +/* Status Register 0 bitfield */ +#define FDC_ST0_IC (0xC0) +// seek end +#define FDC_ST0_SE (1 << 5) +// equipment check: fault signal from FDD or recalibrate failure +#define FDC_ST0_EC (1 << 4) +// not ready +#define FDC_ST0_NR (1 << 3) +// state of the head +#define FDC_ST0_HD (1 << 2) +// drive unit number at interrupt +#define FDC_ST0_US (0x03) + +/* Status Register 1 bitfield */ +#define FDC_ST1_EN (1 << 7) +#define FDC_ST1_DE (1 << 5) +#define FDC_ST1_OR (1 << 4) +#define FDC_ST1_ND (1 << 2) +#define FDC_ST1_NW (1 << 1) +#define FDC_ST1_NA (1 << 0) + +/* Status Register 2 bitfield */ +#define FDC_ST2_CM (1 << 6) +#define FDC_ST2_DD (1 << 5) +#define FDC_ST2_WC (1 << 4) +#define FDC_ST2_SH (1 << 3) +#define FDC_ST2_SN (1 << 2) +#define FDC_ST2_BC (1 << 1) +#define FDC_ST2_MD (1 << 0) + +/* Status Register 3 bitfield */ +#define FDC_ST3_FT (1 << 7) +#define FDC_ST3_WP (1 << 6) +#define FDC_ST3_RY (1 << 5) +#define FDC_ST3_T0 (1 << 4) +#define FDC_ST3_TS (1 << 3) +#define FDC_ST3_HD (1 << 2) +#define FDC_ST3_US (0x03) + #endif // CEDA_FDC_REGISTERS_H_ \ No newline at end of file diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index c9df79c..3cc4a17 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -41,7 +41,7 @@ Test(ceda_fdc, mainStatusRegisterWhenIdle) { fdc_init(); // Try to read status register and check that it is idle - assert_fdc_sr(1 << 7); + assert_fdc_sr(FDC_ST_RQM); } Test(ceda_fdc, specifyCommand) { @@ -51,14 +51,14 @@ Test(ceda_fdc, specifyCommand) { fdc_out(ADDR_DATA_REGISTER, SPECIFY); // Now read status register to check that FDC is ready to receive arguments - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // Pass dummy arguments fdc_out(ADDR_DATA_REGISTER, 0x00); fdc_out(ADDR_DATA_REGISTER, 0x00); // FDC is no more busy - assert_fdc_sr((1 << 7)); + assert_fdc_sr(FDC_ST_RQM); } Test(ceda_fdc, senseInterruptStatusCommand) { @@ -70,14 +70,14 @@ Test(ceda_fdc, senseInterruptStatusCommand) { // This command has no arguments // FDC should be ready to give response - assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); // First response byte is SR0 with interrupt code = 0 and Seek End = 1 data = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(data, (1 << 5)); + cr_expect_eq(data, FDC_ST0_SE); // FDC has another byte of response - assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); // Second response byte is current cylinder, which should be zero at reset data = fdc_in(ADDR_DATA_REGISTER); @@ -92,7 +92,7 @@ Test(ceda_fdc, seekCommand) { fdc_out(ADDR_DATA_REGISTER, SEEK); // Now read status register to check that FDC is ready to receive arguments - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // First argument is number of drive fdc_out(ADDR_DATA_REGISTER, 0x00); @@ -100,21 +100,21 @@ Test(ceda_fdc, seekCommand) { fdc_out(ADDR_DATA_REGISTER, 5); // FDC is no more busy - assert_fdc_sr((1 << 7)); + assert_fdc_sr(FDC_ST_RQM); // A sense interrupt command is expected after SEEK fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); // This command has no arguments // FDC should be ready to give response - assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); // First response byte is SR0 with interrupt code = 0 and Seek End = 1 data = fdc_in(ADDR_DATA_REGISTER); - cr_expect_eq(data, (1 << 5)); + cr_expect_eq(data, FDC_ST0_SE); // FDC has another byte of response - assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); // Second response byte is current cylinder, which should be the one // specified by the seek argument @@ -128,33 +128,33 @@ Test(ceda_fdc, readCommandNoMedium) { fdc_out(ADDR_DATA_REGISTER, READ_DATA); /* Provide the argument, dummy ones! */ - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 1st argument is number of drive fdc_out(ADDR_DATA_REGISTER, 0); - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 2nd argument is cylinder number fdc_out(ADDR_DATA_REGISTER, 1); - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 3rd argument is head number fdc_out(ADDR_DATA_REGISTER, 0); - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 4th argument is record number fdc_out(ADDR_DATA_REGISTER, 1); - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 5th argument is bytes per sector factor fdc_out(ADDR_DATA_REGISTER, 1); - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 6th argument is EOT fdc_out(ADDR_DATA_REGISTER, 5); - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 7th argument is GPL fdc_out(ADDR_DATA_REGISTER, 0); - assert_fdc_sr((1 << 7) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 8th argument is DTL fdc_out(ADDR_DATA_REGISTER, 0); // FDC switches IO mode, but... - assert_fdc_sr((1 << 7) | (1 << 6) | (1 << 5) | (1 << 4)); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); // ... is not ready since no medium is loaded cr_assert_eq(fdc_getIntStatus(), false); From 388a0e89886a0ed64df3f3896207a32275e4ac42 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 18 Jan 2025 18:00:46 +0100 Subject: [PATCH 17/52] refactor(fdc): changed names of exposed constants --- src/fdc.c | 42 +++++++++++++++++++------------------- src/fdc_registers.h | 48 ++++++++++++++++++++++---------------------- src/tests/test_fdc.c | 46 +++++++++++++++++++++--------------------- 3 files changed, 68 insertions(+), 68 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index f25ee23..ba70398 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -25,7 +25,7 @@ typedef enum fdc_status_t { CMD, ARGS, EXEC, RESULT } fdc_status_t; // One may provide callbacks for exec preparation, actual execution and // post execution. typedef struct fdc_operation_t { - cmd_t cmd; + fdc_cmd_t cmd; // An FDC operation is splitted into 4 steps. // If a step len is 0, that step must be skipped. size_t args_len; @@ -99,7 +99,7 @@ static void buffer_write_size(void); // The command descriptors static const fdc_operation_t fdc_operations[] = { { - .cmd = SPECIFY, + .cmd = FDC_SPECIFY, .args_len = 2, .result_len = 0, .pre_exec = pre_exec_specify, @@ -107,7 +107,7 @@ static const fdc_operation_t fdc_operations[] = { .post_exec = NULL, }, { - .cmd = WRITE_DATA, + .cmd = FDC_WRITE_DATA, .args_len = 8, .result_len = 7, .pre_exec = pre_exec_write_data, @@ -115,7 +115,7 @@ static const fdc_operation_t fdc_operations[] = { .post_exec = post_exec_write_data, }, { - .cmd = READ_DATA, + .cmd = FDC_READ_DATA, .args_len = 8, .result_len = 7, .pre_exec = pre_exec_read_data, @@ -123,7 +123,7 @@ static const fdc_operation_t fdc_operations[] = { .post_exec = post_exec_read_data, }, { - .cmd = RECALIBRATE, + .cmd = FDC_RECALIBRATE, .args_len = 1, .result_len = 0, .pre_exec = pre_exec_recalibrate, @@ -131,7 +131,7 @@ static const fdc_operation_t fdc_operations[] = { .post_exec = NULL, }, { - .cmd = SENSE_INTERRUPT, + .cmd = FDC_SENSE_INTERRUPT, .args_len = 0, .result_len = 2, .pre_exec = NULL, @@ -139,7 +139,7 @@ static const fdc_operation_t fdc_operations[] = { .post_exec = post_exec_sense_interrupt, }, { - .cmd = FORMAT_TRACK, + .cmd = FDC_FORMAT_TRACK, .args_len = 5, .result_len = 7, .pre_exec = pre_exec_format_track, @@ -147,7 +147,7 @@ static const fdc_operation_t fdc_operations[] = { .post_exec = post_exec_format_track, }, { - .cmd = SEEK, + .cmd = FDC_SEEK, .args_len = 2, .result_len = 0, .pre_exec = pre_exec_seek, @@ -262,10 +262,10 @@ static uint8_t exec_write_data(uint8_t value) { if (rw_args->record > rw_args->eot) { // Multi track mode, if enabled the read operation go on on the next // side of the same track - if (command_args & CMD_ARGS_MT_bm) { + if (command_args & FDC_CMD_ARGS_MT_bm) { rw_args->head_address = !rw_args->head_address; rw_args->head = !rw_args->head; - }; + } // In any case, reached the end of track we start back from sector 1 rw_args->record = 1; @@ -337,10 +337,10 @@ static uint8_t exec_read_data(uint8_t value) { if (rw_args->record > rw_args->eot) { // Multi track mode, if enabled the read operation go on on the next // side of the same track - if (command_args & CMD_ARGS_MT_bm) { + if (command_args & FDC_CMD_ARGS_MT_bm) { rw_args->head_address = !rw_args->head_address; rw_args->head = !rw_args->head; - }; + } // In any case, reached the end of track we start back from sector 1 rw_args->record = 1; @@ -389,7 +389,7 @@ static void post_exec_sense_interrupt(void) { // head address (last addressed) - TODO(giulio) // result[0] |= ...; // Seek End - TODO(giulio) - result[0] |= 1U << 5; + result[0] |= FDC_ST0_SE; /* PCN - (current track position) */ result[1] = track[drive]; } @@ -584,9 +584,9 @@ void fdc_init(void) { uint8_t fdc_in(ceda_ioaddr_t address) { switch (address & 0x01) { - case ADDR_STATUS_REGISTER: + case FDC_ADDR_STATUS_REGISTER: return status_register.value; - case ADDR_DATA_REGISTER: { + case FDC_ADDR_DATA_REGISTER: { uint8_t value = 0; if (fdc_status == CMD) { @@ -619,14 +619,14 @@ uint8_t fdc_in(ceda_ioaddr_t address) { void fdc_out(ceda_ioaddr_t address, uint8_t value) { switch (address & 0x01) { - case ADDR_STATUS_REGISTER: + case FDC_ADDR_STATUS_REGISTER: LOG_WARN("nobody should write in FDC main status register\n"); return; - case ADDR_DATA_REGISTER: { + case FDC_ADDR_DATA_REGISTER: { if (fdc_status == CMD) { // Split the command itself from option bits - uint8_t cmd = value & CMD_COMMAND_bm; - command_args = value & CMD_ARGS_bm; + uint8_t cmd = value & FDC_CMD_COMMAND_bm; + command_args = value & FDC_CMD_ARGS_bm; // Unroll the command list and place it in the current execution fdc_currop = NULL; @@ -691,11 +691,11 @@ void fdc_kickDiskImage(fdc_read_write_t read_callback, read_buffer_cb = read_callback; write_buffer_cb = write_callback; - if (fdc_status == EXEC && fdc_currop->cmd == READ_DATA) { + if (fdc_status == EXEC && fdc_currop->cmd == FDC_READ_DATA) { buffer_update(); } - if (fdc_status == EXEC && fdc_currop->cmd == WRITE_DATA) { + if (fdc_status == EXEC && fdc_currop->cmd == FDC_WRITE_DATA) { buffer_write_size(); } } diff --git a/src/fdc_registers.h b/src/fdc_registers.h index 1718972..bce7d7c 100644 --- a/src/fdc_registers.h +++ b/src/fdc_registers.h @@ -2,36 +2,36 @@ #define CEDA_FDC_REGISTERS_H_ // The FDC virtually expose two registers, which can be both read or written -#define ADDR_STATUS_REGISTER (0x00) -#define ADDR_DATA_REGISTER (0x01) +#define FDC_ADDR_STATUS_REGISTER (0x00) +#define FDC_ADDR_DATA_REGISTER (0x01) // The FDC can perform 15 different commands. // Don't ask me why they didn't use a single nibble to represent all the // commands. Please refer to the datasheet for more information. -typedef enum cmd_t { - READ_TRACK = 0x02, - SPECIFY = 0x03, - SENSE_DRIVE = 0x04, - WRITE_DATA = 0x05, - READ_DATA = 0x06, - RECALIBRATE = 0x07, - SENSE_INTERRUPT = 0x08, - WRITE_DELETED_DATA = 0x09, - READ_ID = 0x0A, - READ_DELETED_DATA = 0x0C, - FORMAT_TRACK = 0x0D, - SEEK = 0x0F, - SCAN_EQUAL = 0x11, - SCAN_LOW_EQUAL = 0x19, - SCAN_HIGH_EQUAL = 0x1D -} cmd_t; +typedef enum fdc_cmd_t { + FDC_READ_TRACK = 0x02, + FDC_SPECIFY = 0x03, + FDC_SENSE_DRIVE = 0x04, + FDC_WRITE_DATA = 0x05, + FDC_READ_DATA = 0x06, + FDC_RECALIBRATE = 0x07, + FDC_SENSE_INTERRUPT = 0x08, + FDC_WRITE_DELETED_DATA = 0x09, + FDC_READ_ID = 0x0A, + FDC_READ_DELETED_DATA = 0x0C, + FDC_FORMAT_TRACK = 0x0D, + FDC_SEEK = 0x0F, + FDC_SCAN_EQUAL = 0x11, + FDC_SCAN_LOW_EQUAL = 0x19, + FDC_SCAN_HIGH_EQUAL = 0x1D +} fdc_cmd_t; // Some commands carry argument bits in MSb -#define CMD_COMMAND_bm (0x1F) -#define CMD_ARGS_bm (0xE0) -#define CMD_ARGS_MT_bm (0x80) -#define CMD_ARGS_MF_bm (0x40) -#define CMD_ARGS_SK_bm (0x20) +#define FDC_CMD_COMMAND_bm (0x1F) +#define FDC_CMD_ARGS_bm (0xE0) +#define FDC_CMD_ARGS_MT_bm (0x80) +#define FDC_CMD_ARGS_MF_bm (0x40) +#define FDC_CMD_ARGS_SK_bm (0x20) /* Main status register bitfield */ // Request From Master, set if FDC is ready to receive or send data diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 3cc4a17..9d50f36 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -18,7 +18,7 @@ static int fake_read(uint8_t *buffer, uint8_t unit_number, bool phy_head, */ static void assert_fdc_sr(uint8_t expected_sr) { uint8_t sr; - sr = fdc_in(ADDR_STATUS_REGISTER); + sr = fdc_in(FDC_ADDR_STATUS_REGISTER); // cr_log_info("%x != %x", sr, expected_sr); cr_expect_eq(sr, expected_sr); } @@ -48,14 +48,14 @@ Test(ceda_fdc, specifyCommand) { fdc_init(); // Try to read status register and check that it is idle - fdc_out(ADDR_DATA_REGISTER, SPECIFY); + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_SPECIFY); // Now read status register to check that FDC is ready to receive arguments assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // Pass dummy arguments - fdc_out(ADDR_DATA_REGISTER, 0x00); - fdc_out(ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); // FDC is no more busy assert_fdc_sr(FDC_ST_RQM); @@ -66,21 +66,21 @@ Test(ceda_fdc, senseInterruptStatusCommand) { uint8_t data; - fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_SENSE_INTERRUPT); // This command has no arguments // FDC should be ready to give response assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); // First response byte is SR0 with interrupt code = 0 and Seek End = 1 - data = fdc_in(ADDR_DATA_REGISTER); + data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, FDC_ST0_SE); // FDC has another byte of response assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); // Second response byte is current cylinder, which should be zero at reset - data = fdc_in(ADDR_DATA_REGISTER); + data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, 0); } @@ -89,28 +89,28 @@ Test(ceda_fdc, seekCommand) { uint8_t data; - fdc_out(ADDR_DATA_REGISTER, SEEK); + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_SEEK); // Now read status register to check that FDC is ready to receive arguments assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // First argument is number of drive - fdc_out(ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); // Second argument is cylinder position - fdc_out(ADDR_DATA_REGISTER, 5); + fdc_out(FDC_ADDR_DATA_REGISTER, 5); // FDC is no more busy assert_fdc_sr(FDC_ST_RQM); - // A sense interrupt command is expected after SEEK - fdc_out(ADDR_DATA_REGISTER, SENSE_INTERRUPT); + // A sense interrupt command is expected after FDC_SEEK + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_SENSE_INTERRUPT); // This command has no arguments // FDC should be ready to give response assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); // First response byte is SR0 with interrupt code = 0 and Seek End = 1 - data = fdc_in(ADDR_DATA_REGISTER); + data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, FDC_ST0_SE); // FDC has another byte of response @@ -118,40 +118,40 @@ Test(ceda_fdc, seekCommand) { // Second response byte is current cylinder, which should be the one // specified by the seek argument - data = fdc_in(ADDR_DATA_REGISTER); + data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, 5); } Test(ceda_fdc, readCommandNoMedium) { fdc_init(); - fdc_out(ADDR_DATA_REGISTER, READ_DATA); + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_READ_DATA); /* Provide the argument, dummy ones! */ assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 1st argument is number of drive - fdc_out(ADDR_DATA_REGISTER, 0); + fdc_out(FDC_ADDR_DATA_REGISTER, 0); assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 2nd argument is cylinder number - fdc_out(ADDR_DATA_REGISTER, 1); + fdc_out(FDC_ADDR_DATA_REGISTER, 1); assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 3rd argument is head number - fdc_out(ADDR_DATA_REGISTER, 0); + fdc_out(FDC_ADDR_DATA_REGISTER, 0); assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 4th argument is record number - fdc_out(ADDR_DATA_REGISTER, 1); + fdc_out(FDC_ADDR_DATA_REGISTER, 1); assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 5th argument is bytes per sector factor - fdc_out(ADDR_DATA_REGISTER, 1); + fdc_out(FDC_ADDR_DATA_REGISTER, 1); assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 6th argument is EOT - fdc_out(ADDR_DATA_REGISTER, 5); + fdc_out(FDC_ADDR_DATA_REGISTER, 5); assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 7th argument is GPL - fdc_out(ADDR_DATA_REGISTER, 0); + fdc_out(FDC_ADDR_DATA_REGISTER, 0); assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // 8th argument is DTL - fdc_out(ADDR_DATA_REGISTER, 0); + fdc_out(FDC_ADDR_DATA_REGISTER, 0); // FDC switches IO mode, but... assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); From f9a6513115abaf3cd022cbf63a12ae302e295f86 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 18 Jan 2025 18:33:09 +0100 Subject: [PATCH 18/52] test(fdc): implemented reading test --- src/tests/test_fdc.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 9d50f36..234d6ff 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -163,3 +163,63 @@ Test(ceda_fdc, readCommandNoMedium) { // ... now FDC is ready cr_assert_eq(fdc_getIntStatus(), true); } + +Test(ceda_fdc, readCommand) { + uint8_t result[7]; + + fdc_init(); + + // Link a fake reading function. + // TODO(giuliof): mocking may be better here + fdc_kickDiskImage(fake_read, NULL); + + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_READ_DATA); + + // 1st argument is number of drive + fdc_out(FDC_ADDR_DATA_REGISTER, 0); + // 2nd argument is cylinder number + fdc_out(FDC_ADDR_DATA_REGISTER, 1); + // 3rd argument is head number + fdc_out(FDC_ADDR_DATA_REGISTER, 1); + // 4th argument is record number + fdc_out(FDC_ADDR_DATA_REGISTER, 1); + // 5th argument is bytes per sector factor + fdc_out(FDC_ADDR_DATA_REGISTER, 1); + // 6th argument is EOT + fdc_out(FDC_ADDR_DATA_REGISTER, 2); + // 7th argument is GPL + fdc_out(FDC_ADDR_DATA_REGISTER, 0); + // 8th argument is DTL + fdc_out(FDC_ADDR_DATA_REGISTER, 0); + + // FDC is in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); + + // Read four times + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); + + // FDC is still in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); + + // Stop the reading + fdc_tc_out(0, 0); + + // Execution is finished, but still busy waiting for read of results + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); + + result[0] = fdc_in(FDC_ADDR_DATA_REGISTER); + result[1] = fdc_in(FDC_ADDR_DATA_REGISTER); + result[2] = fdc_in(FDC_ADDR_DATA_REGISTER); + result[3] = fdc_in(FDC_ADDR_DATA_REGISTER); + result[4] = fdc_in(FDC_ADDR_DATA_REGISTER); + result[5] = fdc_in(FDC_ADDR_DATA_REGISTER); + result[6] = fdc_in(FDC_ADDR_DATA_REGISTER); + + // Execution is finished + assert_fdc_sr(FDC_ST_RQM); + + // TODO: check te content of result (currently not implemented) +} From 3b69d18a8e1f9c9aa94a2ed1cea64e2587a99124 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 18 Jan 2025 19:16:28 +0100 Subject: [PATCH 19/52] fix(fdc): implemented preliminary response for reading --- src/fdc.c | 16 ++++++++++++++++ src/tests/test_fdc.c | 17 ++++++++++++----- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index ba70398..aa0e880 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -352,12 +352,28 @@ static uint8_t exec_read_data(uint8_t value) { } static void post_exec_read_data(void) { + rw_args_t *rw_args = (rw_args_t *)args; + LOG_DEBUG("Read has ended\n"); // TODO(giulio): populate result (which is pretty the same for read, // write, ...) memset(result, 0x00, sizeof(result)); // TODO(giuliof): populate result as in datasheet (see table 2) + /* ST0 */ + // Current head position + result[0] |= rw_args->unit_select; + result[0] |= rw_args->head_address ? FDC_ST0_HD : 0; + /* ST1 */ + result[1] |= 0; // TODO(giuliof): populate this + /* ST2 */ + result[2] |= 0; // TODO(giuliof): populate this + /* CHR */ + result[3] = rw_args->cylinder; + result[4] = rw_args->head; + result[5] = rw_args->record; + /* Sector size factor */ + result[6] = rw_args->n; } // Recalibrate: diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 234d6ff..fbc4b63 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -176,17 +176,17 @@ Test(ceda_fdc, readCommand) { fdc_out(FDC_ADDR_DATA_REGISTER, FDC_READ_DATA); // 1st argument is number of drive - fdc_out(FDC_ADDR_DATA_REGISTER, 0); + fdc_out(FDC_ADDR_DATA_REGISTER, 2 | 1 << 2); // 2nd argument is cylinder number - fdc_out(FDC_ADDR_DATA_REGISTER, 1); + fdc_out(FDC_ADDR_DATA_REGISTER, 7); // 3rd argument is head number fdc_out(FDC_ADDR_DATA_REGISTER, 1); // 4th argument is record number - fdc_out(FDC_ADDR_DATA_REGISTER, 1); + fdc_out(FDC_ADDR_DATA_REGISTER, 5); // 5th argument is bytes per sector factor fdc_out(FDC_ADDR_DATA_REGISTER, 1); // 6th argument is EOT - fdc_out(FDC_ADDR_DATA_REGISTER, 2); + fdc_out(FDC_ADDR_DATA_REGISTER, 10); // 7th argument is GPL fdc_out(FDC_ADDR_DATA_REGISTER, 0); // 8th argument is DTL @@ -221,5 +221,12 @@ Test(ceda_fdc, readCommand) { // Execution is finished assert_fdc_sr(FDC_ST_RQM); - // TODO: check te content of result (currently not implemented) + // check the content of result + cr_expect_eq(result[0], FDC_ST0_HD | 2); + cr_expect_eq(result[1], 0); + cr_expect_eq(result[2], 0); + cr_expect_eq(result[3], 7); + cr_expect_eq(result[4], 1); + cr_expect_eq(result[5], 6); + cr_expect_eq(result[6], 1); } From 52447e3f39773317bf5a3a93622fbdb88edc56ef Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 23 Jan 2025 20:53:10 +0100 Subject: [PATCH 20/52] fix(fdc): proper read/write response generation --- src/fdc.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index aa0e880..49fe2ef 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -261,10 +261,15 @@ static uint8_t exec_write_data(uint8_t value) { // Last sector of the track if (rw_args->record > rw_args->eot) { // Multi track mode, if enabled the read operation go on on the next - // side of the same track + // side if (command_args & FDC_CMD_ARGS_MT_bm) { rw_args->head_address = !rw_args->head_address; rw_args->head = !rw_args->head; + + if (!rw_args->head_address) + rw_args->cylinder++; + } else { + rw_args->cylinder++; } // In any case, reached the end of track we start back from sector 1 @@ -336,10 +341,15 @@ static uint8_t exec_read_data(uint8_t value) { // Last sector of the track if (rw_args->record > rw_args->eot) { // Multi track mode, if enabled the read operation go on on the next - // side of the same track + // side if (command_args & FDC_CMD_ARGS_MT_bm) { rw_args->head_address = !rw_args->head_address; rw_args->head = !rw_args->head; + + if (!rw_args->head_address) + rw_args->cylinder++; + } else { + rw_args->cylinder++; } // In any case, reached the end of track we start back from sector 1 From d2a29487e3874b4f74252c19d8c70023814b5ccd Mon Sep 17 00:00:00 2001 From: giuliof Date: Wed, 22 Jan 2025 23:15:33 +0100 Subject: [PATCH 21/52] test(fdc): added a more complete reading test Refactored the test with no medium to be more readable --- src/tests/test_fdc.c | 322 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 258 insertions(+), 64 deletions(-) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index fbc4b63..0167a17 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -1,5 +1,6 @@ #include +#include // TODO source path is src! #include "../fdc.h" @@ -122,36 +123,46 @@ Test(ceda_fdc, seekCommand) { cr_expect_eq(data, 5); } +/** + * @brief Auxiliary function, send a data buffer to the FDC checking that it is + * in input mode for each byte + */ +static void sendBuffer(const uint8_t *buffer, size_t size) { + while (size-- > 0) { + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); + fdc_out(FDC_ADDR_DATA_REGISTER, *(buffer++)); + } +} + +/** + * @brief Auxiliary function, receive a data buffer from the FDC checking that + * it is in output mode for each byte + */ +static void receiveBuffer(uint8_t *buffer, size_t size) { + while (size-- > 0) { + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); + *(buffer++) = fdc_in(FDC_ADDR_DATA_REGISTER); + } +} + Test(ceda_fdc, readCommandNoMedium) { + uint8_t arguments[8] = { + 0, // drive number + 1, // cylinder + 0, // head + 1, // record + 1, // N - bytes per sector size factor + 5, // EOT (end of track) + 0, // GPL (ignored) + 0, // DTL (ignored) + }; + fdc_init(); fdc_out(FDC_ADDR_DATA_REGISTER, FDC_READ_DATA); - /* Provide the argument, dummy ones! */ - assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); - // 1st argument is number of drive - fdc_out(FDC_ADDR_DATA_REGISTER, 0); - assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); - // 2nd argument is cylinder number - fdc_out(FDC_ADDR_DATA_REGISTER, 1); - assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); - // 3rd argument is head number - fdc_out(FDC_ADDR_DATA_REGISTER, 0); - assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); - // 4th argument is record number - fdc_out(FDC_ADDR_DATA_REGISTER, 1); - assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); - // 5th argument is bytes per sector factor - fdc_out(FDC_ADDR_DATA_REGISTER, 1); - assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); - // 6th argument is EOT - fdc_out(FDC_ADDR_DATA_REGISTER, 5); - assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); - // 7th argument is GPL - fdc_out(FDC_ADDR_DATA_REGISTER, 0); - assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); - // 8th argument is DTL - fdc_out(FDC_ADDR_DATA_REGISTER, 0); + // Send arguments checking for no error + sendBuffer(arguments, sizeof(arguments)); // FDC switches IO mode, but... assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); @@ -164,38 +175,237 @@ Test(ceda_fdc, readCommandNoMedium) { cr_assert_eq(fdc_getIntStatus(), true); } -Test(ceda_fdc, readCommand) { +/** + * @brief This section covers the cases described in table 2-2 of xxxxxxx + * datasheet. + * Please note that the FDC should accept logical head value different than + * physical one. + */ + +struct rw_test_params_t { + uint8_t cmd_alteration; + uint8_t arguments[8]; uint8_t result[7]; +}; + +ParameterizedTestParameters(ceda_fdc, readCommand0) { + static struct rw_test_params_t params[] = { + { + // No MT, end record < EOT, physical head 0 + 0, // No alteration + { + 0, // drive number + 7, // cylinder + 0, // head + 5, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL (ignored) + 0, // DTL (ignored) + }, + { + 0, // Drive number, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 7, // record + 1, // N + }, + }, + { + // No MT, end record = EOT, physical head 0 + 0, + { + 1, // drive number + 7, // cylinder + 1, // head - different from physical head just for fun + 9, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + 1, // drive number, no error + 0, // no error + 0, // no error + 8, // cylinder + 1, // head + 1, // record + 1, // N + }, + }, + { + // No MT, end record < EOT, physical head 1 + 0, + { + FDC_ST0_HD | 2, // Drive number, physical head 1 + 7, // cylinder + 0, // head - different from physical head just for fun + 5, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + FDC_ST0_HD | 2, // drive number, physical head 1, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 7, // record + 1, // N + }, + }, + { + // No MT, end record = EOT, physical head 1 + 0, + { + FDC_ST0_HD | 3, // Drive number, physical head 1 + 7, // cylinder + 1, // head + 9, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + FDC_ST0_HD | 3, // drive number, physical head 1, no error + 0, // no error + 0, // no error + 8, // cylinder + 1, // head + 1, // record + 1, // N + }, + }, + /* * * * * * */ + { + // MT (multi-track), end record < EOT, physical head 0 + FDC_CMD_ARGS_MT_bm, + { + 3, // Drive number + 7, // cylinder + 0, // head + 5, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + 3, // drive number, physical head 0, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 7, // record + 1, // N + }, + }, + { + // MT (multi-track), end record = EOT, physical head 0 + FDC_CMD_ARGS_MT_bm, + { + 2, // Drive number + 7, // cylinder + 1, // head - different from physical head just for fun + 9, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + FDC_ST0_HD | 2, // drive number, physical head 1, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 1, // record + 1, // N + }, + }, + { + // MT (multi-track), end record < EOT, physical head 1 + FDC_CMD_ARGS_MT_bm, + { + FDC_ST0_HD | 1, // Drive number, physical head 1 + 7, // cylinder + 0, // head + 5, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + FDC_ST0_HD | 1, // drive number, physical head 1, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 7, // record + 1, // N + }, + }, + { + // MT (multi-track), end record = EOT, physical head 1 + FDC_CMD_ARGS_MT_bm, + { + FDC_ST0_HD | 0, // Drive number, physical head 1 + 7, // cylinder + 0, // head - different from physical head just for fun + 9, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + 0, // drive number, physical head 0, no error + 0, // no error + 0, // no error + 8, // cylinder + 1, // head + 1, // record + 1, // N + }, + }, + }; + + size_t nb_params = sizeof(params) / sizeof(struct rw_test_params_t); + return cr_make_param_array(struct rw_test_params_t, params, nb_params); +} + +ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, readCommand0) { + uint8_t result[sizeof(param->result)]; fdc_init(); - // Link a fake reading function. - // TODO(giuliof): mocking may be better here + // Link a fake reading function fdc_kickDiskImage(fake_read, NULL); - fdc_out(FDC_ADDR_DATA_REGISTER, FDC_READ_DATA); + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_READ_DATA | param->cmd_alteration); - // 1st argument is number of drive - fdc_out(FDC_ADDR_DATA_REGISTER, 2 | 1 << 2); - // 2nd argument is cylinder number - fdc_out(FDC_ADDR_DATA_REGISTER, 7); - // 3rd argument is head number - fdc_out(FDC_ADDR_DATA_REGISTER, 1); - // 4th argument is record number - fdc_out(FDC_ADDR_DATA_REGISTER, 5); - // 5th argument is bytes per sector factor - fdc_out(FDC_ADDR_DATA_REGISTER, 1); - // 6th argument is EOT - fdc_out(FDC_ADDR_DATA_REGISTER, 10); - // 7th argument is GPL - fdc_out(FDC_ADDR_DATA_REGISTER, 0); - // 8th argument is DTL - fdc_out(FDC_ADDR_DATA_REGISTER, 0); + // Send arguments checking for no error + sendBuffer(param->arguments, sizeof(param->arguments)); // FDC is in execution mode assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); - // Read four times + // FDC is ready to serve data + cr_assert_eq(fdc_getIntStatus(), true); + + // Read two full sectors + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); fdc_in(FDC_ADDR_DATA_REGISTER); fdc_in(FDC_ADDR_DATA_REGISTER); @@ -207,26 +417,10 @@ Test(ceda_fdc, readCommand) { // Stop the reading fdc_tc_out(0, 0); - // Execution is finished, but still busy waiting for read of results - assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); + receiveBuffer(result, sizeof(result)); - result[0] = fdc_in(FDC_ADDR_DATA_REGISTER); - result[1] = fdc_in(FDC_ADDR_DATA_REGISTER); - result[2] = fdc_in(FDC_ADDR_DATA_REGISTER); - result[3] = fdc_in(FDC_ADDR_DATA_REGISTER); - result[4] = fdc_in(FDC_ADDR_DATA_REGISTER); - result[5] = fdc_in(FDC_ADDR_DATA_REGISTER); - result[6] = fdc_in(FDC_ADDR_DATA_REGISTER); + cr_assert_arr_eq(result, param->result, sizeof(result)); // Execution is finished assert_fdc_sr(FDC_ST_RQM); - - // check the content of result - cr_expect_eq(result[0], FDC_ST0_HD | 2); - cr_expect_eq(result[1], 0); - cr_expect_eq(result[2], 0); - cr_expect_eq(result[3], 7); - cr_expect_eq(result[4], 1); - cr_expect_eq(result[5], 6); - cr_expect_eq(result[6], 1); } From c1a94f3a98b76f42106b27b1e0fa06bc3af0ea4a Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 23 Jan 2025 21:21:39 +0100 Subject: [PATCH 22/52] feat(fdc): introduced deleted data commands At the moment we don't have an image support that can distingush between deleted and not deleted data, so they can be handled by the same routines. --- src/fdc.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/fdc.c b/src/fdc.c index 49fe2ef..b319773 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -138,6 +138,22 @@ static const fdc_operation_t fdc_operations[] = { .exec = NULL, .post_exec = post_exec_sense_interrupt, }, + { + .cmd = FDC_WRITE_DELETED_DATA, + .args_len = 8, + .result_len = 7, + .pre_exec = pre_exec_write_data, + .exec = exec_write_data, + .post_exec = post_exec_write_data, + }, + { + .cmd = FDC_READ_DELETED_DATA, + .args_len = 8, + .result_len = 7, + .pre_exec = pre_exec_read_data, + .exec = exec_read_data, + .post_exec = post_exec_read_data, + }, { .cmd = FDC_FORMAT_TRACK, .args_len = 5, From 72cce14f45a6845d284624ab2b59f7fc8dc88b74 Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 23 Jan 2025 22:04:24 +0100 Subject: [PATCH 23/52] feat(fdc): read track command Preliminary implementation, see in comments --- src/fdc.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/fdc.c b/src/fdc.c index b319773..020b21e 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -76,6 +76,7 @@ typedef struct rw_args_t { } rw_args_t; /* Command callbacks prototypes */ +static void pre_exec_read_track(void); static void pre_exec_specify(void); static void pre_exec_write_data(void); static uint8_t exec_write_data(uint8_t value); @@ -98,6 +99,14 @@ static void buffer_write_size(void); /* Local variables */ // The command descriptors static const fdc_operation_t fdc_operations[] = { + { + .cmd = FDC_READ_TRACK, + .args_len = 8, + .result_len = 7, + .pre_exec = pre_exec_read_track, + .exec = exec_read_data, // same as read data + .post_exec = post_exec_read_data, // same as read data + }, { .cmd = FDC_SPECIFY, .args_len = 2, @@ -206,6 +215,18 @@ static fdc_read_write_t write_buffer_cb = NULL; /* * * * * * * * * * * * * * * Command routines * * * * * * * * * * * * * * */ +static void pre_exec_read_track(void) { + // TODO(giuliof): if I have understood correctly, this is just a read + // command that ignores the record. I can just force it to 1 (first) and go + // on as in read data. + args[3] = 1; + pre_exec_read_data(); + + // TODO(giuliof): another small differences, to be checked, are that this + // command doesn't stop if an error occurs, but stops once reached record = + // EOT. +} + // Specify: // Just print the register values, since the emulator does not care static void pre_exec_specify(void) { From c202aa2dec32cd494052a687c99de8c2d985b740 Mon Sep 17 00:00:00 2001 From: giuliof Date: Fri, 24 Jan 2025 22:47:55 +0100 Subject: [PATCH 24/52] test(fdc): added writing test, which has quite the same pattern as read Test does not pass due to some missing or wrong implementations. --- src/tests/test_fdc.c | 415 +++++++++++++++++++++++++------------------ 1 file changed, 241 insertions(+), 174 deletions(-) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 0167a17..62a782e 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -10,6 +10,9 @@ static void assert_fdc_sr(uint8_t expected_sr); static int fake_read(uint8_t *buffer, uint8_t unit_number, bool phy_head, uint8_t phy_track, bool head, uint8_t track, uint8_t sector); +static int fake_write(uint8_t *buffer, uint8_t unit_number, bool phy_head, + uint8_t phy_track, bool head, uint8_t track, + uint8_t sector); /** * @brief Helper function to check the current status of the FDC main status @@ -188,197 +191,197 @@ struct rw_test_params_t { uint8_t result[7]; }; -ParameterizedTestParameters(ceda_fdc, readCommand0) { - static struct rw_test_params_t params[] = { +static struct rw_test_params_t rwparams[] = { + { + // No MT, end record < EOT, physical head 0 + 0, // No alteration { - // No MT, end record < EOT, physical head 0 - 0, // No alteration - { - 0, // drive number - 7, // cylinder - 0, // head - 5, // record - 1, // N - bytes per sector size factor - 10, // EOT (end of track) - 0, // GPL (ignored) - 0, // DTL (ignored) - }, - { - 0, // Drive number, no error - 0, // no error - 0, // no error - 7, // cylinder - 0, // head - 7, // record - 1, // N - }, + 0, // drive number + 7, // cylinder + 0, // head + 5, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL (ignored) + 0, // DTL (ignored) }, { - // No MT, end record = EOT, physical head 0 - 0, - { - 1, // drive number - 7, // cylinder - 1, // head - different from physical head just for fun - 9, // record - 1, // N - bytes per sector size factor - 10, // EOT (end of track) - 0, // GPL - 0, // DTL - }, - { - 1, // drive number, no error - 0, // no error - 0, // no error - 8, // cylinder - 1, // head - 1, // record - 1, // N - }, + 0, // Drive number, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 7, // record + 1, // N }, + }, + { + // No MT, end record = EOT, physical head 0 + 0, { - // No MT, end record < EOT, physical head 1 - 0, - { - FDC_ST0_HD | 2, // Drive number, physical head 1 - 7, // cylinder - 0, // head - different from physical head just for fun - 5, // record - 1, // N - bytes per sector size factor - 10, // EOT (end of track) - 0, // GPL - 0, // DTL - }, - { - FDC_ST0_HD | 2, // drive number, physical head 1, no error - 0, // no error - 0, // no error - 7, // cylinder - 0, // head - 7, // record - 1, // N - }, + 1, // drive number + 7, // cylinder + 1, // head - different from physical head just for fun + 9, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL }, { - // No MT, end record = EOT, physical head 1 - 0, - { - FDC_ST0_HD | 3, // Drive number, physical head 1 - 7, // cylinder - 1, // head - 9, // record - 1, // N - bytes per sector size factor - 10, // EOT (end of track) - 0, // GPL - 0, // DTL - }, - { - FDC_ST0_HD | 3, // drive number, physical head 1, no error - 0, // no error - 0, // no error - 8, // cylinder - 1, // head - 1, // record - 1, // N - }, + 1, // drive number, no error + 0, // no error + 0, // no error + 8, // cylinder + 1, // head + 1, // record + 1, // N }, - /* * * * * * */ + }, + { + // No MT, end record < EOT, physical head 1 + 0, { - // MT (multi-track), end record < EOT, physical head 0 - FDC_CMD_ARGS_MT_bm, - { - 3, // Drive number - 7, // cylinder - 0, // head - 5, // record - 1, // N - bytes per sector size factor - 10, // EOT (end of track) - 0, // GPL - 0, // DTL - }, - { - 3, // drive number, physical head 0, no error - 0, // no error - 0, // no error - 7, // cylinder - 0, // head - 7, // record - 1, // N - }, + FDC_ST0_HD | 2, // Drive number, physical head 1 + 7, // cylinder + 0, // head - different from physical head just for fun + 5, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL }, { - // MT (multi-track), end record = EOT, physical head 0 - FDC_CMD_ARGS_MT_bm, - { - 2, // Drive number - 7, // cylinder - 1, // head - different from physical head just for fun - 9, // record - 1, // N - bytes per sector size factor - 10, // EOT (end of track) - 0, // GPL - 0, // DTL - }, - { - FDC_ST0_HD | 2, // drive number, physical head 1, no error - 0, // no error - 0, // no error - 7, // cylinder - 0, // head - 1, // record - 1, // N - }, + FDC_ST0_HD | 2, // drive number, physical head 1, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 7, // record + 1, // N }, + }, + { + // No MT, end record = EOT, physical head 1 + 0, { - // MT (multi-track), end record < EOT, physical head 1 - FDC_CMD_ARGS_MT_bm, - { - FDC_ST0_HD | 1, // Drive number, physical head 1 - 7, // cylinder - 0, // head - 5, // record - 1, // N - bytes per sector size factor - 10, // EOT (end of track) - 0, // GPL - 0, // DTL - }, - { - FDC_ST0_HD | 1, // drive number, physical head 1, no error - 0, // no error - 0, // no error - 7, // cylinder - 0, // head - 7, // record - 1, // N - }, + FDC_ST0_HD | 3, // Drive number, physical head 1 + 7, // cylinder + 1, // head + 9, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL }, { - // MT (multi-track), end record = EOT, physical head 1 - FDC_CMD_ARGS_MT_bm, - { - FDC_ST0_HD | 0, // Drive number, physical head 1 - 7, // cylinder - 0, // head - different from physical head just for fun - 9, // record - 1, // N - bytes per sector size factor - 10, // EOT (end of track) - 0, // GPL - 0, // DTL - }, - { - 0, // drive number, physical head 0, no error - 0, // no error - 0, // no error - 8, // cylinder - 1, // head - 1, // record - 1, // N - }, + FDC_ST0_HD | 3, // drive number, physical head 1, no error + 0, // no error + 0, // no error + 8, // cylinder + 1, // head + 1, // record + 1, // N }, - }; + }, + /* * * * * * */ + { + // MT (multi-track), end record < EOT, physical head 0 + FDC_CMD_ARGS_MT_bm, + { + 3, // Drive number + 7, // cylinder + 0, // head + 5, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + 3, // drive number, physical head 0, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 7, // record + 1, // N + }, + }, + { + // MT (multi-track), end record = EOT, physical head 0 + FDC_CMD_ARGS_MT_bm, + { + 2, // Drive number + 7, // cylinder + 1, // head - different from physical head just for fun + 9, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + FDC_ST0_HD | 2, // drive number, physical head 1, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 1, // record + 1, // N + }, + }, + { + // MT (multi-track), end record < EOT, physical head 1 + FDC_CMD_ARGS_MT_bm, + { + FDC_ST0_HD | 1, // Drive number, physical head 1 + 7, // cylinder + 0, // head + 5, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + FDC_ST0_HD | 1, // drive number, physical head 1, no error + 0, // no error + 0, // no error + 7, // cylinder + 0, // head + 7, // record + 1, // N + }, + }, + { + // MT (multi-track), end record = EOT, physical head 1 + FDC_CMD_ARGS_MT_bm, + { + FDC_ST0_HD | 0, // Drive number, physical head 1 + 7, // cylinder + 0, // head - different from physical head just for fun + 9, // record + 1, // N - bytes per sector size factor + 10, // EOT (end of track) + 0, // GPL + 0, // DTL + }, + { + 0, // drive number, physical head 0, no error + 0, // no error + 0, // no error + 8, // cylinder + 1, // head + 1, // record + 1, // N + }, + }, +}; - size_t nb_params = sizeof(params) / sizeof(struct rw_test_params_t); - return cr_make_param_array(struct rw_test_params_t, params, nb_params); +ParameterizedTestParameters(ceda_fdc, readCommand0) { + size_t nb_params = sizeof(rwparams) / sizeof(struct rw_test_params_t); + return cr_make_param_array(struct rw_test_params_t, rwparams, nb_params); } ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, readCommand0) { @@ -424,3 +427,67 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, readCommand0) { // Execution is finished assert_fdc_sr(FDC_ST_RQM); } + +static int fake_write(uint8_t *buffer, uint8_t unit_number, bool phy_head, + uint8_t phy_track, bool head, uint8_t track, + uint8_t sector) { + (void)buffer; + (void)unit_number; + (void)phy_head; + (void)phy_track; + (void)head; + (void)track; + (void)sector; + + // In this case we force sector size to 4 + return 4; +} + +ParameterizedTestParameters(ceda_fdc, writeCommand0) { + size_t nb_params = sizeof(rwparams) / sizeof(struct rw_test_params_t); + return cr_make_param_array(struct rw_test_params_t, rwparams, nb_params); +} + +ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, writeCommand0) { + uint8_t result[sizeof(param->result)]; + + fdc_init(); + + // Link a fake reading function + fdc_kickDiskImage(NULL, fake_write); + + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_WRITE_DATA | param->cmd_alteration); + + // Send arguments checking for no error + sendBuffer(param->arguments, sizeof(param->arguments)); + + // FDC is in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); + + // FDC is ready to receive data + cr_assert_eq(fdc_getIntStatus(), true); + + // Read two full sectors + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + + // FDC is still in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); + + // Stop the writing + fdc_tc_out(0, 0); + + receiveBuffer(result, sizeof(result)); + + cr_assert_arr_eq(result, param->result, sizeof(result)); + + // Execution is finished + assert_fdc_sr(FDC_ST_RQM); +} From 866a6b622cc88cd573fc42633b838a7ec705bec5 Mon Sep 17 00:00:00 2001 From: giuliof Date: Fri, 24 Jan 2025 22:50:26 +0100 Subject: [PATCH 25/52] fix(fdc): writing operation is now correct --- src/fdc.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 020b21e..eeed94a 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -254,7 +254,7 @@ static void pre_exec_write_data(void) { LOG_DEBUG("DTL: %d\n", args[7]); // Set DIO to read for Execution phase - status_register.dio = 1; + status_register.dio = 0; buffer_write_size(); } @@ -319,10 +319,27 @@ static uint8_t exec_write_data(uint8_t value) { } static void post_exec_write_data(void) { + rw_args_t *rw_args = (rw_args_t *)args; + LOG_DEBUG("Write has ended\n"); - // TODO(giulio): populate result (which is pretty the same for read, - // write, ...) + memset(result, 0x00, sizeof(result)); + + // TODO(giuliof): populate result as in datasheet (see table 2) + /* ST0 */ + // Current head position + result[0] |= rw_args->unit_select; + result[0] |= rw_args->head_address ? FDC_ST0_HD : 0; + /* ST1 */ + result[1] |= 0; // TODO(giuliof): populate this + /* ST2 */ + result[2] |= 0; // TODO(giuliof): populate this + /* CHR */ + result[3] = rw_args->cylinder; + result[4] = rw_args->head; + result[5] = rw_args->record; + /* Sector size factor */ + result[6] = rw_args->n; } // Read data: @@ -402,8 +419,7 @@ static void post_exec_read_data(void) { rw_args_t *rw_args = (rw_args_t *)args; LOG_DEBUG("Read has ended\n"); - // TODO(giulio): populate result (which is pretty the same for read, - // write, ...) + memset(result, 0x00, sizeof(result)); // TODO(giuliof): populate result as in datasheet (see table 2) @@ -617,6 +633,7 @@ static void buffer_write_size(void) { rwcount = 0; // TODO(giuliof) rwcount_max = min(DTL, ret) rwcount_max = (size_t)ret; + is_ready = true; } /* * * * * * * * * * * * * * * Public routines * * * * * * * * * * * * * * */ From 63f77b21b5d293f3f56afc395b5b8342e7c04f5c Mon Sep 17 00:00:00 2001 From: giuliof Date: Sun, 26 Jan 2025 14:30:01 +0100 Subject: [PATCH 26/52] fix(fdc): when EOT is reached, read or write operation terminates TODO: check if this is true on the actual FDC, in that case add a proper test! --- src/fdc.c | 30 ++++++++++++++++++++++-------- src/tests/test_fdc.c | 6 ++++-- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index eeed94a..c9b23b3 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -297,20 +297,27 @@ static uint8_t exec_write_data(uint8_t value) { // Last sector of the track if (rw_args->record > rw_args->eot) { + // In any case, reached the end of track we start back from sector 1 + rw_args->record = 1; + // Multi track mode, if enabled the read operation go on on the next // side if (command_args & FDC_CMD_ARGS_MT_bm) { rw_args->head_address = !rw_args->head_address; rw_args->head = !rw_args->head; - if (!rw_args->head_address) + if (!rw_args->head_address) { + // Terminate execution if end of track is reached + tc_status = true; rw_args->cylinder++; + return 0; + } } else { + // Terminate execution if end of track is reached + tc_status = true; rw_args->cylinder++; + return 0; } - - // In any case, reached the end of track we start back from sector 1 - rw_args->record = 1; } buffer_write_size(); @@ -394,20 +401,27 @@ static uint8_t exec_read_data(uint8_t value) { // Last sector of the track if (rw_args->record > rw_args->eot) { + // In any case, reached the end of track we start back from sector 1 + rw_args->record = 1; + // Multi track mode, if enabled the read operation go on on the next // side if (command_args & FDC_CMD_ARGS_MT_bm) { rw_args->head_address = !rw_args->head_address; rw_args->head = !rw_args->head; - if (!rw_args->head_address) + if (!rw_args->head_address) { + // Terminate execution if end of track is reached + tc_status = true; rw_args->cylinder++; + return 0; + } } else { + // Terminate execution if end of track is reached + tc_status = true; rw_args->cylinder++; + return 0; } - - // In any case, reached the end of track we start back from sector 1 - rw_args->record = 1; } buffer_update(); diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 62a782e..05540de 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -415,7 +415,8 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, readCommand0) { fdc_in(FDC_ADDR_DATA_REGISTER); // FDC is still in execution mode - assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); + // TODO(giuliof): This is not always true + // assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); // Stop the reading fdc_tc_out(0, 0); @@ -479,7 +480,8 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, writeCommand0) { fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); // FDC is still in execution mode - assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); + // TODO(giuliof): This is not always true + // assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); // Stop the writing fdc_tc_out(0, 0); From a55d939729bed2d909e246e821eafdf0e92f5b92 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sun, 26 Jan 2025 16:02:19 +0100 Subject: [PATCH 27/52] refactor(fdc): removed bitfields to increase portability --- src/fdc.c | 97 ++++++++++++++++++++++++------------------------------- 1 file changed, 42 insertions(+), 55 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index c9b23b3..bbc0715 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -39,33 +39,9 @@ typedef struct fdc_operation_t { void (*post_exec)(void); } fdc_operation_t; -// Main status register bitfield -typedef union main_status_register_t { - uint8_t value; - struct { - // Drive x is in Seek mode - uint8_t fdd0_busy : 1; - uint8_t fdd1_busy : 1; - uint8_t fdd2_busy : 1; - uint8_t fdd3_busy : 1; - // Controller has already accepted a command - uint8_t fdc_busy : 1; - // Execution mode - uint8_t exm : 1; - // Data I/O, set if FDC is read from CPU, clear otherwise - uint8_t dio : 1; - // Request From Master, set if FDC is ready to receive or send data - uint8_t rqm : 1; - uint8_t : 0; - }; -} main_status_register_t; - // Parsing structure for read and write arguments -// TODO(giuliof): this is not portable typedef struct rw_args_t { - uint8_t unit_select : 2; - uint8_t head_address : 1; - uint8_t : 0; + uint8_t unit_head; uint8_t cylinder; uint8_t head; uint8_t record; @@ -203,7 +179,7 @@ static bool is_ready = false; /* FDC internal registers */ // Main Status Register -static main_status_register_t status_register; +static uint8_t status_register; /* Floppy disk status */ // Current track position @@ -254,13 +230,14 @@ static void pre_exec_write_data(void) { LOG_DEBUG("DTL: %d\n", args[7]); // Set DIO to read for Execution phase - status_register.dio = 0; + status_register &= (uint8_t)~FDC_ST_DIO; buffer_write_size(); } static uint8_t exec_write_data(uint8_t value) { rw_args_t *rw_args = (rw_args_t *)args; + uint8_t drive = rw_args->unit_head & FDC_ST0_US; if (write_buffer_cb == NULL) return 0; @@ -282,9 +259,9 @@ static uint8_t exec_write_data(uint8_t value) { CEDA_STRONG_ASSERT_TRUE(sector != 0); // But all other routines counts sectors from 0 sector--; - int ret = write_buffer_cb( - exec_buffer, rw_args->unit_select, rw_args->head_address, - track[rw_args->unit_select], rw_args->head, rw_args->cylinder, sector); + int ret = + write_buffer_cb(exec_buffer, drive, rw_args->unit_head & FDC_ST0_HD, + track[drive], rw_args->head, rw_args->cylinder, sector); // the image is always loaded and valid CEDA_STRONG_ASSERT_TRUE(ret > 0); // Buffer is statically allocated, be sure that the data can fit it @@ -303,10 +280,10 @@ static uint8_t exec_write_data(uint8_t value) { // Multi track mode, if enabled the read operation go on on the next // side if (command_args & FDC_CMD_ARGS_MT_bm) { - rw_args->head_address = !rw_args->head_address; + rw_args->unit_head ^= FDC_ST0_HD; rw_args->head = !rw_args->head; - if (!rw_args->head_address) { + if (!(rw_args->unit_head & FDC_ST0_HD)) { // Terminate execution if end of track is reached tc_status = true; rw_args->cylinder++; @@ -327,6 +304,7 @@ static uint8_t exec_write_data(uint8_t value) { static void post_exec_write_data(void) { rw_args_t *rw_args = (rw_args_t *)args; + uint8_t drive = rw_args->unit_head & FDC_ST0_US; LOG_DEBUG("Write has ended\n"); @@ -335,8 +313,8 @@ static void post_exec_write_data(void) { // TODO(giuliof): populate result as in datasheet (see table 2) /* ST0 */ // Current head position - result[0] |= rw_args->unit_select; - result[0] |= rw_args->head_address ? FDC_ST0_HD : 0; + result[0] |= drive; + result[0] |= rw_args->unit_head & FDC_ST0_HD ? FDC_ST0_HD : 0; /* ST1 */ result[1] |= 0; // TODO(giuliof): populate this /* ST2 */ @@ -367,7 +345,7 @@ static void pre_exec_read_data(void) { LOG_DEBUG("DTL: %d\n", args[7]); // Set DIO to read for Execution phase - status_register.dio = 1; + status_register |= FDC_ST_DIO; // TODO(giuliof) create handles to manage more than one floppy image at a // time @@ -407,10 +385,10 @@ static uint8_t exec_read_data(uint8_t value) { // Multi track mode, if enabled the read operation go on on the next // side if (command_args & FDC_CMD_ARGS_MT_bm) { - rw_args->head_address = !rw_args->head_address; + rw_args->unit_head ^= FDC_ST0_HD; rw_args->head = !rw_args->head; - if (!rw_args->head_address) { + if (!(rw_args->unit_head & FDC_ST0_HD)) { // Terminate execution if end of track is reached tc_status = true; rw_args->cylinder++; @@ -431,6 +409,7 @@ static uint8_t exec_read_data(uint8_t value) { static void post_exec_read_data(void) { rw_args_t *rw_args = (rw_args_t *)args; + uint8_t drive = rw_args->unit_head & FDC_ST0_US; LOG_DEBUG("Read has ended\n"); @@ -439,8 +418,8 @@ static void post_exec_read_data(void) { // TODO(giuliof): populate result as in datasheet (see table 2) /* ST0 */ // Current head position - result[0] |= rw_args->unit_select; - result[0] |= rw_args->head_address ? FDC_ST0_HD : 0; + result[0] |= drive; + result[0] |= rw_args->unit_head & FDC_ST0_HD ? FDC_ST0_HD : 0; /* ST1 */ result[1] |= 0; // TODO(giuliof): populate this /* ST2 */ @@ -526,7 +505,7 @@ static void fdc_compute_next_status(void) { if (fdc_status == CMD) { // Set DIO to write for ARGS phase - status_register.dio = 0; + status_register &= (uint8_t)~FDC_ST_DIO; fdc_status = ARGS; rwcount_max = fdc_currop->args_len; @@ -546,7 +525,7 @@ static void fdc_compute_next_status(void) { if (fdc_status == EXEC && (tc_status || fdc_currop->exec == NULL)) { tc_status = false; // Set DIO to read for RESULT phase - status_register.dio = 1; + status_register |= FDC_ST_DIO; if (fdc_currop->post_exec) fdc_currop->post_exec(); @@ -558,7 +537,7 @@ static void fdc_compute_next_status(void) { if (fdc_status == RESULT && rwcount == rwcount_max) { // Set DIO to write for CMD and ARGS phases - status_register.dio = 0; + status_register &= (uint8_t)~FDC_ST_DIO; fdc_status = CMD; rwcount_max = 0; @@ -566,12 +545,20 @@ static void fdc_compute_next_status(void) { } // Update step dependant bits in main status register - status_register.exm = fdc_status == EXEC; - status_register.fdc_busy = fdc_status != CMD; + if (fdc_status == EXEC) + status_register |= FDC_ST_EXM; + else + status_register &= (uint8_t)~FDC_ST_EXM; + + if (fdc_status != CMD) + status_register |= FDC_ST_CB; + else + status_register &= (uint8_t)~FDC_ST_CB; } static void buffer_update(void) { rw_args_t *rw_args = (rw_args_t *)args; + uint8_t drive = rw_args->unit_head & FDC_ST0_US; uint8_t sector = rw_args->record; @@ -589,9 +576,9 @@ static void buffer_update(void) { return; // TODO(giuliof): add proper error code, zero is no mounted image - int ret = read_buffer_cb(NULL, rw_args->unit_select, rw_args->head_address, - track[rw_args->unit_select], rw_args->head, - rw_args->cylinder, sector); + int ret = + read_buffer_cb(NULL, drive, rw_args->unit_head & FDC_ST0_HD, + track[drive], rw_args->head, rw_args->cylinder, sector); if (ret != 0) { // TODO(giuliof): At the moment we do not support error codes, we assume @@ -600,8 +587,8 @@ static void buffer_update(void) { // Buffer is statically allocated, be sure that the data can fit it CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); - ret = read_buffer_cb(exec_buffer, rw_args->unit_select, - rw_args->head_address, track[rw_args->unit_select], + ret = read_buffer_cb(exec_buffer, drive, + rw_args->unit_head & FDC_ST0_HD, track[drive], rw_args->head, rw_args->cylinder, sector); // TODO(giuliof): At the moment we do not support error codes, we assume // the image is always loaded and valid @@ -618,6 +605,7 @@ static void buffer_update(void) { static void buffer_write_size(void) { rw_args_t *rw_args = (rw_args_t *)args; + uint8_t drive = rw_args->unit_head & FDC_ST0_US; uint8_t sector = rw_args->record; @@ -634,9 +622,9 @@ static void buffer_write_size(void) { if (write_buffer_cb == NULL) return; - int ret = write_buffer_cb(NULL, rw_args->unit_select, rw_args->head_address, - track[rw_args->unit_select], rw_args->head, - rw_args->cylinder, sector); + int ret = + write_buffer_cb(NULL, drive, rw_args->unit_head & FDC_ST0_HD, + track[drive], rw_args->head, rw_args->cylinder, sector); // TODO(giuliof): At the moment we do not support error codes, we assume the // image is always loaded and valid @@ -665,8 +653,7 @@ void fdc_init(void) { // Reset main status register, but keep RQM active since FDC is always ready // to receive requests - status_register.value = 0x00; - status_register.rqm = 1; + status_register = FDC_ST_RQM; // Reset track positions memset(track, 0, sizeof(track)); @@ -679,7 +666,7 @@ void fdc_init(void) { uint8_t fdc_in(ceda_ioaddr_t address) { switch (address & 0x01) { case FDC_ADDR_STATUS_REGISTER: - return status_register.value; + return status_register; case FDC_ADDR_DATA_REGISTER: { uint8_t value = 0; From 6296a40233d94be8d24e2e49ed32cffdb99fd62e Mon Sep 17 00:00:00 2001 From: giuliof Date: Wed, 5 Feb 2025 09:40:49 +0100 Subject: [PATCH 28/52] test(fdc): improved read/write test TODO: to be confirmed --- src/tests/test_fdc.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 05540de..1c3a436 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -187,6 +187,7 @@ Test(ceda_fdc, readCommandNoMedium) { struct rw_test_params_t { uint8_t cmd_alteration; + bool tc_required; uint8_t arguments[8]; uint8_t result[7]; }; @@ -194,7 +195,8 @@ struct rw_test_params_t { static struct rw_test_params_t rwparams[] = { { // No MT, end record < EOT, physical head 0 - 0, // No alteration + 0, // No alteration + true, // Terminal count required { 0, // drive number 7, // cylinder @@ -218,6 +220,7 @@ static struct rw_test_params_t rwparams[] = { { // No MT, end record = EOT, physical head 0 0, + false, // Terminal count not required { 1, // drive number 7, // cylinder @@ -241,6 +244,7 @@ static struct rw_test_params_t rwparams[] = { { // No MT, end record < EOT, physical head 1 0, + true, // Terminal count required { FDC_ST0_HD | 2, // Drive number, physical head 1 7, // cylinder @@ -264,6 +268,7 @@ static struct rw_test_params_t rwparams[] = { { // No MT, end record = EOT, physical head 1 0, + false, // Terminal count not required { FDC_ST0_HD | 3, // Drive number, physical head 1 7, // cylinder @@ -288,6 +293,7 @@ static struct rw_test_params_t rwparams[] = { { // MT (multi-track), end record < EOT, physical head 0 FDC_CMD_ARGS_MT_bm, + true, // Terminal count required { 3, // Drive number 7, // cylinder @@ -311,6 +317,7 @@ static struct rw_test_params_t rwparams[] = { { // MT (multi-track), end record = EOT, physical head 0 FDC_CMD_ARGS_MT_bm, + true, // Terminal count required { 2, // Drive number 7, // cylinder @@ -334,6 +341,7 @@ static struct rw_test_params_t rwparams[] = { { // MT (multi-track), end record < EOT, physical head 1 FDC_CMD_ARGS_MT_bm, + true, // Terminal count required { FDC_ST0_HD | 1, // Drive number, physical head 1 7, // cylinder @@ -357,6 +365,7 @@ static struct rw_test_params_t rwparams[] = { { // MT (multi-track), end record = EOT, physical head 1 FDC_CMD_ARGS_MT_bm, + false, // Terminal count not required { FDC_ST0_HD | 0, // Drive number, physical head 1 7, // cylinder @@ -414,12 +423,13 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, readCommand0) { fdc_in(FDC_ADDR_DATA_REGISTER); fdc_in(FDC_ADDR_DATA_REGISTER); - // FDC is still in execution mode - // TODO(giuliof): This is not always true - // assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); - - // Stop the reading - fdc_tc_out(0, 0); + // Stop the reading, if needed + if (param->tc_required) { + // FDC is still in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); + // Request execution termination + fdc_tc_out(0, 0); + } receiveBuffer(result, sizeof(result)); @@ -479,12 +489,13 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, writeCommand0) { fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); - // FDC is still in execution mode - // TODO(giuliof): This is not always true - // assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); - - // Stop the writing - fdc_tc_out(0, 0); + // Stop the reading, if needed + if (param->tc_required) { + // FDC is still in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); + // Request execution termination + fdc_tc_out(0, 0); + } receiveBuffer(result, sizeof(result)); From e23828bc6c26b299c4bc346095f473c4aa545de5 Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 6 Feb 2025 19:28:52 +0100 Subject: [PATCH 29/52] feat(fdc): added format command support --- src/fdc.c | 78 +++++++++++++++++++++++++++++++++++++++++++- src/tests/test_fdc.c | 50 ++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/src/fdc.c b/src/fdc.c index bbc0715..6738384 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -51,6 +51,15 @@ typedef struct rw_args_t { uint8_t stp; } rw_args_t; +// Parsing structure for format arguments +typedef struct format_args_t { + uint8_t unit_head; + uint8_t n; // bytes per sector factor + uint8_t sec_per_track; // number of sectors per track + uint8_t gpl; // gap length + uint8_t d; // filler byte +} format_args_t; + /* Command callbacks prototypes */ static void pre_exec_read_track(void); static void pre_exec_specify(void); @@ -468,15 +477,82 @@ static void post_exec_sense_interrupt(void) { // Format track static void pre_exec_format_track(void) { + format_args_t *format_args = (format_args_t *)args; + + // Extract plain data from the bitfield + uint8_t phy_head = !!(format_args->unit_head & FDC_ST0_HD); + uint8_t drive = format_args->unit_head & FDC_ST0_US; + + // TODO(giuliof): eventually: use the appropriate structure LOG_DEBUG("FDC Format track\n"); + LOG_DEBUG("MF: %d\n", (command_args >> 6) & 0x01); + LOG_DEBUG("Drive: %d\n", args[0] & 0x3); + LOG_DEBUG("HD: %d\n", (args[0] >> 2) & 0x1); + LOG_DEBUG("N: %d\n", args[1]); + LOG_DEBUG("SPT: %d\n", args[2]); + LOG_DEBUG("GPL: %d\n", args[3]); + LOG_DEBUG("D: %d\n", args[4]); + + // Initialize execution phase counter. + // The FORMAT command requires the filling of a buffer of "ID fields", one + // for each sector within the same track. Each "ID field" is 4 bytes long. + // The number of sectors per track is specified by the command itself (SPT + // argument). + rwcount = 0; + rwcount_max = (size_t)(format_args->sec_per_track * 4); + assert(rwcount_max <= sizeof(exec_buffer)); + + // check if the medium is present, else no irq is generated + if (write_buffer_cb == NULL) + return; + + // Check if medium is valid by poking sector 0 of the desired track + // TODO(giuliof): add proper error code, zero is no mounted image + int ret = write_buffer_cb(NULL, drive, phy_head, track[drive], phy_head, + track[drive], 0); + + if (ret <= 0) + return; + + is_ready = true; } static uint8_t exec_format_track(uint8_t value) { - return value; + exec_buffer[rwcount++] = value; + + return 0; } static void post_exec_format_track(void) { + format_args_t *format_args = (format_args_t *)args; + + // Extract plain data from the bitfield + uint8_t phy_head = !!(format_args->unit_head & FDC_ST0_HD); + uint8_t drive = format_args->unit_head & FDC_ST0_US; + + // At the moment the track format is just a writing over all "pre-formatted" + // sectors. An arbitrary format is currently not supported. + for (size_t s = 0; s < format_args->sec_per_track; s++) { + uint8_t *id_field = exec_buffer + (4 * s); + uint8_t cylinder = id_field[0]; + uint8_t head = id_field[1]; + uint8_t record = id_field[2] - 1; + + int ret = write_buffer_cb(NULL, drive, phy_head, track[drive], head, + cylinder, record); + CEDA_STRONG_ASSERT_TRUE(ret > 0); + + uint8_t format_buffer[ret]; + memset(format_buffer, format_args->d, (size_t)ret); + + ret = write_buffer_cb(format_buffer, drive, phy_head, track[drive], + head, cylinder, record); + CEDA_STRONG_ASSERT_TRUE(ret > 0); + } + LOG_DEBUG("FDC end Format track\n"); + + memset(result, 0, sizeof(result)); } // Seek diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 1c3a436..ce64d14 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -504,3 +504,53 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, writeCommand0) { // Execution is finished assert_fdc_sr(FDC_ST_RQM); } + +Test(ceda_fdc, formatCommand) { + uint8_t result[7]; + uint8_t arguments[] = { + 0x01 | FDC_ST0_HD, + 0x01, + 0x02, // two sectors per track + 0x00, // gap (we don't care) + 0x35, // fill byte + }; + + fdc_init(); + + // Link a fake reading function + fdc_kickDiskImage(NULL, fake_write); + + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_FORMAT_TRACK); + + // Send arguments checking for no error + sendBuffer(arguments, sizeof(arguments)); + + // FDC is in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); + + // FDC is ready to receive data + cr_assert_eq(fdc_getIntStatus(), true); + + // First sector ID + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x01); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x01); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x01); + + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x01); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x02); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x01); + + // FDC is still in execution mode + // TODO(giuliof): This is not always true + assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); + + // Stop the writing + fdc_tc_out(0, 0); + + receiveBuffer(result, sizeof(result)); + + // Execution is finished + assert_fdc_sr(FDC_ST_RQM); +} From d6c620318b6a4211a29ee58d1d62e8cbbc912af2 Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 6 Feb 2025 21:29:26 +0100 Subject: [PATCH 30/52] feat(fdc): implemented invalid command handling --- src/fdc.c | 13 ++++++++++--- src/fdc_registers.h | 1 + src/tests/test_fdc.c | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 6738384..49107ab 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -747,8 +747,10 @@ uint8_t fdc_in(ceda_ioaddr_t address) { uint8_t value = 0; if (fdc_status == CMD) { - // you should never read during command phase - LOG_WARN("FDC read access during CMD phase!\n"); + // You should never read when in CMD status. + // Just reply with "invalid command" and restore data direction + value = 0x80; + status_register &= (uint8_t)~FDC_ST_DIO; } else if (fdc_status == ARGS) { // you should never read during command phase LOG_WARN("FDC read access during ARGS phase!\n"); @@ -794,8 +796,13 @@ void fdc_out(ceda_ioaddr_t address, uint8_t value) { break; } } - if (fdc_currop == NULL) + if (fdc_currop == NULL) { LOG_WARN("Command %x is not implemented\n", cmd); + + // Invalid command: set the main status register to read to + // serve ST0 and error code + status_register |= FDC_ST_DIO; + } } else if (fdc_status == ARGS) { assert(rwcount < sizeof(args) / sizeof(*args)); args[rwcount] = value; diff --git a/src/fdc_registers.h b/src/fdc_registers.h index bce7d7c..931fb9e 100644 --- a/src/fdc_registers.h +++ b/src/fdc_registers.h @@ -9,6 +9,7 @@ // Don't ask me why they didn't use a single nibble to represent all the // commands. Please refer to the datasheet for more information. typedef enum fdc_cmd_t { + INVALID = 0x00, FDC_READ_TRACK = 0x02, FDC_SPECIFY = 0x03, FDC_SENSE_DRIVE = 0x04, diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index ce64d14..8e1ff52 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -554,3 +554,21 @@ Test(ceda_fdc, formatCommand) { // Execution is finished assert_fdc_sr(FDC_ST_RQM); } + +Test(ceda_fdc, invalidCommand) { + uint8_t st0; + + fdc_init(); + + // Force an invalid command + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO); + + st0 = fdc_in(FDC_ADDR_DATA_REGISTER); + cr_expect_eq(st0, 0x80); + + // TODO(giuliof): this is actually unclear, the direction has to be set + // back? probably yes + assert_fdc_sr(FDC_ST_RQM); +} From 10bcdff16ada252ac2c32831eb836cb899513362 Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 6 Feb 2025 21:44:40 +0100 Subject: [PATCH 31/52] feat(fdc): implemented correct handling of an invalid sequence after seek/recalibrate --- src/fdc.c | 16 +++++++++++----- src/tests/test_fdc.c | 27 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 49107ab..5362965 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -788,12 +788,18 @@ void fdc_out(ceda_ioaddr_t address, uint8_t value) { command_args = value & FDC_CMD_ARGS_bm; // Unroll the command list and place it in the current execution + // But ignore command if in interrupt status (after seek or + // recalibrate) and next command is not sense interrupt. + // In this case, the command is treated as invalid. fdc_currop = NULL; - for (size_t i = 0; - i < sizeof(fdc_operations) / sizeof(*fdc_operations); i++) { - if (cmd == fdc_operations[i].cmd) { - fdc_currop = &fdc_operations[i]; - break; + if (!(is_ready && cmd != FDC_SENSE_INTERRUPT)) { + for (size_t i = 0; + i < sizeof(fdc_operations) / sizeof(*fdc_operations); + i++) { + if (cmd == fdc_operations[i].cmd) { + fdc_currop = &fdc_operations[i]; + break; + } } } if (fdc_currop == NULL) { diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 8e1ff52..688adfc 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -126,6 +126,33 @@ Test(ceda_fdc, seekCommand) { cr_expect_eq(data, 5); } +Test(ceda_fdc, invalidSeekSequence) { + fdc_init(); + + uint8_t data; + + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_SEEK); + + // Now read status register to check that FDC is ready to receive arguments + assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); + + // First argument is number of drive + fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + // Second argument is cylinder position + fdc_out(FDC_ADDR_DATA_REGISTER, 7); + + // FDC is no more busy + assert_fdc_sr(FDC_ST_RQM); + + // Send another command that is not FDC_SENSE_INTERRUPT + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_SPECIFY); + + // FDC does not process this command and asserts invalid command + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO); + data = fdc_in(FDC_ADDR_DATA_REGISTER); + cr_expect_eq(data, 0x80); +} + /** * @brief Auxiliary function, send a data buffer to the FDC checking that it is * in input mode for each byte From 4ad3980ec922db1f4a18e24a0051631d819969a0 Mon Sep 17 00:00:00 2001 From: giuliof Date: Thu, 6 Feb 2025 21:47:25 +0100 Subject: [PATCH 32/52] refactor(fdc): renamed variable according to its function --- src/fdc.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 5362965..4a78fe7 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -184,7 +184,7 @@ static uint8_t exec_buffer[1024]; // Result buffer. Each command has maximum 7 bytes as argument. static uint8_t result[7]; static bool tc_status = false; -static bool is_ready = false; +static bool int_status = false; /* FDC internal registers */ // Main Status Register @@ -452,7 +452,7 @@ static void pre_exec_recalibrate(void) { track[drive] = 0; // We don't have to actually move the head. The drive is immediately ready - is_ready = true; + int_status = true; } // Sense interrupt: @@ -461,7 +461,7 @@ static void post_exec_sense_interrupt(void) { uint8_t drive = 0; // After reading interrupt status, ready can be deasserted - is_ready = false; + int_status = false; LOG_DEBUG("FDC Sense Interrupt\n"); /* Status Register 0 */ @@ -514,7 +514,7 @@ static void pre_exec_format_track(void) { if (ret <= 0) return; - is_ready = true; + int_status = true; } static uint8_t exec_format_track(uint8_t value) { @@ -566,7 +566,7 @@ static void pre_exec_seek(void) { LOG_DEBUG("NCN: %d\n", track[drive]); // We don't have to actually move the head. The drive is immediately ready - is_ready = true; + int_status = true; } /* * * * * * * * * * * * * * * Utility routines * * * * * * * * * * * * * * */ @@ -639,7 +639,7 @@ static void buffer_update(void) { uint8_t sector = rw_args->record; // Default: no data ready to be served - is_ready = false; + int_status = false; rwcount_max = 0; // FDC counts sectors from 1 @@ -671,7 +671,7 @@ static void buffer_update(void) { CEDA_STRONG_ASSERT_TRUE(ret > 0); // Ready to serve data - is_ready = true; + int_status = true; } rwcount = 0; @@ -686,7 +686,7 @@ static void buffer_write_size(void) { uint8_t sector = rw_args->record; // Default: no data ready to be served - is_ready = false; + int_status = false; rwcount_max = 0; // FDC counts sectors from 1 @@ -711,7 +711,7 @@ static void buffer_write_size(void) { rwcount = 0; // TODO(giuliof) rwcount_max = min(DTL, ret) rwcount_max = (size_t)ret; - is_ready = true; + int_status = true; } /* * * * * * * * * * * * * * * Public routines * * * * * * * * * * * * * * */ @@ -725,7 +725,7 @@ void fdc_init(void) { rwcount_max = 0; memset(result, 0, sizeof(result)); tc_status = false; - is_ready = false; + int_status = false; // Reset main status register, but keep RQM active since FDC is always ready // to receive requests @@ -792,7 +792,7 @@ void fdc_out(ceda_ioaddr_t address, uint8_t value) { // recalibrate) and next command is not sense interrupt. // In this case, the command is treated as invalid. fdc_currop = NULL; - if (!(is_ready && cmd != FDC_SENSE_INTERRUPT)) { + if (!(int_status && cmd != FDC_SENSE_INTERRUPT)) { for (size_t i = 0; i < sizeof(fdc_operations) / sizeof(*fdc_operations); i++) { @@ -848,7 +848,7 @@ void fdc_tc_out(ceda_ioaddr_t address, uint8_t value) { // (beginning of result phase). When first byte of result phase data // is read, INT=0. bool fdc_getIntStatus(void) { - return is_ready; + return int_status; } // TODO(giuliof): describe better this function From d4199a8f5df6554f2e38121b09f9e5096c7628ea Mon Sep 17 00:00:00 2001 From: giuliof Date: Fri, 7 Feb 2025 23:03:19 +0100 Subject: [PATCH 33/52] refactor: introduced disk image errors with the aim of handling them in FDC --- src/fdc.c | 16 ++++++++-------- src/fdc.h | 6 ++++++ src/floppy.c | 34 ++++++++++++++++++---------------- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 4a78fe7..2443c48 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -272,7 +272,7 @@ static uint8_t exec_write_data(uint8_t value) { write_buffer_cb(exec_buffer, drive, rw_args->unit_head & FDC_ST0_HD, track[drive], rw_args->head, rw_args->cylinder, sector); // the image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret > 0); + CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); // Buffer is statically allocated, be sure that the data can fit it CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); @@ -511,7 +511,7 @@ static void pre_exec_format_track(void) { int ret = write_buffer_cb(NULL, drive, phy_head, track[drive], phy_head, track[drive], 0); - if (ret <= 0) + if (ret <= DISK_IMAGE_NOMEDIUM) return; int_status = true; @@ -540,14 +540,14 @@ static void post_exec_format_track(void) { int ret = write_buffer_cb(NULL, drive, phy_head, track[drive], head, cylinder, record); - CEDA_STRONG_ASSERT_TRUE(ret > 0); + CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); uint8_t format_buffer[ret]; memset(format_buffer, format_args->d, (size_t)ret); ret = write_buffer_cb(format_buffer, drive, phy_head, track[drive], head, cylinder, record); - CEDA_STRONG_ASSERT_TRUE(ret > 0); + CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); } LOG_DEBUG("FDC end Format track\n"); @@ -656,10 +656,10 @@ static void buffer_update(void) { read_buffer_cb(NULL, drive, rw_args->unit_head & FDC_ST0_HD, track[drive], rw_args->head, rw_args->cylinder, sector); - if (ret != 0) { + if (ret != DISK_IMAGE_NOMEDIUM) { // TODO(giuliof): At the moment we do not support error codes, we assume // the image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret > 0); + CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); // Buffer is statically allocated, be sure that the data can fit it CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); @@ -668,7 +668,7 @@ static void buffer_update(void) { rw_args->head, rw_args->cylinder, sector); // TODO(giuliof): At the moment we do not support error codes, we assume // the image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret > 0); + CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); // Ready to serve data int_status = true; @@ -704,7 +704,7 @@ static void buffer_write_size(void) { // TODO(giuliof): At the moment we do not support error codes, we assume the // image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret > 0); + CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); // Buffer is statically allocated, be sure that the data can fit it CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); diff --git a/src/fdc.h b/src/fdc.h index 36ef2e5..eba636b 100644 --- a/src/fdc.h +++ b/src/fdc.h @@ -6,6 +6,12 @@ #include #include +typedef enum disk_image_err_t { + DISK_IMAGE_NOMEDIUM = 0, // No medium available + DISK_IMAGE_ERR = -1, // Generic error + DISK_IMAGE_INVALID_GEOMETRY = -2, +} disk_image_err_t; + typedef int (*fdc_read_write_t)(uint8_t *buffer, uint8_t unit_number, bool phy_head, uint8_t phy_track, bool head, uint8_t track, uint8_t sector); diff --git a/src/floppy.c b/src/floppy.c index 8293a2f..7bf49ec 100644 --- a/src/floppy.c +++ b/src/floppy.c @@ -88,37 +88,38 @@ ssize_t floppy_unload_image(unsigned int unit_number) { static int floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, uint8_t phy_track, bool head, uint8_t track, uint8_t sector) { - assert(unit_number < ARRAY_SIZE(floppy_units)); + if (unit_number >= ARRAY_SIZE(floppy_units)) + return DISK_IMAGE_NOMEDIUM; FILE *fd = floppy_units[unit_number].fd; size_t len = 256; long offset; - // No disk loaded, raise error + // No disk loaded if (fd == NULL) - return 0; // TODO(giuliof): no disk loaded error + return DISK_IMAGE_NOMEDIUM; // Due to its structure, with Ceda File Format the physical head and track // must be same as their logical counterpart. if (phy_head != head) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; if (phy_track != track) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; // CFF has up to 80 tracks if (track > 79) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; // Locate sector start if (track == 0 && head == 0) { // First track has max 16 sectors if (sector > 15) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; // Compute byte offset to sector start offset = (long)sector * 256; } else { if (sector > 5) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; // Compute byte offset to sector start offset = (long)track * 1024 * 5 * 2; @@ -135,7 +136,7 @@ static int floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, // If requested, load sector into buffer if (buffer) { if (fseek(fd, offset, SEEK_SET) < 0) - return -1; + return DISK_IMAGE_ERR; len = fread(buffer, sizeof(uint8_t), len, fd); } @@ -146,7 +147,8 @@ static int floppy_read_buffer(uint8_t *buffer, uint8_t unit_number, static int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, bool phy_head, uint8_t phy_track, bool head, uint8_t track, uint8_t sector) { - assert(unit_number < ARRAY_SIZE(floppy_units)); + if (unit_number >= ARRAY_SIZE(floppy_units)) + return DISK_IMAGE_NOMEDIUM; FILE *fd = floppy_units[unit_number].fd; size_t len = 256; @@ -159,24 +161,24 @@ static int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, // Due to its structure, with Ceda File Format the physical head and track // must be same as their logical counterpart. if (phy_head != head) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; if (phy_track != track) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; // CFF has up to 80 tracks if (track > 79) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; // Locate sector start if (track == 0 && head == 0) { // First track has max 16 sectors if (sector > 15) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; // Compute byte offset to sector start offset = (long)sector * 256; } else { if (sector > 5) - return -1; + return DISK_IMAGE_INVALID_GEOMETRY; // Compute byte offset to sector start offset = (long)track * 1024 * 5 * 2; @@ -193,7 +195,7 @@ static int floppy_write_buffer(uint8_t *buffer, uint8_t unit_number, // If requested, load sector into buffer if (buffer) { if (fseek(fd, offset, SEEK_SET) < 0) - return -1; + return DISK_IMAGE_ERR; len = fwrite(buffer, sizeof(uint8_t), len, fd); } From 8654c11b5a1bd835a97c601acf6a1547d1072f4b Mon Sep 17 00:00:00 2001 From: giuliof Date: Fri, 7 Feb 2025 23:06:50 +0100 Subject: [PATCH 34/52] fix: handle DTL and use it for test purposes DTL is data transfer length and it is used with N=0 to specify the number of bytes per sector transferred during read or write operations. In tests, this is used to avoid very long transfers. --- src/fdc.c | 12 ++++++---- src/tests/test_fdc.c | 52 ++++++++++++++++++++++---------------------- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 2443c48..5b2b78a 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -675,8 +675,10 @@ static void buffer_update(void) { } rwcount = 0; - // TODO(giuliof) rwcount_max = min(DTL, ret) - rwcount_max = (size_t)ret; + if (rw_args->n == 0) + rwcount_max = MIN((size_t)rw_args->stp, (size_t)ret); + else + rwcount_max = (size_t)ret; } static void buffer_write_size(void) { @@ -709,8 +711,10 @@ static void buffer_write_size(void) { CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); rwcount = 0; - // TODO(giuliof) rwcount_max = min(DTL, ret) - rwcount_max = (size_t)ret; + if (rw_args->n == 0) + rwcount_max = MIN((size_t)rw_args->stp, (size_t)ret); + else + rwcount_max = (size_t)ret; int_status = true; } diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 688adfc..1f59edf 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -181,10 +181,10 @@ Test(ceda_fdc, readCommandNoMedium) { 1, // cylinder 0, // head 1, // record - 1, // N - bytes per sector size factor + 0, // N - bytes per sector size factor 5, // EOT (end of track) 0, // GPL (ignored) - 0, // DTL (ignored) + 4, // DTL }; fdc_init(); @@ -229,10 +229,10 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head 5, // record - 1, // N - bytes per sector size factor + 0, // N - bytes per sector size factor 10, // EOT (end of track) 0, // GPL (ignored) - 0, // DTL (ignored) + 4, // DTL }, { 0, // Drive number, no error @@ -241,7 +241,7 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head 7, // record - 1, // N + 0, // N }, }, { @@ -253,10 +253,10 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 1, // head - different from physical head just for fun 9, // record - 1, // N - bytes per sector size factor + 0, // N - bytes per sector size factor 10, // EOT (end of track) 0, // GPL - 0, // DTL + 4, // DTL }, { 1, // drive number, no error @@ -265,7 +265,7 @@ static struct rw_test_params_t rwparams[] = { 8, // cylinder 1, // head 1, // record - 1, // N + 0, // N }, }, { @@ -277,10 +277,10 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head - different from physical head just for fun 5, // record - 1, // N - bytes per sector size factor + 0, // N - bytes per sector size factor 10, // EOT (end of track) 0, // GPL - 0, // DTL + 4, // DTL }, { FDC_ST0_HD | 2, // drive number, physical head 1, no error @@ -289,7 +289,7 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head 7, // record - 1, // N + 0, // N }, }, { @@ -301,10 +301,10 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 1, // head 9, // record - 1, // N - bytes per sector size factor + 0, // N - bytes per sector size factor 10, // EOT (end of track) 0, // GPL - 0, // DTL + 4, // DTL }, { FDC_ST0_HD | 3, // drive number, physical head 1, no error @@ -313,7 +313,7 @@ static struct rw_test_params_t rwparams[] = { 8, // cylinder 1, // head 1, // record - 1, // N + 0, // N }, }, /* * * * * * */ @@ -326,10 +326,10 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head 5, // record - 1, // N - bytes per sector size factor + 0, // N - bytes per sector size factor 10, // EOT (end of track) 0, // GPL - 0, // DTL + 4, // DTL }, { 3, // drive number, physical head 0, no error @@ -338,7 +338,7 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head 7, // record - 1, // N + 0, // N }, }, { @@ -350,10 +350,10 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 1, // head - different from physical head just for fun 9, // record - 1, // N - bytes per sector size factor + 0, // N - bytes per sector size factor 10, // EOT (end of track) 0, // GPL - 0, // DTL + 4, // DTL }, { FDC_ST0_HD | 2, // drive number, physical head 1, no error @@ -362,7 +362,7 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head 1, // record - 1, // N + 0, // N }, }, { @@ -374,10 +374,10 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head 5, // record - 1, // N - bytes per sector size factor + 0, // N - bytes per sector size factor 10, // EOT (end of track) 0, // GPL - 0, // DTL + 4, // DTL }, { FDC_ST0_HD | 1, // drive number, physical head 1, no error @@ -386,7 +386,7 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head 7, // record - 1, // N + 0, // N }, }, { @@ -398,10 +398,10 @@ static struct rw_test_params_t rwparams[] = { 7, // cylinder 0, // head - different from physical head just for fun 9, // record - 1, // N - bytes per sector size factor + 0, // N - bytes per sector size factor 10, // EOT (end of track) 0, // GPL - 0, // DTL + 4, // DTL }, { 0, // drive number, physical head 0, no error @@ -410,7 +410,7 @@ static struct rw_test_params_t rwparams[] = { 8, // cylinder 1, // head 1, // record - 1, // N + 0, // N }, }, }; From 7ab39c9a5ca995fa00cdfa78dec661ad8653ded1 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 8 Feb 2025 19:03:52 +0100 Subject: [PATCH 35/52] feat(fdc): implemented error handling for reading Added status registers to store error flags --- src/fdc.c | 107 ++++++++++++++++++++++++++----------------- src/tests/test_fdc.c | 62 +++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 43 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 5b2b78a..59c5525 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -187,8 +187,9 @@ static bool tc_status = false; static bool int_status = false; /* FDC internal registers */ +enum { MSR, ST0, ST1, ST2, ST3, NUM_OF_SREG }; // Main Status Register -static uint8_t status_register; +static uint8_t status_register[NUM_OF_SREG]; /* Floppy disk status */ // Current track position @@ -239,7 +240,7 @@ static void pre_exec_write_data(void) { LOG_DEBUG("DTL: %d\n", args[7]); // Set DIO to read for Execution phase - status_register &= (uint8_t)~FDC_ST_DIO; + status_register[MSR] &= (uint8_t)~FDC_ST_DIO; buffer_write_size(); } @@ -354,7 +355,13 @@ static void pre_exec_read_data(void) { LOG_DEBUG("DTL: %d\n", args[7]); // Set DIO to read for Execution phase - status_register |= FDC_ST_DIO; + status_register[MSR] |= FDC_ST_DIO; + // Clear errors on other status registers + // TODO put this in a function + status_register[ST0] = 0; + status_register[ST1] = 0; + status_register[ST2] = 0; + status_register[ST3] = 0; // TODO(giuliof) create handles to manage more than one floppy image at a // time @@ -401,12 +408,14 @@ static uint8_t exec_read_data(uint8_t value) { // Terminate execution if end of track is reached tc_status = true; rw_args->cylinder++; + status_register[ST0] = rw_args->unit_head; return 0; } } else { // Terminate execution if end of track is reached tc_status = true; rw_args->cylinder++; + status_register[ST0] = rw_args->unit_head; return 0; } } @@ -418,21 +427,15 @@ static uint8_t exec_read_data(uint8_t value) { static void post_exec_read_data(void) { rw_args_t *rw_args = (rw_args_t *)args; - uint8_t drive = rw_args->unit_head & FDC_ST0_US; LOG_DEBUG("Read has ended\n"); memset(result, 0x00, sizeof(result)); - // TODO(giuliof): populate result as in datasheet (see table 2) - /* ST0 */ - // Current head position - result[0] |= drive; - result[0] |= rw_args->unit_head & FDC_ST0_HD ? FDC_ST0_HD : 0; - /* ST1 */ - result[1] |= 0; // TODO(giuliof): populate this - /* ST2 */ - result[2] |= 0; // TODO(giuliof): populate this + /* Status registers 0-2 */ + result[0] |= status_register[ST0]; + result[1] |= status_register[ST1]; + result[2] |= status_register[ST2]; /* CHR */ result[3] = rw_args->cylinder; result[4] = rw_args->head; @@ -581,7 +584,7 @@ static void fdc_compute_next_status(void) { if (fdc_status == CMD) { // Set DIO to write for ARGS phase - status_register &= (uint8_t)~FDC_ST_DIO; + status_register[MSR] &= (uint8_t)~FDC_ST_DIO; fdc_status = ARGS; rwcount_max = fdc_currop->args_len; @@ -601,7 +604,7 @@ static void fdc_compute_next_status(void) { if (fdc_status == EXEC && (tc_status || fdc_currop->exec == NULL)) { tc_status = false; // Set DIO to read for RESULT phase - status_register |= FDC_ST_DIO; + status_register[MSR] |= FDC_ST_DIO; if (fdc_currop->post_exec) fdc_currop->post_exec(); @@ -613,7 +616,7 @@ static void fdc_compute_next_status(void) { if (fdc_status == RESULT && rwcount == rwcount_max) { // Set DIO to write for CMD and ARGS phases - status_register &= (uint8_t)~FDC_ST_DIO; + status_register[MSR] &= (uint8_t)~FDC_ST_DIO; fdc_status = CMD; rwcount_max = 0; @@ -622,14 +625,14 @@ static void fdc_compute_next_status(void) { // Update step dependant bits in main status register if (fdc_status == EXEC) - status_register |= FDC_ST_EXM; + status_register[MSR] |= FDC_ST_EXM; else - status_register &= (uint8_t)~FDC_ST_EXM; + status_register[MSR] &= (uint8_t)~FDC_ST_EXM; if (fdc_status != CMD) - status_register |= FDC_ST_CB; + status_register[MSR] |= FDC_ST_CB; else - status_register &= (uint8_t)~FDC_ST_CB; + status_register[MSR] &= (uint8_t)~FDC_ST_CB; } static void buffer_update(void) { @@ -638,9 +641,11 @@ static void buffer_update(void) { uint8_t sector = rw_args->record; - // Default: no data ready to be served + // Default: no data ready to be served, no error int_status = false; + rwcount = 0; rwcount_max = 0; + status_register[ST0] = rw_args->unit_head; // FDC counts sectors from 1 assert(sector != 0); @@ -651,34 +656,46 @@ static void buffer_update(void) { if (read_buffer_cb == NULL) return; - // TODO(giuliof): add proper error code, zero is no mounted image int ret = read_buffer_cb(NULL, drive, rw_args->unit_head & FDC_ST0_HD, track[drive], rw_args->head, rw_args->cylinder, sector); - if (ret != DISK_IMAGE_NOMEDIUM) { - // TODO(giuliof): At the moment we do not support error codes, we assume - // the image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); + if (ret > DISK_IMAGE_NOMEDIUM) { // Buffer is statically allocated, be sure that the data can fit it CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); ret = read_buffer_cb(exec_buffer, drive, rw_args->unit_head & FDC_ST0_HD, track[drive], rw_args->head, rw_args->cylinder, sector); - // TODO(giuliof): At the moment we do not support error codes, we assume - // the image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); - - // Ready to serve data - int_status = true; } - rwcount = 0; - if (rw_args->n == 0) - rwcount_max = MIN((size_t)rw_args->stp, (size_t)ret); - else - rwcount_max = (size_t)ret; + // No medium, FDC is in EXEC state until a disk is inserted, or manual + // termination + if (ret == DISK_IMAGE_NOMEDIUM) + return; + + // generate interrupt since an event has occurred + int_status = true; + + // Ready to serve data + if (ret > DISK_IMAGE_NOMEDIUM) { + if (rw_args->n == 0) + rwcount_max = MIN((size_t)rw_args->stp, (size_t)ret); + else + rwcount_max = (size_t)ret; + } + // Error condition + // TODO(giuliof): errors may be differentiated, but for the moment cath all + // as a generic error + else { + LOG_WARN("Reading error occurred, code %d\n", ret); + // Update status register setting error condition and error type flags + status_register[ST0] |= 0x40; + status_register[ST1] |= 0x20; + status_register[ST2] |= 0x20; + // Execution is terminated after an error + tc_status = true; + } } static void buffer_write_size(void) { @@ -733,7 +750,7 @@ void fdc_init(void) { // Reset main status register, but keep RQM active since FDC is always ready // to receive requests - status_register = FDC_ST_RQM; + status_register[MSR] = FDC_ST_RQM; // Reset track positions memset(track, 0, sizeof(track)); @@ -746,15 +763,16 @@ void fdc_init(void) { uint8_t fdc_in(ceda_ioaddr_t address) { switch (address & 0x01) { case FDC_ADDR_STATUS_REGISTER: - return status_register; + return status_register[MSR]; case FDC_ADDR_DATA_REGISTER: { uint8_t value = 0; if (fdc_status == CMD) { // You should never read when in CMD status. - // Just reply with "invalid command" and restore data direction - value = 0x80; - status_register &= (uint8_t)~FDC_ST_DIO; + // Just reply with ST0. + value = status_register[ST0]; + + status_register[MSR] &= (uint8_t)~FDC_ST_DIO; } else if (fdc_status == ARGS) { // you should never read during command phase LOG_WARN("FDC read access during ARGS phase!\n"); @@ -811,7 +829,10 @@ void fdc_out(ceda_ioaddr_t address, uint8_t value) { // Invalid command: set the main status register to read to // serve ST0 and error code - status_register |= FDC_ST_DIO; + status_register[ST0] &= (uint8_t)~FDC_ST0_IC; + status_register[ST0] |= 0x80; + + status_register[MSR] |= FDC_ST_DIO; } } else if (fdc_status == ARGS) { assert(rwcount < sizeof(args) / sizeof(*args)); diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 1f59edf..f40cb82 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -41,6 +41,20 @@ static int fake_read(uint8_t *buffer, uint8_t unit_number, bool phy_head, return 4; } +static int fake_wrong_rw(uint8_t *buffer, uint8_t unit_number, bool phy_head, + uint8_t phy_track, bool head, uint8_t track, + uint8_t sector) { + (void)buffer; + (void)unit_number; + (void)phy_head; + (void)phy_track; + (void)head; + (void)track; + (void)sector; + + return DISK_IMAGE_ERR; +} + Test(ceda_fdc, mainStatusRegisterWhenIdle) { fdc_init(); @@ -205,6 +219,54 @@ Test(ceda_fdc, readCommandNoMedium) { cr_assert_eq(fdc_getIntStatus(), true); } +Test(ceda_fdc, readCommandInvalidParams) { + const uint8_t arguments[8] = { + 0, // drive number + 1, // cylinder + 0, // head + 1, // record + 0, // N - bytes per sector size factor + 5, // EOT (end of track) + 0, // GPL (ignored) + 4, // DTL + }; + + const uint8_t expected_result[7] = { + 0x40, // Drive number, error code + 0x20, // ST1 + 0x20, // ST2 + 1, // cylinder + 0, // head + 1, // record + 0, // N + }; + + uint8_t result[sizeof(expected_result)]; + + fdc_init(); + + // Link a fake reading function + fdc_kickDiskImage(fake_wrong_rw, NULL); + + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_READ_DATA); + + // Send arguments checking for no error + sendBuffer(arguments, sizeof(arguments)); + + // FDC generates an interrupt + cr_assert_eq(fdc_getIntStatus(), true); + + // FDC is NOT in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); + + receiveBuffer(result, sizeof(result)); + + cr_assert_arr_eq(result, expected_result, sizeof(result)); + + // Execution is finished + assert_fdc_sr(FDC_ST_RQM); +} + /** * @brief This section covers the cases described in table 2-2 of xxxxxxx * datasheet. From 978b68731752b8985a702ae6edf32ef1107ec7a8 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 8 Feb 2025 19:30:18 +0100 Subject: [PATCH 36/52] fix(fdc): format result implementation was missing --- src/fdc.c | 12 ++++++++++++ src/tests/test_fdc.c | 14 +++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/fdc.c b/src/fdc.c index 59c5525..17d5675 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -496,6 +496,12 @@ static void pre_exec_format_track(void) { LOG_DEBUG("GPL: %d\n", args[3]); LOG_DEBUG("D: %d\n", args[4]); + // Set deafult values of status registers + status_register[ST0] = format_args->unit_head; + status_register[ST1] = 0; + status_register[ST2] = 0; + status_register[ST3] = 0; + // Initialize execution phase counter. // The FORMAT command requires the filling of a buffer of "ID fields", one // for each sector within the same track. Each "ID field" is 4 bytes long. @@ -556,6 +562,12 @@ static void post_exec_format_track(void) { LOG_DEBUG("FDC end Format track\n"); memset(result, 0, sizeof(result)); + + /* Status registers 0-2 */ + result[0] |= status_register[ST0]; + result[1] |= status_register[ST1]; + result[2] |= status_register[ST2]; + /* CHR and N have no meaning */ } // Seek diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index f40cb82..80e1e55 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -595,7 +595,6 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, writeCommand0) { } Test(ceda_fdc, formatCommand) { - uint8_t result[7]; uint8_t arguments[] = { 0x01 | FDC_ST0_HD, 0x01, @@ -604,6 +603,16 @@ Test(ceda_fdc, formatCommand) { 0x35, // fill byte }; + const uint8_t expected_result[] = { + 0x01 | FDC_ST0_HD, // Drive number + // 0x0, // ST0 + // 0x0, // ST1 + // 0x0, // ST2 + // CHR and N have no meaning here + }; + + uint8_t result[7]; + fdc_init(); // Link a fake reading function @@ -640,6 +649,9 @@ Test(ceda_fdc, formatCommand) { receiveBuffer(result, sizeof(result)); + // CHR and N are ignored! + cr_assert_arr_eq(result, expected_result, sizeof(expected_result)); + // Execution is finished assert_fdc_sr(FDC_ST_RQM); } From 3d3e0c83650907e21ec0e37813e8195358edf7c3 Mon Sep 17 00:00:00 2001 From: giuliof Date: Mon, 10 Feb 2025 18:34:01 +0100 Subject: [PATCH 37/52] feat(fdc): introduced error handling in write and format --- src/fdc.c | 118 +++++++++++++++++++++++++++++++------------ src/tests/test_fdc.c | 90 +++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+), 33 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 17d5675..6e4b37e 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -241,6 +241,12 @@ static void pre_exec_write_data(void) { // Set DIO to read for Execution phase status_register[MSR] &= (uint8_t)~FDC_ST_DIO; + // Clear errors on other status registers + // TODO put this in a function + status_register[ST0] = 0; + status_register[ST1] = 0; + status_register[ST2] = 0; + status_register[ST3] = 0; buffer_write_size(); } @@ -272,10 +278,23 @@ static uint8_t exec_write_data(uint8_t value) { int ret = write_buffer_cb(exec_buffer, drive, rw_args->unit_head & FDC_ST0_HD, track[drive], rw_args->head, rw_args->cylinder, sector); - // the image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); - // Buffer is statically allocated, be sure that the data can fit it - CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); + + // Error condition + // TODO(giuliof): errors may be differentiated, but for the moment cath all + // as a generic error -- even if no medium because it's too late to handle + // by pausing execution! + if (ret <= DISK_IMAGE_NOMEDIUM) { + LOG_WARN("Reading error occurred, code %d\n", ret); + // Update status register setting error condition and error type flags + status_register[ST0] |= 0x40; + status_register[ST1] |= 0x20; + status_register[ST2] |= 0x20; + // Execution is terminated after an error + tc_status = true; + // Force an interrupt + int_status = true; + return 0; + } // Multi-sector mode (enabled by default). // If read is not interrupted at the end of the sector, the next logical @@ -297,12 +316,14 @@ static uint8_t exec_write_data(uint8_t value) { // Terminate execution if end of track is reached tc_status = true; rw_args->cylinder++; + status_register[ST0] = rw_args->unit_head; return 0; } } else { // Terminate execution if end of track is reached tc_status = true; rw_args->cylinder++; + status_register[ST0] = rw_args->unit_head; return 0; } } @@ -314,21 +335,15 @@ static uint8_t exec_write_data(uint8_t value) { static void post_exec_write_data(void) { rw_args_t *rw_args = (rw_args_t *)args; - uint8_t drive = rw_args->unit_head & FDC_ST0_US; LOG_DEBUG("Write has ended\n"); memset(result, 0x00, sizeof(result)); - // TODO(giuliof): populate result as in datasheet (see table 2) - /* ST0 */ - // Current head position - result[0] |= drive; - result[0] |= rw_args->unit_head & FDC_ST0_HD ? FDC_ST0_HD : 0; - /* ST1 */ - result[1] |= 0; // TODO(giuliof): populate this - /* ST2 */ - result[2] |= 0; // TODO(giuliof): populate this + /* Status registers 0-2 */ + result[0] |= status_register[ST0]; + result[1] |= status_register[ST1]; + result[2] |= status_register[ST2]; /* CHR */ result[3] = rw_args->cylinder; result[4] = rw_args->head; @@ -516,12 +531,19 @@ static void pre_exec_format_track(void) { return; // Check if medium is valid by poking sector 0 of the desired track - // TODO(giuliof): add proper error code, zero is no mounted image int ret = write_buffer_cb(NULL, drive, phy_head, track[drive], phy_head, track[drive], 0); - if (ret <= DISK_IMAGE_NOMEDIUM) - return; + if (ret <= DISK_IMAGE_NOMEDIUM) { + LOG_WARN("Format error occurred, code %d\n", ret); + // Update status register setting error condition and error type + // flags + status_register[ST0] |= 0x40; + status_register[ST1] |= 0x20; + status_register[ST2] |= 0x20; + // Execution is terminated after an error + tc_status = true; + } int_status = true; } @@ -549,14 +571,27 @@ static void post_exec_format_track(void) { int ret = write_buffer_cb(NULL, drive, phy_head, track[drive], head, cylinder, record); - CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); - uint8_t format_buffer[ret]; - memset(format_buffer, format_args->d, (size_t)ret); + if (ret > DISK_IMAGE_NOMEDIUM) { + uint8_t format_buffer[ret]; + memset(format_buffer, format_args->d, (size_t)ret); - ret = write_buffer_cb(format_buffer, drive, phy_head, track[drive], - head, cylinder, record); - CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); + ret = write_buffer_cb(format_buffer, drive, phy_head, track[drive], + head, cylinder, record); + } + + if (ret <= DISK_IMAGE_NOMEDIUM) { + LOG_WARN("Format error occurred, code %d\n", ret); + // Update status register setting error condition and error type + // flags + status_register[ST0] |= 0x40; + status_register[ST1] |= 0x20; + status_register[ST2] |= 0x20; + // Execution is terminated after an error + tc_status = true; + // Force an interrupt + int_status = true; + } } LOG_DEBUG("FDC end Format track\n"); @@ -718,7 +753,9 @@ static void buffer_write_size(void) { // Default: no data ready to be served int_status = false; + rwcount = 0; rwcount_max = 0; + status_register[ST0] = rw_args->unit_head; // FDC counts sectors from 1 assert(sector != 0); @@ -733,18 +770,33 @@ static void buffer_write_size(void) { write_buffer_cb(NULL, drive, rw_args->unit_head & FDC_ST0_HD, track[drive], rw_args->head, rw_args->cylinder, sector); - // TODO(giuliof): At the moment we do not support error codes, we assume the - // image is always loaded and valid - CEDA_STRONG_ASSERT_TRUE(ret > DISK_IMAGE_NOMEDIUM); - // Buffer is statically allocated, be sure that the data can fit it - CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); + // No medium, FDC is in EXEC state until a disk is inserted, or manual + // termination + if (ret == DISK_IMAGE_NOMEDIUM) + return; - rwcount = 0; - if (rw_args->n == 0) - rwcount_max = MIN((size_t)rw_args->stp, (size_t)ret); - else - rwcount_max = (size_t)ret; + // generate interrupt since an event has occurred int_status = true; + + // Ready to serve data + if (ret > DISK_IMAGE_NOMEDIUM) { + if (rw_args->n == 0) + rwcount_max = MIN((size_t)rw_args->stp, (size_t)ret); + else + rwcount_max = (size_t)ret; + } + // Error condition + // TODO(giuliof): errors may be differentiated, but for the moment cath all + // as a generic error + else { + LOG_WARN("Reading error occurred, code %d\n", ret); + // Update status register setting error condition and error type flags + status_register[ST0] |= 0x40; + status_register[ST1] |= 0x20; + status_register[ST2] |= 0x20; + // Execution is terminated after an error + tc_status = true; + } } /* * * * * * * * * * * * * * * Public routines * * * * * * * * * * * * * * */ diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 80e1e55..b3a0117 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -594,6 +594,54 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, writeCommand0) { assert_fdc_sr(FDC_ST_RQM); } +Test(ceda_fdc, writeCommandInvalidParams) { + const uint8_t arguments[8] = { + 1, // drive number + 1, // cylinder + 0, // head + 1, // record + 0, // N - bytes per sector size factor + 5, // EOT (end of track) + 0, // GPL (ignored) + 4, // DTL + }; + + const uint8_t expected_result[7] = { + 0x41, // Drive number, error code + 0x20, // ST1 + 0x20, // ST2 + 1, // cylinder + 0, // head + 1, // record + 0, // N + }; + + uint8_t result[sizeof(expected_result)]; + + fdc_init(); + + // Link a fake reading function + fdc_kickDiskImage(NULL, fake_wrong_rw); + + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_WRITE_DATA); + + // Send arguments checking for no error + sendBuffer(arguments, sizeof(arguments)); + + // FDC generates an interrupt + cr_assert_eq(fdc_getIntStatus(), true); + + // FDC is NOT in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); + + receiveBuffer(result, sizeof(result)); + + cr_assert_arr_eq(result, expected_result, sizeof(result)); + + // Execution is finished + assert_fdc_sr(FDC_ST_RQM); +} + Test(ceda_fdc, formatCommand) { uint8_t arguments[] = { 0x01 | FDC_ST0_HD, @@ -656,6 +704,48 @@ Test(ceda_fdc, formatCommand) { assert_fdc_sr(FDC_ST_RQM); } +Test(ceda_fdc, formatCommandInvalidParams) { + uint8_t arguments[] = { + 0x03 | FDC_ST0_HD, // drive number + 0x01, + 0x02, // two sectors per track + 0x00, // gap (we don't care) + 0x35, // fill byte + }; + + const uint8_t expected_result[] = { + 0x43 | FDC_ST0_HD, // Drive number, error code + 0x20, // ST1 + 0x20, // ST2 + }; + + uint8_t result[7]; + + fdc_init(); + + // Link a fake reading function + fdc_kickDiskImage(NULL, fake_wrong_rw); + + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_FORMAT_TRACK); + + // Send arguments checking for no error + sendBuffer(arguments, sizeof(arguments)); + + // FDC has generated an interrupt + cr_assert_eq(fdc_getIntStatus(), true); + + // FDC is not in execution mode (error) + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); + + receiveBuffer(result, sizeof(result)); + + // CHR and N are ignored! + cr_assert_arr_eq(result, expected_result, sizeof(expected_result)); + + // Execution is finished + assert_fdc_sr(FDC_ST_RQM); +} + Test(ceda_fdc, invalidCommand) { uint8_t st0; From 921b18c88b0b229facc31b58064652150cfcd3a7 Mon Sep 17 00:00:00 2001 From: giuliof Date: Tue, 11 Feb 2025 09:00:47 +0100 Subject: [PATCH 38/52] fix(fdc): interrupt signal deassertion See tests, interrupt must be deasserted in result phase or when signalling an invalid command --- src/fdc.c | 5 +++++ src/tests/test_fdc.c | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/fdc.c b/src/fdc.c index 6e4b37e..231899a 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -837,6 +837,9 @@ uint8_t fdc_in(ceda_ioaddr_t address) { value = status_register[ST0]; status_register[MSR] &= (uint8_t)~FDC_ST_DIO; + + // remove interrupt condition + int_status = false; } else if (fdc_status == ARGS) { // you should never read during command phase LOG_WARN("FDC read access during ARGS phase!\n"); @@ -851,6 +854,8 @@ uint8_t fdc_in(ceda_ioaddr_t address) { } else if (fdc_status == RESULT) { assert(rwcount < sizeof(result) / sizeof(*result)); value = result[rwcount]; + // ... + int_status = false; } fdc_compute_next_status(); diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index b3a0117..904b1a0 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -117,6 +117,9 @@ Test(ceda_fdc, seekCommand) { // Second argument is cylinder position fdc_out(FDC_ADDR_DATA_REGISTER, 5); + // Seek raises an interrupt and expects SENSE_INTERRUPT command + cr_assert_eq(fdc_getIntStatus(), true); + // FDC is no more busy assert_fdc_sr(FDC_ST_RQM); @@ -138,6 +141,9 @@ Test(ceda_fdc, seekCommand) { // specified by the seek argument data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, 5); + + // No interrupt must be present after result phase + cr_assert_eq(fdc_getIntStatus(), false); } Test(ceda_fdc, invalidSeekSequence) { @@ -165,6 +171,9 @@ Test(ceda_fdc, invalidSeekSequence) { assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO); data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, 0x80); + + // No interrupt must be present after an invalid command + cr_assert_eq(fdc_getIntStatus(), false); } /** @@ -526,6 +535,9 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, readCommand0) { // Execution is finished assert_fdc_sr(FDC_ST_RQM); + + // No interrupt must be present after result phase + cr_assert_eq(fdc_getIntStatus(), false); } static int fake_write(uint8_t *buffer, uint8_t unit_number, bool phy_head, @@ -592,6 +604,9 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, writeCommand0) { // Execution is finished assert_fdc_sr(FDC_ST_RQM); + + // No interrupt must be present after result phase + cr_assert_eq(fdc_getIntStatus(), false); } Test(ceda_fdc, writeCommandInvalidParams) { @@ -744,6 +759,9 @@ Test(ceda_fdc, formatCommandInvalidParams) { // Execution is finished assert_fdc_sr(FDC_ST_RQM); + + // No interrupt must be present after result phase + cr_assert_eq(fdc_getIntStatus(), false); } Test(ceda_fdc, invalidCommand) { @@ -762,4 +780,7 @@ Test(ceda_fdc, invalidCommand) { // TODO(giuliof): this is actually unclear, the direction has to be set // back? probably yes assert_fdc_sr(FDC_ST_RQM); + + // No interrupt must be present after result phase + cr_assert_eq(fdc_getIntStatus(), false); } From 9be9ccbc21aa7cd7f57ae4840ecb65d196b8db35 Mon Sep 17 00:00:00 2001 From: giuliof Date: Tue, 11 Feb 2025 18:27:45 +0100 Subject: [PATCH 39/52] refactor: typo --- src/fdc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 231899a..331a1ea 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -48,7 +48,7 @@ typedef struct rw_args_t { uint8_t n; uint8_t eot; uint8_t gpl; - uint8_t stp; + uint8_t dtl; } rw_args_t; // Parsing structure for format arguments @@ -727,7 +727,7 @@ static void buffer_update(void) { // Ready to serve data if (ret > DISK_IMAGE_NOMEDIUM) { if (rw_args->n == 0) - rwcount_max = MIN((size_t)rw_args->stp, (size_t)ret); + rwcount_max = MIN((size_t)rw_args->dtl, (size_t)ret); else rwcount_max = (size_t)ret; } @@ -781,7 +781,7 @@ static void buffer_write_size(void) { // Ready to serve data if (ret > DISK_IMAGE_NOMEDIUM) { if (rw_args->n == 0) - rwcount_max = MIN((size_t)rw_args->stp, (size_t)ret); + rwcount_max = MIN((size_t)rw_args->dtl, (size_t)ret); else rwcount_max = (size_t)ret; } From dadc6978fee11486a9803a00e27b7e96ae0e91b0 Mon Sep 17 00:00:00 2001 From: giuliof Date: Tue, 11 Feb 2025 21:10:59 +0100 Subject: [PATCH 40/52] fix(fdc): seek and recalibrate set drive number in ST0 - Removed one TODO with kinda working implementation - Improved seek test by removing edge case - Can be further improved, ad the moment sequential seek/recalibrate without interleaved sense interrupt are not supported --- src/fdc.c | 15 +++++++-------- src/tests/test_fdc.c | 4 ++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 331a1ea..b612bd2 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -471,24 +471,21 @@ static void pre_exec_recalibrate(void) { // We don't have to actually move the head. The drive is immediately ready int_status = true; + // Update the status register with the drive info and the seek end flag + status_register[ST0] = drive; } // Sense interrupt: static void post_exec_sense_interrupt(void) { - // TODO(giuliof): last accessed drive - uint8_t drive = 0; + // Last accessed drive number is in ST0 + uint8_t drive = status_register[ST0] & FDC_ST0_US; // After reading interrupt status, ready can be deasserted int_status = false; LOG_DEBUG("FDC Sense Interrupt\n"); /* Status Register 0 */ - // Drive number - result[0] = drive; - // head address (last addressed) - TODO(giulio) - // result[0] |= ...; - // Seek End - TODO(giulio) - result[0] |= FDC_ST0_SE; + result[0] = status_register[ST0] | FDC_ST0_SE; /* PCN - (current track position) */ result[1] = track[drive]; } @@ -617,6 +614,8 @@ static void pre_exec_seek(void) { // We don't have to actually move the head. The drive is immediately ready int_status = true; + // Update the status register with the drive info and the seek end flag + status_register[ST0] = drive; } /* * * * * * * * * * * * * * * Utility routines * * * * * * * * * * * * * * */ diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 904b1a0..af7b793 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -113,7 +113,7 @@ Test(ceda_fdc, seekCommand) { assert_fdc_sr(FDC_ST_RQM | FDC_ST_CB); // First argument is number of drive - fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); + fdc_out(FDC_ADDR_DATA_REGISTER, 0x02); // Second argument is cylinder position fdc_out(FDC_ADDR_DATA_REGISTER, 5); @@ -132,7 +132,7 @@ Test(ceda_fdc, seekCommand) { // First response byte is SR0 with interrupt code = 0 and Seek End = 1 data = fdc_in(FDC_ADDR_DATA_REGISTER); - cr_expect_eq(data, FDC_ST0_SE); + cr_expect_eq(data, FDC_ST0_SE | 2); // FDC has another byte of response assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); From 3ac5707e8ded071b3a611866c76c2c165a1ba2cf Mon Sep 17 00:00:00 2001 From: giuliof Date: Sun, 16 Feb 2025 22:17:40 +0100 Subject: [PATCH 41/52] test(fdc): swapped read register/poll INT operation order This is the proper execution order expected by the FDC, else the INT would be cleared by read operation --- src/tests/test_fdc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index af7b793..2027ed2 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -504,12 +504,12 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, readCommand0) { // Send arguments checking for no error sendBuffer(param->arguments, sizeof(param->arguments)); - // FDC is in execution mode - assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); - // FDC is ready to serve data cr_assert_eq(fdc_getIntStatus(), true); + // FDC is in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); + // Read two full sectors fdc_in(FDC_ADDR_DATA_REGISTER); fdc_in(FDC_ADDR_DATA_REGISTER); @@ -573,12 +573,12 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, writeCommand0) { // Send arguments checking for no error sendBuffer(param->arguments, sizeof(param->arguments)); - // FDC is in execution mode - assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); - // FDC is ready to receive data cr_assert_eq(fdc_getIntStatus(), true); + // FDC is in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); + // Read two full sectors fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); @@ -686,12 +686,12 @@ Test(ceda_fdc, formatCommand) { // Send arguments checking for no error sendBuffer(arguments, sizeof(arguments)); - // FDC is in execution mode - assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); - // FDC is ready to receive data cr_assert_eq(fdc_getIntStatus(), true); + // FDC is in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); + // First sector ID fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); fdc_out(FDC_ADDR_DATA_REGISTER, 0x01); From afd41104dd7b301a15149f1cbfaecf93986e5819 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sun, 16 Feb 2025 22:22:15 +0100 Subject: [PATCH 42/52] fix(fdc): proper reset of INT flag This temporarily breaks one test, but will be fixed soon --- src/fdc.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index b612bd2..4e8b75f 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -480,9 +480,6 @@ static void post_exec_sense_interrupt(void) { // Last accessed drive number is in ST0 uint8_t drive = status_register[ST0] & FDC_ST0_US; - // After reading interrupt status, ready can be deasserted - int_status = false; - LOG_DEBUG("FDC Sense Interrupt\n"); /* Status Register 0 */ result[0] = status_register[ST0] | FDC_ST0_SE; @@ -687,8 +684,6 @@ static void buffer_update(void) { uint8_t sector = rw_args->record; - // Default: no data ready to be served, no error - int_status = false; rwcount = 0; rwcount_max = 0; status_register[ST0] = rw_args->unit_head; @@ -750,8 +745,6 @@ static void buffer_write_size(void) { uint8_t sector = rw_args->record; - // Default: no data ready to be served - int_status = false; rwcount = 0; rwcount_max = 0; status_register[ST0] = rw_args->unit_head; @@ -824,6 +817,9 @@ void fdc_init(void) { } uint8_t fdc_in(ceda_ioaddr_t address) { + // The interrupt is cleared by reading/writing data to the FDC + int_status = false; + switch (address & 0x01) { case FDC_ADDR_STATUS_REGISTER: return status_register[MSR]; @@ -836,9 +832,6 @@ uint8_t fdc_in(ceda_ioaddr_t address) { value = status_register[ST0]; status_register[MSR] &= (uint8_t)~FDC_ST_DIO; - - // remove interrupt condition - int_status = false; } else if (fdc_status == ARGS) { // you should never read during command phase LOG_WARN("FDC read access during ARGS phase!\n"); @@ -853,8 +846,6 @@ uint8_t fdc_in(ceda_ioaddr_t address) { } else if (fdc_status == RESULT) { assert(rwcount < sizeof(result) / sizeof(*result)); value = result[rwcount]; - // ... - int_status = false; } fdc_compute_next_status(); @@ -867,6 +858,9 @@ uint8_t fdc_in(ceda_ioaddr_t address) { } void fdc_out(ceda_ioaddr_t address, uint8_t value) { + // The interrupt is cleared by reading/writing data to the FDC + int_status = false; + switch (address & 0x01) { case FDC_ADDR_STATUS_REGISTER: LOG_WARN("nobody should write in FDC main status register\n"); From 58a984a38c75570ac2268235f835b4bb1b5b66f3 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 22 Feb 2025 22:47:13 +0100 Subject: [PATCH 43/52] fix(fdc): corrected implementation of seek/recalibrate/sense int --- src/fdc.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 4e8b75f..76e0233 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -473,14 +473,29 @@ static void pre_exec_recalibrate(void) { int_status = true; // Update the status register with the drive info and the seek end flag status_register[ST0] = drive; + // Update the FDD n busy flag, will be cleared by sense interrupt + status_register[MSR] |= (1 << drive); } // Sense interrupt: static void post_exec_sense_interrupt(void) { - // Last accessed drive number is in ST0 - uint8_t drive = status_register[ST0] & FDC_ST0_US; - LOG_DEBUG("FDC Sense Interrupt\n"); + + // Get the last "seeked" drive number from the MSR + uint8_t fdc_busy = status_register[MSR] & + (FDC_ST_D3B | FDC_ST_D2B | FDC_ST_D1B | FDC_ST_D0B); + // This routine should be called only if fdc is busy + assert(fdc_busy != 0); + uint8_t drive = 0; + for (; (fdc_busy & (1 << drive)) == 0; drive++) + ; + + // Deassert busy state and eventually retrigger INT (TODO: verify) + status_register[MSR] &= (uint8_t) ~(1 << drive); + if (status_register[MSR] & + (FDC_ST_D3B | FDC_ST_D2B | FDC_ST_D1B | FDC_ST_D0B)) + int_status = true; + /* Status Register 0 */ result[0] = status_register[ST0] | FDC_ST0_SE; /* PCN - (current track position) */ @@ -613,6 +628,8 @@ static void pre_exec_seek(void) { int_status = true; // Update the status register with the drive info and the seek end flag status_register[ST0] = drive; + // Update the FDD n busy flag, will be cleared by sense interrupt + status_register[MSR] |= (1 << drive); } /* * * * * * * * * * * * * * * Utility routines * * * * * * * * * * * * * * */ From 327610a28734f93beb91067f5e600c8ff9af9423 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 22 Feb 2025 22:48:50 +0100 Subject: [PATCH 44/52] fix(fdc): wrong implementation of out of sequence commands e.g. Sense Interrupt that is not following Seek --- src/fdc.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/fdc.c b/src/fdc.c index 76e0233..d803177 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -76,6 +76,7 @@ static uint8_t exec_format_track(uint8_t value); static void post_exec_format_track(void); static void pre_exec_seek(void); /* Utility routines prototypes */ +static bool is_cmd_out_of_sequence(uint8_t cmd); static void fdc_compute_next_status(void); // Update read buffer with the data from current ths static void buffer_update(void); @@ -634,6 +635,27 @@ static void pre_exec_seek(void) { /* * * * * * * * * * * * * * * Utility routines * * * * * * * * * * * * * * */ +static bool is_cmd_out_of_sequence(uint8_t cmd) { + bool fdc_busy = status_register[MSR] & + ((FDC_ST_D3B | FDC_ST_D2B | FDC_ST_D1B | FDC_ST_D0B)); + + if (cmd == FDC_SEEK || cmd == FDC_RECALIBRATE) + return false; + // TODO: to be correct, I should check int, but it was already + // cleared in command read/write routine + else if (cmd == FDC_SENSE_INTERRUPT) { + if (fdc_busy) + return false; + } + // + else { + if (!fdc_busy) + return false; + } + + return true; +} + static void fdc_compute_next_status(void) { if (!fdc_currop) return; @@ -893,7 +915,7 @@ void fdc_out(ceda_ioaddr_t address, uint8_t value) { // recalibrate) and next command is not sense interrupt. // In this case, the command is treated as invalid. fdc_currop = NULL; - if (!(int_status && cmd != FDC_SENSE_INTERRUPT)) { + if (!is_cmd_out_of_sequence(cmd)) { for (size_t i = 0; i < sizeof(fdc_operations) / sizeof(*fdc_operations); i++) { @@ -903,6 +925,7 @@ void fdc_out(ceda_ioaddr_t address, uint8_t value) { } } } + if (fdc_currop == NULL) { LOG_WARN("Command %x is not implemented\n", cmd); From 283064c691924cbe45c494e45192962b1aa63a5f Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 8 Mar 2025 18:08:56 +0100 Subject: [PATCH 45/52] test(fdc): fixed test patterns for read and write, handled error conditions - terminal count is always required, read and write operation never stop by themselves (qui pro quo from the readq track command that seems to stop at the end of track? To be verified, not in tests). - Maybe, the error codes are not be fully true, but the CHS values should be fine. - Tests now give error since the implementation is not fully correct. See next commit. --- src/tests/test_fdc.c | 123 +++++++++++++++++++++++++++++++++---------- 1 file changed, 96 insertions(+), 27 deletions(-) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index 2027ed2..e12c5a3 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -1,6 +1,7 @@ #include #include +#include // TODO source path is src! #include "../fdc.h" @@ -55,6 +56,23 @@ static int fake_wrong_rw(uint8_t *buffer, uint8_t unit_number, bool phy_head, return DISK_IMAGE_ERR; } +static int fake_read_check_track(uint8_t *buffer, uint8_t unit_number, + bool phy_head, uint8_t phy_track, bool head, + uint8_t track, uint8_t sector) { + + (void)buffer; + (void)unit_number; + (void)phy_head; + (void)head; + (void)sector; + + if (phy_track != track) { + return DISK_IMAGE_INVALID_GEOMETRY; + } + + return 4; +} + Test(ceda_fdc, mainStatusRegisterWhenIdle) { fdc_init(); @@ -79,6 +97,8 @@ Test(ceda_fdc, specifyCommand) { assert_fdc_sr(FDC_ST_RQM); } +// This test is not valid! +#if 0 Test(ceda_fdc, senseInterruptStatusCommand) { fdc_init(); @@ -101,6 +121,7 @@ Test(ceda_fdc, senseInterruptStatusCommand) { data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, 0); } +#endif Test(ceda_fdc, seekCommand) { fdc_init(); @@ -121,7 +142,7 @@ Test(ceda_fdc, seekCommand) { cr_assert_eq(fdc_getIntStatus(), true); // FDC is no more busy - assert_fdc_sr(FDC_ST_RQM); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_D2B); // A sense interrupt command is expected after FDC_SEEK fdc_out(FDC_ADDR_DATA_REGISTER, FDC_SENSE_INTERRUPT); @@ -162,13 +183,13 @@ Test(ceda_fdc, invalidSeekSequence) { fdc_out(FDC_ADDR_DATA_REGISTER, 7); // FDC is no more busy - assert_fdc_sr(FDC_ST_RQM); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_D0B); // Send another command that is not FDC_SENSE_INTERRUPT fdc_out(FDC_ADDR_DATA_REGISTER, FDC_SPECIFY); // FDC does not process this command and asserts invalid command - assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_D0B); data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, 0x80); @@ -228,6 +249,7 @@ Test(ceda_fdc, readCommandNoMedium) { cr_assert_eq(fdc_getIntStatus(), true); } +// 20 20? Test(ceda_fdc, readCommandInvalidParams) { const uint8_t arguments[8] = { 0, // drive number @@ -276,6 +298,66 @@ Test(ceda_fdc, readCommandInvalidParams) { assert_fdc_sr(FDC_ST_RQM); } +Test(ceda_fdc, readCommandOverEot) { + const uint8_t arguments[8] = { + 0, // drive number + 0, // cylinder + 0, // head + 6, // record + 0, // N - bytes per sector size factor + 6, // EOT (end of track) + 0, // GPL (ignored) + 4, // DTL + }; + + const uint8_t expected_result[7] = { + 0x40, // Drive number, error code + 0x20, // ST1 + 0x20, // ST2 + 0, // cylinder + 0, // head + 6, // record + 0, // N + }; + + uint8_t result[sizeof(expected_result)]; + + fdc_init(); + + // Link a fake reading function + fdc_kickDiskImage(fake_read_check_track, NULL); + + fdc_out(FDC_ADDR_DATA_REGISTER, FDC_READ_DATA); + + // Send arguments checking for no error + sendBuffer(arguments, sizeof(arguments)); + + // FDC generates an interrupt + cr_assert_eq(fdc_getIntStatus(), true); + + // Read sector 6 + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); + fdc_in(FDC_ADDR_DATA_REGISTER); + + // Try to read sector beyond EOT + fdc_in(FDC_ADDR_DATA_REGISTER); + + // FDC generates an interrupt + cr_assert_eq(fdc_getIntStatus(), true); + + // FDC is NOT in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); + + receiveBuffer(result, sizeof(result)); + + cr_assert_arr_eq(result, expected_result, sizeof(result)); + + // Execution is finished + assert_fdc_sr(FDC_ST_RQM); +} + /** * @brief This section covers the cases described in table 2-2 of xxxxxxx * datasheet. @@ -285,7 +367,6 @@ Test(ceda_fdc, readCommandInvalidParams) { struct rw_test_params_t { uint8_t cmd_alteration; - bool tc_required; uint8_t arguments[8]; uint8_t result[7]; }; @@ -293,8 +374,7 @@ struct rw_test_params_t { static struct rw_test_params_t rwparams[] = { { // No MT, end record < EOT, physical head 0 - 0, // No alteration - true, // Terminal count required + 0, // No alteration { 0, // drive number 7, // cylinder @@ -318,7 +398,6 @@ static struct rw_test_params_t rwparams[] = { { // No MT, end record = EOT, physical head 0 0, - false, // Terminal count not required { 1, // drive number 7, // cylinder @@ -342,7 +421,6 @@ static struct rw_test_params_t rwparams[] = { { // No MT, end record < EOT, physical head 1 0, - true, // Terminal count required { FDC_ST0_HD | 2, // Drive number, physical head 1 7, // cylinder @@ -366,7 +444,6 @@ static struct rw_test_params_t rwparams[] = { { // No MT, end record = EOT, physical head 1 0, - false, // Terminal count not required { FDC_ST0_HD | 3, // Drive number, physical head 1 7, // cylinder @@ -391,7 +468,6 @@ static struct rw_test_params_t rwparams[] = { { // MT (multi-track), end record < EOT, physical head 0 FDC_CMD_ARGS_MT_bm, - true, // Terminal count required { 3, // Drive number 7, // cylinder @@ -415,7 +491,6 @@ static struct rw_test_params_t rwparams[] = { { // MT (multi-track), end record = EOT, physical head 0 FDC_CMD_ARGS_MT_bm, - true, // Terminal count required { 2, // Drive number 7, // cylinder @@ -436,10 +511,10 @@ static struct rw_test_params_t rwparams[] = { 0, // N }, }, +#if 1 { // MT (multi-track), end record < EOT, physical head 1 FDC_CMD_ARGS_MT_bm, - true, // Terminal count required { FDC_ST0_HD | 1, // Drive number, physical head 1 7, // cylinder @@ -463,7 +538,6 @@ static struct rw_test_params_t rwparams[] = { { // MT (multi-track), end record = EOT, physical head 1 FDC_CMD_ARGS_MT_bm, - false, // Terminal count not required { FDC_ST0_HD | 0, // Drive number, physical head 1 7, // cylinder @@ -484,6 +558,7 @@ static struct rw_test_params_t rwparams[] = { 0, // N }, }, +#endif }; ParameterizedTestParameters(ceda_fdc, readCommand0) { @@ -521,13 +596,10 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, readCommand0) { fdc_in(FDC_ADDR_DATA_REGISTER); fdc_in(FDC_ADDR_DATA_REGISTER); - // Stop the reading, if needed - if (param->tc_required) { - // FDC is still in execution mode - assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); - // Request execution termination - fdc_tc_out(0, 0); - } + // FDC is still in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_EXM | FDC_ST_CB); + // Request execution termination + fdc_tc_out(0, 0); receiveBuffer(result, sizeof(result)); @@ -590,13 +662,10 @@ ParameterizedTest(struct rw_test_params_t *param, ceda_fdc, writeCommand0) { fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); - // Stop the reading, if needed - if (param->tc_required) { - // FDC is still in execution mode - assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); - // Request execution termination - fdc_tc_out(0, 0); - } + // FDC is still in execution mode + assert_fdc_sr(FDC_ST_RQM | FDC_ST_EXM | FDC_ST_CB); + // Request execution termination + fdc_tc_out(0, 0); receiveBuffer(result, sizeof(result)); From d2352b9f557d43a8dfad9b522e6e111f53333c51 Mon Sep 17 00:00:00 2001 From: giuliof Date: Sat, 8 Mar 2025 18:15:37 +0100 Subject: [PATCH 46/52] fix(fdc): read/write CHS result generation in error condition are now compliant Introduced IDR (ID Register) and next IDR to handle the result phase in both success and error condition. Briefly, when one or more sectors are succesfully read, IDR is incremented to the next one (handling head/track overflow). If an error occur, the CHS in result are referred to the latest correctly read sector. Tested on the actual FDC device. --- src/fdc.c | 392 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 220 insertions(+), 172 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index d803177..bc650c6 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -60,6 +60,14 @@ typedef struct format_args_t { uint8_t d; // filler byte } format_args_t; +// ID Register, tracks the current ID during rw operations +typedef struct idr_t { + uint8_t phy_head; + uint8_t cylinder; + uint8_t head; + uint8_t record; +} idr_t; + /* Command callbacks prototypes */ static void pre_exec_read_track(void); static void pre_exec_specify(void); @@ -79,8 +87,8 @@ static void pre_exec_seek(void); static bool is_cmd_out_of_sequence(uint8_t cmd); static void fdc_compute_next_status(void); // Update read buffer with the data from current ths -static void buffer_update(void); -static void buffer_write_size(void); +static bool buffer_update(void); +static bool buffer_write_size(void); /* Local variables */ // The command descriptors @@ -200,6 +208,9 @@ static uint8_t track[4]; static fdc_read_write_t read_buffer_cb = NULL; static fdc_read_write_t write_buffer_cb = NULL; +/* ID Register, store CHR for the current and the next record under execution */ +idr_t idr, next_idr; + /* * * * * * * * * * * * * * * Command routines * * * * * * * * * * * * * * */ static void pre_exec_read_track(void) { @@ -226,28 +237,29 @@ static void pre_exec_specify(void) { // Write data: static void pre_exec_write_data(void) { - // TODO(giuliof): eventually: use the appropriate structure + rw_args_t *rw_args = (rw_args_t *)args; + LOG_DEBUG("FDC Write Data\n"); - LOG_DEBUG("MF: %d\n", (command_args >> 6) & 0x01); - LOG_DEBUG("MT: %d\n", (command_args >> 7) & 0x01); - LOG_DEBUG("Drive: %d\n", args[0] & 0x3); - LOG_DEBUG("HD: %d\n", (args[0] >> 2) & 0x1); - LOG_DEBUG("Cyl: %d\n", args[1]); - LOG_DEBUG("Head: %d\n", args[2]); - LOG_DEBUG("Record: %d\n", args[3]); - LOG_DEBUG("N: %d\n", args[4]); - LOG_DEBUG("EOT: %d\n", args[5]); - LOG_DEBUG("GPL: %d\n", args[6]); - LOG_DEBUG("DTL: %d\n", args[7]); + LOG_DEBUG("MF: %d\n", (bool)(command_args & FDC_CMD_ARGS_MF_bm)); + LOG_DEBUG("MT: %d\n", (bool)(command_args & FDC_CMD_ARGS_MT_bm)); + LOG_DEBUG("Drive: %d\n", rw_args->unit_head & FDC_ST0_US); + LOG_DEBUG("HD: %d\n", (bool)(rw_args->unit_head & FDC_ST0_HD)); + LOG_DEBUG("Cyl: %d\n", rw_args->cylinder); + LOG_DEBUG("Head: %d\n", rw_args->head); + LOG_DEBUG("Record: %d\n", rw_args->record); + LOG_DEBUG("N: %d\n", rw_args->n); + LOG_DEBUG("EOT: %d\n", rw_args->eot); + LOG_DEBUG("GPL: %d\n", rw_args->gpl); + LOG_DEBUG("DTL: %d\n", rw_args->dtl); // Set DIO to read for Execution phase status_register[MSR] &= (uint8_t)~FDC_ST_DIO; - // Clear errors on other status registers - // TODO put this in a function - status_register[ST0] = 0; - status_register[ST1] = 0; - status_register[ST2] = 0; - status_register[ST3] = 0; + + idr.phy_head = rw_args->unit_head; + idr.cylinder = rw_args->cylinder; + idr.head = rw_args->head; + idr.record = rw_args->record; + memcpy(&next_idr, &idr, sizeof(idr)); buffer_write_size(); } @@ -259,26 +271,29 @@ static uint8_t exec_write_data(uint8_t value) { if (write_buffer_cb == NULL) return 0; - if (rwcount_max == 0) { - LOG_WARN("Write execution happened when no data can be written"); - return 0; + if (rwcount >= rwcount_max) { + if (!buffer_write_size()) + return 0; } exec_buffer[rwcount++] = value; + // From the manual: in NON-DMA mode, interrupt is generated during + // execution phase (as soon as new data is available) + int_status = true; // More data can be written, just go on with the current buffer if (rwcount != rwcount_max) return 0; /* Commit the current buffer and prepare the next one to be written */ - uint8_t sector = rw_args->record; + uint8_t sector = idr.record; // FDC counts sectors from 1 CEDA_STRONG_ASSERT_TRUE(sector != 0); // But all other routines counts sectors from 0 sector--; int ret = - write_buffer_cb(exec_buffer, drive, rw_args->unit_head & FDC_ST0_HD, - track[drive], rw_args->head, rw_args->cylinder, sector); + write_buffer_cb(exec_buffer, drive, next_idr.phy_head & FDC_ST0_HD, + track[drive], next_idr.head, next_idr.cylinder, sector); // Error condition // TODO(giuliof): errors may be differentiated, but for the moment cath all @@ -297,40 +312,6 @@ static uint8_t exec_write_data(uint8_t value) { return 0; } - // Multi-sector mode (enabled by default). - // If read is not interrupted at the end of the sector, the next logical - // sector is loaded - rw_args->record++; - - // Last sector of the track - if (rw_args->record > rw_args->eot) { - // In any case, reached the end of track we start back from sector 1 - rw_args->record = 1; - - // Multi track mode, if enabled the read operation go on on the next - // side - if (command_args & FDC_CMD_ARGS_MT_bm) { - rw_args->unit_head ^= FDC_ST0_HD; - rw_args->head = !rw_args->head; - - if (!(rw_args->unit_head & FDC_ST0_HD)) { - // Terminate execution if end of track is reached - tc_status = true; - rw_args->cylinder++; - status_register[ST0] = rw_args->unit_head; - return 0; - } - } else { - // Terminate execution if end of track is reached - tc_status = true; - rw_args->cylinder++; - status_register[ST0] = rw_args->unit_head; - return 0; - } - } - - buffer_write_size(); - return 0; } @@ -342,42 +323,58 @@ static void post_exec_write_data(void) { memset(result, 0x00, sizeof(result)); /* Status registers 0-2 */ - result[0] |= status_register[ST0]; - result[1] |= status_register[ST1]; - result[2] |= status_register[ST2]; + result[0] = status_register[ST0]; + result[1] = status_register[ST1]; + result[2] = status_register[ST2]; /* CHR */ - result[3] = rw_args->cylinder; - result[4] = rw_args->head; - result[5] = rw_args->record; + // When the FDC exits from exec mode with no error, next IDR should be used + if ((result[0] & FDC_ST0_IC) == 0) { + result[0] &= (uint8_t)~FDC_ST0_HD; + if (next_idr.phy_head & FDC_ST0_HD) + result[0] |= FDC_ST0_HD; + result[3] = next_idr.cylinder; + result[4] = next_idr.head; + result[5] = next_idr.record; + } + // Else, in case of error, use the last valid read sector (current IDR) + else { + result[0] &= (uint8_t)~FDC_ST0_HD; + if (idr.phy_head & FDC_ST0_HD) + result[0] |= FDC_ST0_HD; + result[3] = idr.cylinder; + result[4] = idr.head; + result[5] = idr.record; + } /* Sector size factor */ result[6] = rw_args->n; } // Read data: static void pre_exec_read_data(void) { - // TODO(giuliof): eventually: use the appropriate structure + rw_args_t *rw_args = (rw_args_t *)args; + LOG_DEBUG("FDC Read Data\n"); - LOG_DEBUG("SK: %d\n", (command_args >> 5) & 0x01); - LOG_DEBUG("MF: %d\n", (command_args >> 6) & 0x01); - LOG_DEBUG("MT: %d\n", (command_args >> 7) & 0x01); - LOG_DEBUG("Drive: %d\n", args[0] & 0x3); - LOG_DEBUG("HD: %d\n", (args[0] >> 2) & 0x1); - LOG_DEBUG("Cyl: %d\n", args[1]); - LOG_DEBUG("Head: %d\n", args[2]); - LOG_DEBUG("Record: %d\n", args[3]); - LOG_DEBUG("N: %d\n", args[4]); - LOG_DEBUG("EOT: %d\n", args[5]); - LOG_DEBUG("GPL: %d\n", args[6]); - LOG_DEBUG("DTL: %d\n", args[7]); + LOG_DEBUG("SK: %d\n", (bool)(command_args & FDC_CMD_ARGS_SK_bm)); + LOG_DEBUG("MF: %d\n", (bool)(command_args & FDC_CMD_ARGS_MF_bm)); + LOG_DEBUG("MT: %d\n", (bool)(command_args & FDC_CMD_ARGS_MT_bm)); + LOG_DEBUG("Drive: %d\n", rw_args->unit_head & FDC_ST0_US); + LOG_DEBUG("HD: %d\n", (bool)(rw_args->unit_head & FDC_ST0_HD)); + LOG_DEBUG("Cyl: %d\n", rw_args->cylinder); + LOG_DEBUG("Head: %d\n", rw_args->head); + LOG_DEBUG("Record: %d\n", rw_args->record); + LOG_DEBUG("N: %d\n", rw_args->n); + LOG_DEBUG("EOT: %d\n", rw_args->eot); + LOG_DEBUG("GPL: %d\n", rw_args->gpl); + LOG_DEBUG("DTL: %d\n", rw_args->dtl); // Set DIO to read for Execution phase status_register[MSR] |= FDC_ST_DIO; - // Clear errors on other status registers - // TODO put this in a function - status_register[ST0] = 0; - status_register[ST1] = 0; - status_register[ST2] = 0; - status_register[ST3] = 0; + + idr.phy_head = rw_args->unit_head; + idr.cylinder = rw_args->cylinder; + idr.head = rw_args->head; + idr.record = rw_args->record; + memcpy(&next_idr, &idr, sizeof(idr)); // TODO(giuliof) create handles to manage more than one floppy image at a // time @@ -392,52 +389,23 @@ static uint8_t exec_read_data(uint8_t value) { rw_args_t *rw_args = (rw_args_t *)args; - if (rwcount_max == 0) { - LOG_WARN("Read execution happened when no data can be read"); - return 0; - } - - uint8_t ret = exec_buffer[rwcount++]; + uint8_t ret = 0; - // More data can be read, just go on with the current buffer - if (rwcount != rwcount_max) - return ret; - - /* Prepare the next buffer to be read */ - // Multi-sector mode (enabled by default). - // If read is not interrupted at the end of the sector, the next logical - // sector is loaded - rw_args->record++; - - // Last sector of the track - if (rw_args->record > rw_args->eot) { - // In any case, reached the end of track we start back from sector 1 - rw_args->record = 1; - - // Multi track mode, if enabled the read operation go on on the next - // side - if (command_args & FDC_CMD_ARGS_MT_bm) { - rw_args->unit_head ^= FDC_ST0_HD; - rw_args->head = !rw_args->head; - - if (!(rw_args->unit_head & FDC_ST0_HD)) { - // Terminate execution if end of track is reached - tc_status = true; - rw_args->cylinder++; - status_register[ST0] = rw_args->unit_head; - return 0; - } - } else { - // Terminate execution if end of track is reached - tc_status = true; - rw_args->cylinder++; - status_register[ST0] = rw_args->unit_head; - return 0; - } + // Sector buffer already populated and on-going reading + if (rwcount < rwcount_max) { + ret = exec_buffer[rwcount++]; + } + // No sector buffer or finished one, try to get another sector from image + else if ((rwcount_max == 0 || rwcount >= rwcount_max)) { + if (buffer_update()) + ret = exec_buffer[rwcount++]; } - buffer_update(); + // From the manual: in NON-DMA mode, interrupt is generated during + // execution phase (as soon as new data is available) + int_status = true; + /* Prepare the next buffer to be read */ return ret; } @@ -446,16 +414,29 @@ static void post_exec_read_data(void) { LOG_DEBUG("Read has ended\n"); - memset(result, 0x00, sizeof(result)); - /* Status registers 0-2 */ - result[0] |= status_register[ST0]; - result[1] |= status_register[ST1]; - result[2] |= status_register[ST2]; + result[0] = status_register[ST0]; + result[1] = status_register[ST1]; + result[2] = status_register[ST2]; /* CHR */ - result[3] = rw_args->cylinder; - result[4] = rw_args->head; - result[5] = rw_args->record; + // When the FDC exits from exec mode with no error, next IDR should be used + if ((result[0] & FDC_ST0_IC) == 0) { + result[0] &= (uint8_t)~FDC_ST0_HD; + if (next_idr.phy_head & FDC_ST0_HD) + result[0] |= FDC_ST0_HD; + result[3] = next_idr.cylinder; + result[4] = next_idr.head; + result[5] = next_idr.record; + } + // Else, in case of error, use the last valid read sector (current IDR) + else { + result[0] &= (uint8_t)~FDC_ST0_HD; + if (idr.phy_head & FDC_ST0_HD) + result[0] |= FDC_ST0_HD; + result[3] = idr.cylinder; + result[4] = idr.head; + result[5] = idr.record; + } /* Sector size factor */ result[6] = rw_args->n; } @@ -717,15 +698,17 @@ static void fdc_compute_next_status(void) { status_register[MSR] &= (uint8_t)~FDC_ST_CB; } -static void buffer_update(void) { +static bool buffer_update(void) { rw_args_t *rw_args = (rw_args_t *)args; uint8_t drive = rw_args->unit_head & FDC_ST0_US; - - uint8_t sector = rw_args->record; + uint8_t sector = next_idr.record; rwcount = 0; rwcount_max = 0; - status_register[ST0] = rw_args->unit_head; + status_register[ST0] = drive; + status_register[ST1] = 0; + status_register[ST2] = 0; + status_register[ST3] = 0; // FDC counts sectors from 1 assert(sector != 0); @@ -734,25 +717,25 @@ static void buffer_update(void) { sector--; if (read_buffer_cb == NULL) - return; + return false; int ret = - read_buffer_cb(NULL, drive, rw_args->unit_head & FDC_ST0_HD, - track[drive], rw_args->head, rw_args->cylinder, sector); + read_buffer_cb(NULL, drive, next_idr.phy_head & FDC_ST0_HD, + track[drive], next_idr.head, next_idr.cylinder, sector); if (ret > DISK_IMAGE_NOMEDIUM) { // Buffer is statically allocated, be sure that the data can fit it CEDA_STRONG_ASSERT_TRUE((size_t)ret <= sizeof(exec_buffer)); - ret = read_buffer_cb(exec_buffer, drive, - rw_args->unit_head & FDC_ST0_HD, track[drive], - rw_args->head, rw_args->cylinder, sector); + ret = read_buffer_cb(exec_buffer, drive, next_idr.phy_head & FDC_ST0_HD, + track[drive], next_idr.head, next_idr.cylinder, + sector); } // No medium, FDC is in EXEC state until a disk is inserted, or manual // termination if (ret == DISK_IMAGE_NOMEDIUM) - return; + return false; // generate interrupt since an event has occurred int_status = true; @@ -763,30 +746,63 @@ static void buffer_update(void) { rwcount_max = MIN((size_t)rw_args->dtl, (size_t)ret); else rwcount_max = (size_t)ret; + + // Update IDR for the next sector to be read + // TODO(giuliof): This can be done in a function + + // Confirm the current IDR, since the buffer update was successful + memcpy(&idr, &next_idr, sizeof(idr)); + + // Multi-sector mode (enabled by default). + // If read is not interrupted at the end of the sector, the next logical + // sector is loaded + next_idr.record++; + + // Last sector of the track + if (next_idr.record > rw_args->eot) { + // In any case, reached the end of track we start back from sector 1 + next_idr.record = 1; + + // Multi track mode, if enabled the read operation go on on the next + // side + if (command_args & FDC_CMD_ARGS_MT_bm) { + next_idr.phy_head ^= FDC_ST0_HD; + next_idr.head = !next_idr.head; + + if (!(next_idr.phy_head & FDC_ST0_HD)) + next_idr.cylinder++; + + } else { + next_idr.cylinder++; + } + } + + return true; } // Error condition // TODO(giuliof): errors may be differentiated, but for the moment cath all // as a generic error - else { - LOG_WARN("Reading error occurred, code %d\n", ret); - // Update status register setting error condition and error type flags - status_register[ST0] |= 0x40; - status_register[ST1] |= 0x20; - status_register[ST2] |= 0x20; - // Execution is terminated after an error - tc_status = true; - } + LOG_WARN("Reading error occurred, code %d\n", ret); + // Update status register setting error condition and error type flags + status_register[ST0] |= 0x40; + status_register[ST1] |= 0x20; + status_register[ST2] |= 0x20; + // Execution is terminated after an error + tc_status = true; + return false; } -static void buffer_write_size(void) { +static bool buffer_write_size(void) { rw_args_t *rw_args = (rw_args_t *)args; uint8_t drive = rw_args->unit_head & FDC_ST0_US; - - uint8_t sector = rw_args->record; + uint8_t sector = next_idr.record; rwcount = 0; rwcount_max = 0; - status_register[ST0] = rw_args->unit_head; + status_register[ST0] = drive; + status_register[ST1] = 0; + status_register[ST2] = 0; + status_register[ST3] = 0; // FDC counts sectors from 1 assert(sector != 0); @@ -795,16 +811,16 @@ static void buffer_write_size(void) { sector--; if (write_buffer_cb == NULL) - return; + return false; int ret = - write_buffer_cb(NULL, drive, rw_args->unit_head & FDC_ST0_HD, - track[drive], rw_args->head, rw_args->cylinder, sector); + write_buffer_cb(NULL, drive, next_idr.phy_head & FDC_ST0_HD, + track[drive], next_idr.head, next_idr.cylinder, sector); // No medium, FDC is in EXEC state until a disk is inserted, or manual // termination if (ret == DISK_IMAGE_NOMEDIUM) - return; + return false; // generate interrupt since an event has occurred int_status = true; @@ -815,19 +831,51 @@ static void buffer_write_size(void) { rwcount_max = MIN((size_t)rw_args->dtl, (size_t)ret); else rwcount_max = (size_t)ret; + + // Update IDR for the next sector to be read + // TODO(giuliof): This can be done in a function + + // Confirm the current IDR, since the buffer update was successful + memcpy(&idr, &next_idr, sizeof(idr)); + + // Multi-sector mode (enabled by default). + // If read is not interrupted at the end of the sector, the next logical + // sector is loaded + next_idr.record++; + + // Last sector of the track + if (next_idr.record > rw_args->eot) { + // In any case, reached the end of track we start back from sector 1 + next_idr.record = 1; + + // Multi track mode, if enabled the read operation go on on the next + // side + if (command_args & FDC_CMD_ARGS_MT_bm) { + next_idr.phy_head ^= FDC_ST0_HD; + next_idr.head = !next_idr.head; + + if (!(next_idr.phy_head & FDC_ST0_HD)) + next_idr.cylinder++; + + } else { + next_idr.cylinder++; + } + } + + return true; } // Error condition // TODO(giuliof): errors may be differentiated, but for the moment cath all // as a generic error - else { - LOG_WARN("Reading error occurred, code %d\n", ret); - // Update status register setting error condition and error type flags - status_register[ST0] |= 0x40; - status_register[ST1] |= 0x20; - status_register[ST2] |= 0x20; - // Execution is terminated after an error - tc_status = true; - } + LOG_WARN("Reading error occurred, code %d\n", ret); + // Update status register setting error condition and error type flags + status_register[ST0] |= 0x40; + status_register[ST1] |= 0x20; + status_register[ST2] |= 0x20; + // Execution is terminated after an error + tc_status = true; + + return false; } /* * * * * * * * * * * * * * * Public routines * * * * * * * * * * * * * * */ From b39688ebc6272cd38c9aff88f67c0b0018d42af1 Mon Sep 17 00:00:00 2001 From: giuliof Date: Mon, 10 Mar 2025 18:24:39 +0100 Subject: [PATCH 47/52] refactor: double ! instead of cast to bool when printing boolean flags --- src/fdc.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index bc650c6..3b92f69 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -240,10 +240,10 @@ static void pre_exec_write_data(void) { rw_args_t *rw_args = (rw_args_t *)args; LOG_DEBUG("FDC Write Data\n"); - LOG_DEBUG("MF: %d\n", (bool)(command_args & FDC_CMD_ARGS_MF_bm)); - LOG_DEBUG("MT: %d\n", (bool)(command_args & FDC_CMD_ARGS_MT_bm)); + LOG_DEBUG("MF: %d\n", !!(command_args & FDC_CMD_ARGS_MF_bm)); + LOG_DEBUG("MT: %d\n", !!(command_args & FDC_CMD_ARGS_MT_bm)); LOG_DEBUG("Drive: %d\n", rw_args->unit_head & FDC_ST0_US); - LOG_DEBUG("HD: %d\n", (bool)(rw_args->unit_head & FDC_ST0_HD)); + LOG_DEBUG("HD: %d\n", !!(rw_args->unit_head & FDC_ST0_HD)); LOG_DEBUG("Cyl: %d\n", rw_args->cylinder); LOG_DEBUG("Head: %d\n", rw_args->head); LOG_DEBUG("Record: %d\n", rw_args->record); @@ -354,11 +354,11 @@ static void pre_exec_read_data(void) { rw_args_t *rw_args = (rw_args_t *)args; LOG_DEBUG("FDC Read Data\n"); - LOG_DEBUG("SK: %d\n", (bool)(command_args & FDC_CMD_ARGS_SK_bm)); - LOG_DEBUG("MF: %d\n", (bool)(command_args & FDC_CMD_ARGS_MF_bm)); - LOG_DEBUG("MT: %d\n", (bool)(command_args & FDC_CMD_ARGS_MT_bm)); + LOG_DEBUG("SK: %d\n", !!(command_args & FDC_CMD_ARGS_SK_bm)); + LOG_DEBUG("MF: %d\n", !!(command_args & FDC_CMD_ARGS_MF_bm)); + LOG_DEBUG("MT: %d\n", !!(command_args & FDC_CMD_ARGS_MT_bm)); LOG_DEBUG("Drive: %d\n", rw_args->unit_head & FDC_ST0_US); - LOG_DEBUG("HD: %d\n", (bool)(rw_args->unit_head & FDC_ST0_HD)); + LOG_DEBUG("HD: %d\n", !!(rw_args->unit_head & FDC_ST0_HD)); LOG_DEBUG("Cyl: %d\n", rw_args->cylinder); LOG_DEBUG("Head: %d\n", rw_args->head); LOG_DEBUG("Record: %d\n", rw_args->record); @@ -492,15 +492,14 @@ static void pre_exec_format_track(void) { uint8_t phy_head = !!(format_args->unit_head & FDC_ST0_HD); uint8_t drive = format_args->unit_head & FDC_ST0_US; - // TODO(giuliof): eventually: use the appropriate structure LOG_DEBUG("FDC Format track\n"); - LOG_DEBUG("MF: %d\n", (command_args >> 6) & 0x01); - LOG_DEBUG("Drive: %d\n", args[0] & 0x3); - LOG_DEBUG("HD: %d\n", (args[0] >> 2) & 0x1); - LOG_DEBUG("N: %d\n", args[1]); - LOG_DEBUG("SPT: %d\n", args[2]); - LOG_DEBUG("GPL: %d\n", args[3]); - LOG_DEBUG("D: %d\n", args[4]); + LOG_DEBUG("MF: %d\n", !!(command_args & FDC_CMD_ARGS_MF_bm)); + LOG_DEBUG("Drive: %d\n", format_args->unit_head & FDC_ST0_US); + LOG_DEBUG("HD: %d\n", !!(format_args->unit_head & FDC_ST0_HD)); + LOG_DEBUG("N: %d\n", format_args->n); + LOG_DEBUG("SPT: %d\n", format_args->sec_per_track); + LOG_DEBUG("GPL: %d\n", format_args->gpl); + LOG_DEBUG("D: %d\n", format_args->d); // Set deafult values of status registers status_register[ST0] = format_args->unit_head; From 4954835de3357d1cb04069d03669f4e2afa8e521 Mon Sep 17 00:00:00 2001 From: giuliof Date: Mon, 10 Mar 2025 18:26:46 +0100 Subject: [PATCH 48/52] docs: removed fixed comments --- src/fdc.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 3b92f69..e50d2b4 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -376,9 +376,6 @@ static void pre_exec_read_data(void) { idr.record = rw_args->record; memcpy(&next_idr, &idr, sizeof(idr)); - // TODO(giuliof) create handles to manage more than one floppy image at a - // time - // read_buffer_cb(exec_buffer, track_size, head, track, sector); // TODO(giuliof): may be a good idea to pass a sort of "floppy context" buffer_update(); } @@ -1011,16 +1008,11 @@ void fdc_tc_out(ceda_ioaddr_t address, uint8_t value) { (void)value; if (fdc_status == EXEC) { - // TODO(giuliof) tc may be an argument to the fdc_compute_next_status, - // since it is just a trigger. tc_status = true; fdc_compute_next_status(); } } -// TODO(giuliof): After Execution Phase or EOR sector read, INT=1 -// (beginning of result phase). When first byte of result phase data -// is read, INT=0. bool fdc_getIntStatus(void) { return int_status; } From f03131aac77f706765d0cf77a735c59c2806b61f Mon Sep 17 00:00:00 2001 From: giuliof Date: Tue, 11 Mar 2025 21:44:16 +0100 Subject: [PATCH 49/52] test(fdc): fixed invalid command tests Note, the tests should now fail. This is legit, the previous implementation was not fully correct (and probably neither this one, see comments...). Code fixup in the next commit. --- src/tests/test_fdc.c | 46 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index e12c5a3..a07ebed 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -167,6 +167,11 @@ Test(ceda_fdc, seekCommand) { cr_assert_eq(fdc_getIntStatus(), false); } +/* Invalid Seek Sequence + * From the manual: "a Sense Interrupt Status command must be sent after a Seek + * or Recalibrate Interrupt, otherwise the FDC will consider the next command to + * be an Invalid Command" (see invalidCommand test). + */ Test(ceda_fdc, invalidSeekSequence) { fdc_init(); @@ -182,19 +187,29 @@ Test(ceda_fdc, invalidSeekSequence) { // Second argument is cylinder position fdc_out(FDC_ADDR_DATA_REGISTER, 7); - // FDC is no more busy - assert_fdc_sr(FDC_ST_RQM | FDC_ST_D0B); + // Seek is ended, irq is raised + cr_assert_eq(fdc_getIntStatus(), true); + // Not quite sure about this, D0B may be zeroed after IRQ + // assert_fdc_sr(FDC_ST_RQM | FDC_ST_D0B); // Send another command that is not FDC_SENSE_INTERRUPT fdc_out(FDC_ADDR_DATA_REGISTER, FDC_SPECIFY); + // No interrupt must be present after an invalid command + cr_assert_eq(fdc_getIntStatus(), false); + + { + uint8_t sr; + sr = fdc_in(FDC_ADDR_STATUS_REGISTER); + // Remove busy drives, not interested + sr &= (uint8_t) ~(FDC_ST_D0B | FDC_ST_D1B | FDC_ST_D2B | FDC_ST_D3B); + cr_expect_eq(sr, (FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB)); + } + // FDC does not process this command and asserts invalid command - assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_D0B); data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, 0x80); - // No interrupt must be present after an invalid command - cr_assert_eq(fdc_getIntStatus(), false); } /** @@ -833,6 +848,15 @@ Test(ceda_fdc, formatCommandInvalidParams) { cr_assert_eq(fdc_getIntStatus(), false); } +/* Invalid Command + * From the manual: "If an invalid command is sent to the FDC, then the FDC will + * terminate the command after bits 7 and 6 of Status Register 0 are set to 1 + * and 0 respectively. No interrupt is generated by the FDC during this + * condition. Bit 6 and 7 (DIO and RQM) in the Main Status Register are both + * high" (result phase, data to be read) + * From tests on the actual FDC, CB bit is set too, indicating that invalid + * command is treated as an actual command. + */ Test(ceda_fdc, invalidCommand) { uint8_t st0; @@ -841,15 +865,17 @@ Test(ceda_fdc, invalidCommand) { // Force an invalid command fdc_out(FDC_ADDR_DATA_REGISTER, 0x00); - assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO); + assert_fdc_sr(FDC_ST_RQM | FDC_ST_DIO | FDC_ST_CB); + + // No interrupt must be present after result phase + cr_assert_eq(fdc_getIntStatus(), false); st0 = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(st0, 0x80); - // TODO(giuliof): this is actually unclear, the direction has to be set - // back? probably yes - assert_fdc_sr(FDC_ST_RQM); - // No interrupt must be present after result phase cr_assert_eq(fdc_getIntStatus(), false); + + // FDC is in idle state + assert_fdc_sr(FDC_ST_RQM); } From 67ae9278002f483082cdee00478c771081d53a65 Mon Sep 17 00:00:00 2001 From: giuliof Date: Tue, 11 Mar 2025 22:49:36 +0100 Subject: [PATCH 50/52] fix(fdc): correct handling of invalid commands --- src/fdc.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index e50d2b4..9821e4e 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -86,6 +86,7 @@ static void pre_exec_seek(void); /* Utility routines prototypes */ static bool is_cmd_out_of_sequence(uint8_t cmd); static void fdc_compute_next_status(void); +static void set_invalid_cmd(void); // Update read buffer with the data from current ths static bool buffer_update(void); static bool buffer_write_size(void); @@ -174,6 +175,15 @@ static const fdc_operation_t fdc_operations[] = { .post_exec = NULL, }, }; +// This is a dummy operation used when an invalid command has to be handled +static const fdc_operation_t invalid_op = { + .cmd = 0x00, // This command code does't exists + .args_len = 0, + .result_len = 1, // Invalid command has to report just ST0 + .pre_exec = NULL, + .exec = NULL, + .post_exec = NULL, +}; // Current FDC status static fdc_status_t fdc_status = CMD; // Currently selected operation @@ -694,6 +704,21 @@ static void fdc_compute_next_status(void) { status_register[MSR] &= (uint8_t)~FDC_ST_CB; } +static void set_invalid_cmd(void) { + // Invalid command is not an actual command... + fdc_currop = &invalid_op; + + fdc_status = CMD; + + // TODO(giuliof): current drive has to be preserved? + status_register[ST0] &= (uint8_t)~FDC_ST0_US; + // TODO(giuliof): should present other flags? + status_register[ST0] |= 0x80; + + // Immediately prepare result + result[0] = status_register[ST0]; +} + static bool buffer_update(void) { rw_args_t *rw_args = (rw_args_t *)args; uint8_t drive = rw_args->unit_head & FDC_ST0_US; @@ -973,12 +998,10 @@ void fdc_out(ceda_ioaddr_t address, uint8_t value) { if (fdc_currop == NULL) { LOG_WARN("Command %x is not implemented\n", cmd); - // Invalid command: set the main status register to read to - // serve ST0 and error code - status_register[ST0] &= (uint8_t)~FDC_ST0_IC; - status_register[ST0] |= 0x80; + // Invalid command is not an actual command... + fdc_currop = &invalid_op; - status_register[MSR] |= FDC_ST_DIO; + set_invalid_cmd(); } } else if (fdc_status == ARGS) { assert(rwcount < sizeof(args) / sizeof(*args)); From 5d0f62d4602d8d2fa2eb1a801e862eb776c7e0fc Mon Sep 17 00:00:00 2001 From: giuliof Date: Tue, 11 Mar 2025 23:18:58 +0100 Subject: [PATCH 51/52] fixup! fix(fdc): interrupt signal deassertion --- src/tests/test_fdc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/test_fdc.c b/src/tests/test_fdc.c index a07ebed..5cc7a9a 100644 --- a/src/tests/test_fdc.c +++ b/src/tests/test_fdc.c @@ -209,7 +209,6 @@ Test(ceda_fdc, invalidSeekSequence) { // FDC does not process this command and asserts invalid command data = fdc_in(FDC_ADDR_DATA_REGISTER); cr_expect_eq(data, 0x80); - } /** From a416f1d781b8af5631cd2ea65ac3508cd46f1bdd Mon Sep 17 00:00:00 2001 From: giuliof Date: Tue, 11 Mar 2025 23:38:50 +0100 Subject: [PATCH 52/52] fix(fdc): removed unused variable --- src/fdc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/fdc.c b/src/fdc.c index 9821e4e..ab84867 100644 --- a/src/fdc.c +++ b/src/fdc.c @@ -394,8 +394,6 @@ static uint8_t exec_read_data(uint8_t value) { // read doesn't care of in value (void)value; - rw_args_t *rw_args = (rw_args_t *)args; - uint8_t ret = 0; // Sector buffer already populated and on-going reading