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

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

[Hervé Poussineau]

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.

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);
-- 
1.7.10.4

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

[John Snow]

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);
>

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

[Hervé Poussineau]

Hi,

Le 16/03/2015 22:48, John Snow a écrit :
> 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?)

According to 82407d1a4035e5bfefb53ffdcb270872f813b34c, there was probably a hang when SS-5 OBP firmware was initializing the floppy. I checked my change with the OBP SS-5 firmware 2.15, which doesn't 
hang and goes up to firmware prompt. I also tested SS-4, SS-10 (OBP v3.45) and SS-20 (OBP 2.25) which also go up to firmware prompt. I was unable to test the other platforms with OBP.
Then, I tested SS-5 with Debian 4.0r9, and boot messages now detect the floppy controller with "Floppy drive(s): fd0 is 1.44M" and "FDC 0 is a S82078B"

However, in 82407d1a4035e5bfefb53ffdcb270872f813b34c, floppy was probably not working, and is still broken after this commit. This may be due to a missing DMA controller on sparc platform.

Regards,

Hervé

>
> 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);
>>
>

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

[Mark Cave-Ayland]

On 17/03/15 06:43, Hervé Poussineau wrote:

> Hi,
> 
> Le 16/03/2015 22:48, John Snow a écrit :
>> 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?)
> 
> According to 82407d1a4035e5bfefb53ffdcb270872f813b34c, there was
> probably a hang when SS-5 OBP firmware was initializing the floppy. I
> checked my change with the OBP SS-5 firmware 2.15, which doesn't hang
> and goes up to firmware prompt. I also tested SS-4, SS-10 (OBP v3.45)
> and SS-20 (OBP 2.25) which also go up to firmware prompt. I was unable
> to test the other platforms with OBP.
> Then, I tested SS-5 with Debian 4.0r9, and boot messages now detect the
> floppy controller with "Floppy drive(s): fd0 is 1.44M" and "FDC 0 is a
> S82078B"
> 
> However, in 82407d1a4035e5bfefb53ffdcb270872f813b34c, floppy was
> probably not working, and is still broken after this commit. This may be
> due to a missing DMA controller on sparc platform.

Yeah, I'm fairly sure from Hervé's work in this area that the existing
sun4m floppy disk could never function correctly anyway. Given that this
is the case, I'd be okay with having this patch applied for all sun4m
models so that it can at least be detected, and hopefully persuaded to
work with only a little more effort.


ATB,

Mark.

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

[John Snow]

On 03/19/2015 10:25 AM, Mark Cave-Ayland wrote:
> On 17/03/15 06:43, Hervé Poussineau wrote:
>
>> Hi,
>>
>> Le 16/03/2015 22:48, John Snow a écrit :
>>> 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?)
>>
>> According to 82407d1a4035e5bfefb53ffdcb270872f813b34c, there was
>> probably a hang when SS-5 OBP firmware was initializing the floppy. I
>> checked my change with the OBP SS-5 firmware 2.15, which doesn't hang
>> and goes up to firmware prompt. I also tested SS-4, SS-10 (OBP v3.45)
>> and SS-20 (OBP 2.25) which also go up to firmware prompt. I was unable
>> to test the other platforms with OBP.
>> Then, I tested SS-5 with Debian 4.0r9, and boot messages now detect the
>> floppy controller with "Floppy drive(s): fd0 is 1.44M" and "FDC 0 is a
>> S82078B"
>>
>> However, in 82407d1a4035e5bfefb53ffdcb270872f813b34c, floppy was
>> probably not working, and is still broken after this commit. This may be
>> due to a missing DMA controller on sparc platform.
>
> Yeah, I'm fairly sure from Hervé's work in this area that the existing
> sun4m floppy disk could never function correctly anyway. Given that this
> is the case, I'd be okay with having this patch applied for all sun4m
> models so that it can at least be detected, and hopefully persuaded to
> work with only a little more effort.
>
>
> ATB,
>
> Mark.
>

Sounds good to me. Will stage for 2.4.

Thanks,
--js

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

[Mark Cave-Ayland]

On 14/03/15 16:50, 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.
> 
> 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);
> 

Hi Hervé,

I can confirm that this fixes detection of the FDC on my Linux and BSD
images so:

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.