fdc: remove sparc sun4m mutations

PATCH In review Hervé Poussineau 3 participants 6 messages Mar 14, 2015 – Mar 19, 2015

Share & tools

Hervé Poussineau

author

2015-03-14 16:55 UTC #0 lore raw

PATCH 1/1 fdc: remove sparc sun4m mutations

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 <[email protected]>

---
 hw/block/fdc.c |   17 -----------------
 1 file changed, 17 deletions(-)

hw/block/fdc.c +0 ~~-17~~

index 2bf87c9..f72a392 100644
@@ -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);

Signature · 1 line

1.7.10.4

John Snow

2015-03-16 21:48 UTC #1 lore raw

Quoted 6 lines · 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.

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

Quoted 55 lines

Signed-off-by Hervé Poussineau <[email protected]>

---
  hw/block/fdc.c |   17 -----------------
  1 file changed, 17 deletions(-)

hw/block/fdc.c +0 ~~-17~~

index 2bf87c9..f72a392 100644
@@ -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);

Hervé Poussineau

author

2015-03-17 06:47 UTC #2 lore raw

Hi,

Quoted 8 lines · John Snow

Quoted 6 lines · 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.

Quoted 6 lines

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é

Quoted 60 lines

Thanks,
--js

Quoted 55 lines

Signed-off-by Hervé Poussineau <[email protected]>

---
  hw/block/fdc.c |   17 -----------------
  1 file changed, 17 deletions(-)

hw/block/fdc.c +0 ~~-17~~

index 2bf87c9..f72a392 100644
@@ -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);

Mark Cave-Ayland

2015-03-19 14:50 UTC #3 lore raw

Quoted 36 lines · Hervé Poussineau

Hi,
Quoted 16 lines · John Snow
Quoted 8 lines · 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.
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 :)
Quoted 6 lines
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"

Quoted 3 lines

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.

John Snow

2015-03-19 17:58 UTC #4 lore raw

Quoted 49 lines · Mark Cave-Ayland

Quoted 36 lines · Hervé Poussineau
Hi,
Quoted 16 lines · John Snow
Quoted 8 lines · 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.
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 :)
Quoted 6 lines
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"
Quoted 3 lines
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.

Quoted 4 lines

ATB,
Mark.

Sounds good to me. Will stage for 2.4.

Thanks,
--js

Mark Cave-Ayland

Tested-by on v1 1/1

2015-03-19 14:17 UTC #5 lore raw

In reply to Hervé Poussineau

Quoted 53 lines · 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 <[email protected]>

---
 hw/block/fdc.c |   17 -----------------
 1 file changed, 17 deletions(-)

hw/block/fdc.c +0 ~~-15~~

index 2bf87c9..f72a392 100644
@@ -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;

hw/block/fdc.c static void sun4m_fdc_initfn(Object *obj) · line 2231

Quoted 8 lines

hw/block/fdc.c +0 -2

@@ -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 <[email protected]>

ATB,

Mark.