Re: [Qemu-devel] FDC: improve emulation [v2]

[Blue Swirl]

On 4/23/08, Hervé Poussineau <address@hidden> wrote:
> Hi,
>
>  First, Blue Swirl, thanks for the review.
>
>  Blue Swirl a écrit :
>
> > On 4/16/08, Hervé Poussineau <address@hidden> wrote:
> >
> > >  I've done big improvements on floppy drive controller.
> > >  For easier reviewing, I've tried to split them up in differents parts.
> > > Sorry if some parts are still big.
> > >  Each patch applies on top of the previous one.
> > >
> > >  If only some of them are accepted, I can provide a global patch for
> only
> > > these ones. Patches which needs more work will be resubmitted later.
> > >
> >
>
>
>  Here is a global patch after your comments.
>
>  Changelog:
>  - Adds a command lookup table, as suggested by Fabrice at
> http://lists.gnu.org/archive/html/qemu-devel/2008-04/msg00143.html
>  - This also moves initialization functions at the bottom of the file to
> prevent multiple forward declarations.
>  - Extract seeking to next sector handling in a function. Add a sector seek
> in PIO read and write modes
>  - Fixes status A and status B registers. It removes one Sun4m mutation.
> Also removes the internal FD_CTRL_INTR flag.
>  - Handles correctly FD_MSR_NONDMA/FD_DOR_NONDMA flags, and uses them when
> possible. Fixes a problem with SPECIFY command.
>  - Better handling of DOR register. DOR register shouldn't forget motor
> states for non-existing drives (drives 2 and 3), even if no physical pin for
> it exists.
>  - Use FD_DOR_nRESET flag instead of internal FD_CTRL_RESET flag.
>  - Support writing to DOR register even in reset mode (as said in
> specification)
>  - Stores controller state in MSR register instead of internal state field.
> This simplifies the fdctrl_read_main_status() function, which may be called
> in some tight loops.
>  - Replaces bootsel field by the whole tdr register. It may be easier if we
> want to later add support for tapes.
>  - Supports up to 4 floppy drives if MAX_FD is set to 4.
>  - Replaces access to cur_drv field by macros.
>
>  Changes from v1:
>  - patch 01: command_to_handler table size is now 256 (instead of 255)
>  - patch 04: not included in this serie
>  - patch 05: comment about Sun4m differences kept
>  - patch 07: fixed data_pos incrementation
>  - patch 08: version field kept
>  - patch 11: fixed compilation
>  - patch 12: not included in this serie
>
>  I'll rework patch 4 and send later patch 12, once this one is committed.

These changes pass my Sparc32 and Sparc64 tests. On Sparc32 floppy
handling is improved, so I'd like to commit these. If there are any
objections, please let me know soon.

But it's better to commit many small patches rather than one big
patch, because if there are problems, the culprit patch can be
identified and fixed. So could you still rework the patch set?

>  >>  fdc_04_status_012.diff
>  >
>  > Not good, this makes Sparc64 print
>  > SENSEI c0 00
>  > SENSEI c0 00
>  > forever.
>
>  I've removed patch 04 from this serie, so this shouldn't happen anymore.
> Blue Swirl, can you still please tell me how to reproduce the problem, so I
> can take a look?

./sparc64-softmmu/qemu-system-sparc64 -kernel /dev/null -L pc-bios

Without the patch, emulator exits quickly because of error state, you
may want to add -nographic and start Qemu in a terminal.