Menu
▾
▴
[fuse-emulator-devel] [ fuse-emulator-Patches-1779138 ] separated disk image, disk drive, disk controller emulation
|
From: SourceForge.net <no...@so...> - 2007-08-31 01:22:16
|
Patches item #1779138, was opened at 2007-08-22 02:25 Message generated for change (Comment added) made by szaszg You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=596650&aid=1779138&group_id=91293 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Gergely Szasz (szaszg) Assigned to: Nobody/Anonymous (nobody) Summary: separated disk image,disk drive, disk controller emulation Initial Comment: This patch try to separate: - the disk image handling - the floppy disk drive (fdd) emulation - and the floppy disk /drive/ controller (fdc) emulation this working for the MGT plusd (disk/wd1770.c,plusd.c) there are 4 new file: - disk.h and disk.c - fdd.h and fdd.c disk: - create an unformatted disk - open a disk image file (UDI/MGT/IMG/CPC/ExtCPC) and convert it into the inner format (UDI) - write a disk image file from the memory (that time only UDI format implemented) fdd: - we can initialise fdd with different drive geometry, e.g. 1 head 40 track, or even 2 head 83 track - load a disk image (created or opened). we can load a disk with upsidedown - fdc can change/step the head - read/write a data byte. all read/write go with raw data, so gap and marks with missing/other clock. now, the plusd.c 'open file section' is a very simple code :) the wd1770.c changed a lot: - can select SD(FM) or DD(MFM) mode - implemented the readtrack command - rewrited the Type I commands (restore/seek/step...) code - revrited the Type II/III commands code now the wd1770.c doing only the needed things: read/write bytes, step/change head :-) ---------------------------------------------------------------------- >Comment By: Gergely Szasz (szaszg) Date: 2007-08-31 01:22 Message: Logged In: YES user_id=57243 Originator: YES - new SCL open code (thanks zub) - FDI open 35 sector/track limitation fixed (i just hope it works, because i have no a disk image with more than 35 sector in a track - crc16 generation bug fixed in disk.c - crc check/generation implemented in wd1770.c File Added: fuse.beta_10.diff ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-30 19:54 Message: Logged In: YES user_id=57243 Originator: YES - FDI save - SCL open - some #define e.g. SECTOR_BASE_1, INTERLEAVE_2, etc... File Added: fuse.beta_09.diff ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-28 17:19 Message: Logged In: YES user_id=57243 Originator: YES - check_geom: if mixed fm/mfm tracks mfm set to 1 - SAVE CPC: now can save mixed fm/mfm tracks - sector_base removed from disk_t and now a new parameter in functions - wrprt -> wrprot; tr -> track, cl -> clocks, bps -> sector_length, spt -> sectors, disk.idx -> disk.i, fdd.idx -> fdd.index - FDI read support - UDI files which has tracks greater size than the first track now supported - spelling fixed - new SAVE format: .log it dump the disk data at free level to the given file in human readable format: 1. sector headers 2. sector data 3. raw track data missing: - UDI CRC32 check during open - ..track_geom detect sector interleave/mixing/duplication - CRC in the wd177x code - FDI write - TD0 read/write - better fm/mfm detection during save and open - FM may use 0x00 for GAP instead of 0xff? - better GAP constant set selection for opening disks (based on calculated sector/track length) - ... File Added: fuse.beta_08.diff ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-27 13:30 Message: Logged In: YES user_id=207506 Originator: NO Definitely getting closer. :) It seems that guess_track_geom() would not notice if all the sectors on a track had the same ID... or if the sector IDs were not contiguous. I doubt that this would be a problem in reality, but it would be nice to detect non-uniform tracks. For disks with both FM and MFM tracks, should mfm be set to 1 in check_disk_geom? At the moment, mfm's value depends on the format of the first track on the disk. There is one problem that I've spotted -- where d->sector_base is set, prior to savetrack being called? (Please double check before telling me I'm an idiot, this time. ;) I think a few variable names still need changing. 'bpt' and 'tlen' aren't good enough, but I'm not sure what to suggest in their place. I think that 'wrprt' should be 'wrprot', 'tr' and 'cl' should be 'tracks' and 'clocks', and 'bps' should be 'sector_length'. 'idx' in fdd_t and disk_t doing totally different things doesn't seem great. Also, tempfile is still defined in disk_t. There are also a few spelling fixes that are needed... there's no such word as 'readed' (it's just 'read', pronounced /ri:d/ for the present tense, and /rEd/ for the past tense). Also, 'writen' -> 'written', 'cycl' -> 'cycle' and 'onli' -> 'only'. There's also my own mistake ('the the' -> 'the'). :) I would be happy to make those changes, provided there are no objections. After I'm happy with the savetrack, I will post what I intend to commit here. (This won't include the Betadisk changes for the time being, as there are some missing features such as .scl support which. must be addressed.) ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-26 23:11 Message: Logged In: YES user_id=57243 Originator: YES - fix disk_close bug (free again if called multiple time) - fix disk_init bug (did not reset fdd->loaded) - fix SAD file generation - disk.c: added FM and MFM minimal gap constants File Added: fuse.beta_07.diff ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-25 21:58 Message: Logged In: YES user_id=57243 Originator: YES - now '.trd' images converted to 'raw' track data with sector interleave=2 - wd1770.c fixed "b& 0x01" - geom_check determine sector_base and report if not constant - geom check report mixed MF and MFM tracks - savetrack fixed - crc_udi added, so saved UDI files now has correct CRC32 - added TR-DOS FORMAT GAP constants File Added: fuse.beta_06.diff ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-25 08:09 Message: Logged In: YES user_id=207506 Originator: NO I would rather prefer not to be quoted on things I say in #fuse at 2 AM... ;) but as you raise the issue, I was actually suggesting that tracks be held internally in interleaved order after reading from '.trd' images. Having said that, the new code looks great. I think I may have spotted one mistake, though: on line 3796, "b & 0x01" should probably be "f->ddam". A small nit: disk_check_geom() and savetrack() don't check/use the sector base. We can get away with this sort of thing with the DISCiPLE/+D and Betadisk, but we probably shouldn't. Oh, also, in the CRC16 code, it may be good to mention the generator polynomial (G(x) = x^16 + x^12 + x^5 + 1), and the name of the CRC, which is CRC-16-CCITT. Also, if the crc table can be generated by code, that may well be preferable. Looks good, though! ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-25 03:17 Message: Logged In: YES user_id=57243 Originator: YES - cut-off TRD handling (now we can 'autofill' the missing part with a given byte (0x00) ) - not uses the file length to determine TRD geometry - SAD file writing - initial CRC (crc.c crc.h) routines, now only for the fdc/trackgen() - disk_t.sector_base now used for generating 'raw' track data - wd1770.c: dden changed to positive logic, so dden = 1 -> MFM; dden = 0 -> FM zub: "gergely: I find the ordering of tracks in TR-DOS interesting... 1,9,2,10,3,11,4,12,5,13,6,14,7,15,8,16 trackgen() doesn't seem to handle that." hmm.. this is the "sector interleave" ;)but no problem in the image file, the sector order: 1,2,3,4 File Added: fuse.beta_05.diff ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-24 19:07 Message: Logged In: YES user_id=207506 Originator: NO I've attached an updated version, which adds support for '.sad' disk images, and adds a few things to the disk type enumeration. There are a few remaining issues: 'Cut-off' .trd images (where any trailing zero-filled sectors are removed from the image) aren't supported. Due to this feature, I feel that it does not make sense to determine the disk geometry from the image size. '.scl' images aren't supported. Looking at Scl2Trd in fuse-utils/scl2trd.c and also fuse/trdos.c, it seems to be an archive format rather than a disk image format, but I can't find any proper documentation for it. The sector base shouldn't be assumed to be 1, as we might well come across a wd1770-based interface that uses 0 instead. Autoload and snapshot support for Betadisk still have to be added, and the index pulse and fdc init/reset code still needs to be cleaned up. File Added: fuse.beta_04-zub2.diff ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-24 15:18 Message: Logged In: YES user_id=207506 Originator: NO No, you lseek here, but you don't read this. ;) Also, the "lseek( fd, 0, SEEK_SET );" should happen after the disk geometry has been determined. ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-24 15:08 Message: Logged In: YES user_id=57243 Originator: YES TR-DOS disk specs v1.0 - last revised on 9-28-1997: ...- Logical sector 8 (9th physical) holds disc info... Logical sector 8 structure:... ... Offset | Lenght | Description ... 227 | 1 | disk type ... (*) Byte at offset 227 could be one of the following: 16h : 80 tracks, double sided 17h : 40 tracks, double sided 18h : 80 tracks, single sided 19h : 40 tracks, single sided ... so i try to lseek here and read this... btw: thanks for the patch :) ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-24 14:44 Message: Logged In: YES user_id=207506 Originator: NO Sorry, I now realise that you must lseek() to the start of the file so the initial 256 bytes are read by data_add(), when calling trackgen(). What I don't understand is "lseek( fd, 8*256, SEEK_SET );" -- why are you skipping 8 sectors? Anyway, I've attached an updated patch with a few fixes and cleanups. File Added: fuse.beta_04-zub.diff ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-24 12:44 Message: Logged In: YES user_id=207506 Originator: NO No, you only break if it's _not_ write protected. If you intend to fall through, you should have a comment saying /* fall through */ before the next case label. The seek and restore handling is a bit better, but it's still not right. Firstly, bit 4 of the command byte distinguishes between reads and writes, but we're testing with 0x04 instead of 0x10. This was originally my fault. Updating is now done correctly for seek and restore, which is great! However, the 'update' variable should really be moved into the 'if' for the step commands, though. Also, instead of the extra check for 0xfc, it would probably be better to check for ( b & 0xf8 ) == 0xf8. I'm not sure why TR-DOS sends 0xff for RUN, and 0xf8 for LIST... and I wonder what's sending 0xfc? Maybe these do something on the KR1818VG93? What are the lseek()s for in the mgt/img/trd handling in disk_open()? ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-24 08:26 Message: Logged In: YES user_id=57243 Originator: YES - fixed Type I commands, - new 'extra' Forced Interrupt (0xfc) - .trd handling "...but it's in the wrong place, isn't it?" no :-) we break only if write protect, because than the command ended (aborted). If not write protect we go ahead, because READTRACK and WRITETRACK preparation is the same (just only the state differ) File Added: fuse.beta_04.diff ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-24 01:14 Message: Logged In: YES user_id=207506 Originator: NO As discussed in #fuse, a few more problems: When performing multisector, a cmdint should probably be signalled (and datarq cleared) at the end of each sector. Then, after a pause, datarq should be signalled and cmdint should be cleared. Some software seems to expect a cmdint at the end of each sector. My original Type I handling code set the verify flag according to bit 2 of the command byte. This should only be done for step commands, and not for restore and seek commands (where apparently, verification is always performed). ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-23 21:05 Message: Logged In: YES user_id=207506 Originator: NO I do think so. There is a 'break'... but it's in the wrong place, isn't it? This sort of thing worries me. ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-23 20:40 Message: Logged In: YES user_id=57243 Originator: YES - changed the inner track/head struct to ALT from OUT-OUT (it fixes UDI impl.) - implement saving disk images as .mgt/.img and CPC (.dsk) "The 'write track' code falls though into the 'read track' code if the disk is write protected." i don't think so... there is a 'break' :) File Added: fuse.beta_03.diff ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-22 22:37 Message: Logged In: YES user_id=57243 Originator: YES I change: - dens to density - spd to sides - tps to cylinders - c_cyl to c_cylinder - fdd_cyls to fdd_cylinders and now the wd1773 (betadisk) interpret f8 as a 'forced interrupt' command. plusd (wd1770/2) _not_ so betadisk looks working File Added: fuse.beta_02.diff ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-22 21:29 Message: Logged In: YES user_id=207506 Originator: NO As pointed out to me in #fuse, the sector register should indeed be set to the track id during execution of the read address command. There's a few more issues, though: A cmdint should be signalled at the end of most (if not all) commands. The 'write track' code falls though into the 'read track' code if the disk is write protected. ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-22 13:30 Message: Logged In: YES user_id=57243 Originator: YES File Added: fuse.beta_01.diff ---------------------------------------------------------------------- Comment By: Gergely Szasz (szaszg) Date: 2007-08-22 13:28 Message: Logged In: YES user_id=57243 Originator: YES ...e.g. "dens" when "density" isn't much typing. "tps" and "spd"? -- what's wrong with "cylinders" and "heads"?... nothing ;-) tps and spd just shorter and all 3 char length... :) but I will fix it.. the cylinder and tracks... i think, we use cylinder for disk drive and track for the disk itself, but not a crucial problem :) so here is a fast fix for the 'LOST' bit and one patch for betadisk code (stuart patch) File Added: fuse.disk_04.diff ---------------------------------------------------------------------- Comment By: Stuart Brady (zubzero) Date: 2007-08-22 12:10 Message: Logged In: YES user_id=207506 Originator: NO Looks good so far. Only three obvious problems: Line 1183 of the patch (f->data_register = f->sector_register = f->id_track;) doesn't make sense. We no longer set the 'LOST' status bit after a forced interrupt with the head over track 0. The comments/whitespace need tidying up. (Also, I feel there's too much abbreviation -- e.g. "dens" when "density" isn't much typing. "tps" and "spd"? -- what's wrong with "cylinders" and "heads"?) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=596650&aid=1779138&group_id=91293 |