Re: [Qemu-devel] [PATCH] fdc: remove sparc sun4m mutations

[John Snow <jsnow@redhat.com>]

On 03/14/2015 12:50 PM, Hervé Poussineau wrote:
> They were introduced in 6f7e9aec5eb5bdfa57a9e458e391b785c283a007 and
> 82407d1a4035e5bfefb53ffdcb270872f813b34c and lots of bug fixes were done after that.
>
> This fixes (at least) the detection of the floppy controller on Debian 4.0r9/SPARC,
> and SS-5's OBP initialization routine still works.
>

Removing workaround code from six years ago in a device we hardly touch 
seems sane to me if it doesn't appear to break the machine it was 
originally architected for (SS-5, from 82407d1a's commit message), but I 
am not well versed in SPARC configurations, unfortunately for us :)

It appears this quirk is active for a wide number of machine 
configurations (basically all that appear under sun4m_machine_init) -- 
What's the risk of us breaking one of those configurations?

How did you test SS-5? (Can we test the others similarly? Is there a 
justification for not doing so?)

Thanks,
--js

> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>   hw/block/fdc.c |   17 -----------------
>   1 file changed, 17 deletions(-)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 2bf87c9..f72a392 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -535,8 +535,6 @@ struct FDCtrl {
>       uint8_t pwrd;
>       /* Floppy drives */
>       uint8_t num_floppies;
> -    /* Sun4m quirks? */
> -    int sun4m;
>       FDrive drives[MAX_FD];
>       int reset_sensei;
>       uint32_t check_media_rate;
> @@ -885,13 +883,6 @@ static void fdctrl_reset_irq(FDCtrl *fdctrl)
>
>   static void fdctrl_raise_irq(FDCtrl *fdctrl)
>   {
> -    /* Sparc mutation */
> -    if (fdctrl->sun4m && (fdctrl->msr & FD_MSR_CMDBUSY)) {
> -        /* XXX: not sure */
> -        fdctrl->msr &= ~FD_MSR_CMDBUSY;
> -        fdctrl->msr |= FD_MSR_RQM | FD_MSR_DIO;
> -        return;
> -    }
>       if (!(fdctrl->sra & FD_SRA_INTPEND)) {
>           qemu_set_irq(fdctrl->irq, 1);
>           fdctrl->sra |= FD_SRA_INTPEND;
> @@ -1080,12 +1071,6 @@ static uint32_t fdctrl_read_main_status(FDCtrl *fdctrl)
>       fdctrl->dsr &= ~FD_DSR_PWRDOWN;
>       fdctrl->dor |= FD_DOR_nRESET;
>
> -    /* Sparc mutation */
> -    if (fdctrl->sun4m) {
> -        retval |= FD_MSR_DIO;
> -        fdctrl_reset_irq(fdctrl);
> -    };
> -
>       FLOPPY_DPRINTF("main status register: 0x%02x\n", retval);
>
>       return retval;
> @@ -2241,8 +2226,6 @@ static void sun4m_fdc_initfn(Object *obj)
>       FDCtrlSysBus *sys = SYSBUS_FDC(obj);
>       FDCtrl *fdctrl = &sys->state;
>
> -    fdctrl->sun4m = 1;
> -
>       memory_region_init_io(&fdctrl->iomem, obj, &fdctrl_mem_strict_ops,
>                             fdctrl, "fdctrl", 0x08);
>       sysbus_init_mmio(sbd, &fdctrl->iomem);
>