Re: [Qemu-devel] [PATCH] Floppy: Remove Sense Interrupt Hack

[Justin Chevrier]

--- On Tue, 12/23/08, Justin Chevrier <address@hidden> wrote:

> From: Justin Chevrier <address@hidden>
> Subject: Re: [Qemu-devel] [PATCH] Floppy: Remove Sense Interrupt Hack
> To: "Blue Swirl" <address@hidden>
> Cc: address@hidden
> Date: Tuesday, December 23, 2008, 1:02 PM
> --- On Sat, 12/20/08, Blue Swirl
> <address@hidden> wrote:
> 
> > From: Blue Swirl <address@hidden>
> > Subject: Re: [Qemu-devel] [PATCH] Floppy: Remove Sense
> Interrupt Hack
> > To: address@hidden
> > Cc: address@hidden
> > Date: Saturday, December 20, 2008, 5:54 AM
> > On 12/19/08, Justin Chevrier
> <address@hidden>
> > wrote:
> > >
> > >
> > >
> > >  --- On Fri, 12/19/08, Blue Swirl
> > <address@hidden> wrote:
> > >
> > >  > From: Blue Swirl
> <address@hidden>
> > >  > Subject: Re: [Qemu-devel] [PATCH] Floppy:
> Remove
> > Sense Interrupt Hack
> > >  > To: address@hidden,
> address@hidden
> > >  > Date: Friday, December 19, 2008, 2:43 PM
> > >
> > > > On 12/18/08, Justin Chevrier
> > <address@hidden>
> > >  > wrote:
> > >  > > Hey guys,
> > >  > >
> > >  > >  I'm tracking down the next
> issue(s)
> > with the
> > >  > Openserver installation. The current issue
> is
> > that during
> > >  > installation when the installer tries to
> link
> > the BLTD (eg.
> > >  > SCSI drivers) drivers into the kernel it is
> > unable to access
> > >  > the floppy drive. After reviewing the debug
> logs
> > it turns
> > >  > out that the installer dosen't like the
> > value being
> > >  > returned by Sense Interrupt. The cause of
> this
> > is a hack in
> > >  > the Sense Interrupt code that always
> returns an
> > OR'd
> > >  > value based on FD_SR0_SEEK instead of the
> actual
> > Status0
> > >  > regsiter. The hack identifies that the
> floppy
> > code is broken
> > >  > with regard to Status0 and Read/Writes.
> Outside
> > of setting
> > >  > particular values in case of CRC errors/etc
> I
> > can't
> > >  > identify from datasheets what we should be
> > doing. I've
> > >  > tracked down what I believe is the original
> > mailing list
> > >  > post regarding this hack and it seems to be
> > related to
> > >  > Windows 2000 and higher OS's
> recognizing the
> > floppy
> > >  > drive. I applied the patch below and
> verified
> > that
> > >  > >   detection/read/writes/formatting
> still
> > work in
> > >  > Windows XP SP2.
> > >  > >
> > >  > >  Original post:
> > >  > >
> > >  >
> >
> http://lists.gnu.org/archive/html/qemu-devel/2004-10/msg00076.html
> > >  > >
> > >  > >  SVN change:
> > >  > >
> > >  >
> >
> http://svn.savannah.gnu.org/viewvc?view=rev&root=qemu&revision=1118
> > >  > >
> > >  > >  Having said the above does anyone
> know
> > what the
> > >  > comment in the hack is referencing? Is it
> still
> > needed? If
> > >  > so what needs to be done to eliminate it?
> > >  > >
> > >  > >  If it's no longer needed I
> propose the
> > patch
> > >  > below.
> > >  > >
> > >  > >  Justin
> > >  > >
> > >  > >  Changelog:
> > >  > >
> > >  > >  Remove Sense Interrupt hack
> > >  > >
> > >  > >  Signed-off-by: Justin Chevrier
> > >  > <address@hidden>
> > >  >
> > >  > It may be a hack, but if we remove it,
> Sparc64
> > refuses to
> > >  > boot and
> > >  > prints endlessly:
> > >  > SENSEI c0 00
> > >  > SENSEI c0 00
> > >  > SENSEI c0 00
> > >  > SENSEI c0 00
> > >
> > >
> > > Okay, thanks for the feedback!
> > >
> > >  I'll start looking into what SPARC64 expects
> > here.
> > 
> > The message is printed here:
> >
> http://tracker.coreboot.org/trac/openbios/browser/openbios-devel/drivers/floppy.c#L359
> 
> Thanks again!
> 
> Does the patch below work for you?
> 
> In this version we return 0xc0 (polling) for the first
> interrupt status after reset (Openserver appears to require
> this). After returning 0xc0 we set the Status0 register to
> 0x20 (seek complete). This should work as the spec says
> Sense Interrupt should be called up to 4 times to retrieve
> the queued status values. Tested successfully in both
> Windows 98 and XP SP2.
> 
> It also appears that the code referenced in the link loops
> infinitely due to a missing: "max_sensei--;" which
> is present in the Linux Kernel's equivalent code.
> 
> Justin

Sorry for the malformed patch. Corrected one:

--- hw/fdc.c    (revision 6125)
+++ hw/fdc.c    (working copy)
@@ -1601,20 +1601,13 @@
 {
     fdrive_t *cur_drv = get_cur_drv(fdctrl);

-#if 0
     fdctrl->fifo[0] =
         fdctrl->status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
-#else
-    /* XXX: status0 handling is broken for read/write
-       commands, so we do this hack. It should be suppressed
-       ASAP */
-    fdctrl->fifo[0] =
-        FD_SR0_SEEK | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
-#endif
+
     fdctrl->fifo[1] = cur_drv->track;
     fdctrl_set_fifo(fdctrl, 2, 0);
     fdctrl_reset_irq(fdctrl);
-    fdctrl->status0 = FD_SR0_RDYCHG;
+    fdctrl->status0 = FD_SR0_SEEK;
 }

 static void fdctrl_handle_seek (fdctrl_t *fdctrl, int direction)