[PATCH v4 0/9] hw/block/fdc: Allow Kconfig-selecting ISA bus/SysBus floppy controllers

Philippe Mathieu-Daudé posted 9 patches 2 years, 11 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210517183954.1223193-1-philmd@redhat.com
Maintainers: Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
There is a newer version of this series
hw/block/fdc-internal.h | 156 +++++++++++
include/hw/block/fdc.h  |   7 +-
hw/block/fdc-isa.c      | 313 +++++++++++++++++++++
hw/block/fdc-sysbus.c   | 224 +++++++++++++++
hw/block/fdc.c          | 608 +---------------------------------------
hw/mips/jazz.c          |  16 ++
hw/sparc/sun4m.c        |  16 ++
MAINTAINERS             |   3 +
hw/block/Kconfig        |   8 +
hw/block/meson.build    |   2 +
hw/block/trace-events   |   3 +
hw/i386/Kconfig         |   2 +-
hw/isa/Kconfig          |   7 +-
hw/mips/Kconfig         |   2 +-
hw/sparc/Kconfig        |   2 +-
hw/sparc64/Kconfig      |   2 +-
16 files changed, 759 insertions(+), 612 deletions(-)
create mode 100644 hw/block/fdc-internal.h
create mode 100644 hw/block/fdc-isa.c
create mode 100644 hw/block/fdc-sysbus.c
[PATCH v4 0/9] hw/block/fdc: Allow Kconfig-selecting ISA bus/SysBus floppy controllers
Posted by Philippe Mathieu-Daudé 2 years, 11 months ago
Missing review: #1

Hi,

The floppy disc controllers pulls in irrelevant devices (sysbus in
an ISA-only machine, ISA bus + isa devices on a sysbus-only machine).

This series clean that by extracting each device in its own file,
adding the corresponding Kconfig symbols: FDC_ISA and FDC_SYSBUS.

Since v3:
- Fix ISA_SUPERIO -> FDC Kconfig dependency (jsnow)

Since v2:
- rebased

Since v1:
- added missing "hw/block/block.h" header (jsnow)
- inlined hardware specific calls (Mark)
- added R-b/A-b tags

Regards,

Phil.

Philippe Mathieu-Daudé (9):
  hw/isa/Kconfig: Fix missing dependency ISA_SUPERIO -> FDC
  hw/block/fdc: Replace disabled fprintf() by trace event
  hw/block/fdc: Declare shared prototypes in fdc-internal.h
  hw/block/fdc: Extract ISA floppy controllers to fdc-isa.c
  hw/block/fdc: Extract SysBus floppy controllers to fdc-sysbus.c
  hw/block/fdc: Add sysbus_fdc_init_drives() method
  hw/sparc/sun4m: Inline sun4m_fdctrl_init()
  hw/block/fdc-sysbus: Add 'dma-channel' property
  hw/mips/jazz: Inline fdctrl_init_sysbus()

 hw/block/fdc-internal.h | 156 +++++++++++
 include/hw/block/fdc.h  |   7 +-
 hw/block/fdc-isa.c      | 313 +++++++++++++++++++++
 hw/block/fdc-sysbus.c   | 224 +++++++++++++++
 hw/block/fdc.c          | 608 +---------------------------------------
 hw/mips/jazz.c          |  16 ++
 hw/sparc/sun4m.c        |  16 ++
 MAINTAINERS             |   3 +
 hw/block/Kconfig        |   8 +
 hw/block/meson.build    |   2 +
 hw/block/trace-events   |   3 +
 hw/i386/Kconfig         |   2 +-
 hw/isa/Kconfig          |   7 +-
 hw/mips/Kconfig         |   2 +-
 hw/sparc/Kconfig        |   2 +-
 hw/sparc64/Kconfig      |   2 +-
 16 files changed, 759 insertions(+), 612 deletions(-)
 create mode 100644 hw/block/fdc-internal.h
 create mode 100644 hw/block/fdc-isa.c
 create mode 100644 hw/block/fdc-sysbus.c

-- 
2.26.3


Re: [PATCH v4 0/9] hw/block/fdc: Allow Kconfig-selecting ISA bus/SysBus floppy controllers
Posted by John Snow 2 years, 11 months ago
On 5/17/21 2:39 PM, Philippe Mathieu-Daudé wrote:
> Missing review: #1
> 
> Hi,
> 
> The floppy disc controllers pulls in irrelevant devices (sysbus in
> an ISA-only machine, ISA bus + isa devices on a sysbus-only machine).
> 
> This series clean that by extracting each device in its own file,
> adding the corresponding Kconfig symbols: FDC_ISA and FDC_SYSBUS.
> 
> Since v3:
> - Fix ISA_SUPERIO -> FDC Kconfig dependency (jsnow)
> 
> Since v2:
> - rebased
> 
> Since v1:
> - added missing "hw/block/block.h" header (jsnow)
> - inlined hardware specific calls (Mark)
> - added R-b/A-b tags
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (9):
>    hw/isa/Kconfig: Fix missing dependency ISA_SUPERIO -> FDC
>    hw/block/fdc: Replace disabled fprintf() by trace event
>    hw/block/fdc: Declare shared prototypes in fdc-internal.h
>    hw/block/fdc: Extract ISA floppy controllers to fdc-isa.c
>    hw/block/fdc: Extract SysBus floppy controllers to fdc-sysbus.c
>    hw/block/fdc: Add sysbus_fdc_init_drives() method
>    hw/sparc/sun4m: Inline sun4m_fdctrl_init()
>    hw/block/fdc-sysbus: Add 'dma-channel' property
>    hw/mips/jazz: Inline fdctrl_init_sysbus()
> 
>   hw/block/fdc-internal.h | 156 +++++++++++
>   include/hw/block/fdc.h  |   7 +-
>   hw/block/fdc-isa.c      | 313 +++++++++++++++++++++
>   hw/block/fdc-sysbus.c   | 224 +++++++++++++++
>   hw/block/fdc.c          | 608 +---------------------------------------
>   hw/mips/jazz.c          |  16 ++
>   hw/sparc/sun4m.c        |  16 ++
>   MAINTAINERS             |   3 +
>   hw/block/Kconfig        |   8 +
>   hw/block/meson.build    |   2 +
>   hw/block/trace-events   |   3 +
>   hw/i386/Kconfig         |   2 +-
>   hw/isa/Kconfig          |   7 +-
>   hw/mips/Kconfig         |   2 +-
>   hw/sparc/Kconfig        |   2 +-
>   hw/sparc64/Kconfig      |   2 +-
>   16 files changed, 759 insertions(+), 612 deletions(-)
>   create mode 100644 hw/block/fdc-internal.h
>   create mode 100644 hw/block/fdc-isa.c
>   create mode 100644 hw/block/fdc-sysbus.c
> 

Hi, tentatively staged:

https://gitlab.com/jsnow/qemu/-/commits/floppy/

pending CI:

https://gitlab.com/jsnow/qemu/-/pipelines/304308461

--js


Re: [PATCH v4 0/9] hw/block/fdc: Allow Kconfig-selecting ISA bus/SysBus floppy controllers
Posted by Philippe Mathieu-Daudé 2 years, 11 months ago
On 5/17/21 9:19 PM, John Snow wrote:
> On 5/17/21 2:39 PM, Philippe Mathieu-Daudé wrote:
>> Missing review: #1
>>
>> Hi,
>>
>> The floppy disc controllers pulls in irrelevant devices (sysbus in
>> an ISA-only machine, ISA bus + isa devices on a sysbus-only machine).
>>
>> This series clean that by extracting each device in its own file,
>> adding the corresponding Kconfig symbols: FDC_ISA and FDC_SYSBUS.
>>
>> Since v3:
>> - Fix ISA_SUPERIO -> FDC Kconfig dependency (jsnow)
>>
>> Since v2:
>> - rebased
>>
>> Since v1:
>> - added missing "hw/block/block.h" header (jsnow)
>> - inlined hardware specific calls (Mark)
>> - added R-b/A-b tags
>>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (9):
>>    hw/isa/Kconfig: Fix missing dependency ISA_SUPERIO -> FDC
>>    hw/block/fdc: Replace disabled fprintf() by trace event
>>    hw/block/fdc: Declare shared prototypes in fdc-internal.h
>>    hw/block/fdc: Extract ISA floppy controllers to fdc-isa.c
>>    hw/block/fdc: Extract SysBus floppy controllers to fdc-sysbus.c
>>    hw/block/fdc: Add sysbus_fdc_init_drives() method
>>    hw/sparc/sun4m: Inline sun4m_fdctrl_init()
>>    hw/block/fdc-sysbus: Add 'dma-channel' property
>>    hw/mips/jazz: Inline fdctrl_init_sysbus()
>>
>>   hw/block/fdc-internal.h | 156 +++++++++++
>>   include/hw/block/fdc.h  |   7 +-
>>   hw/block/fdc-isa.c      | 313 +++++++++++++++++++++
>>   hw/block/fdc-sysbus.c   | 224 +++++++++++++++
>>   hw/block/fdc.c          | 608 +---------------------------------------
>>   hw/mips/jazz.c          |  16 ++
>>   hw/sparc/sun4m.c        |  16 ++
>>   MAINTAINERS             |   3 +
>>   hw/block/Kconfig        |   8 +
>>   hw/block/meson.build    |   2 +
>>   hw/block/trace-events   |   3 +
>>   hw/i386/Kconfig         |   2 +-
>>   hw/isa/Kconfig          |   7 +-
>>   hw/mips/Kconfig         |   2 +-
>>   hw/sparc/Kconfig        |   2 +-
>>   hw/sparc64/Kconfig      |   2 +-
>>   16 files changed, 759 insertions(+), 612 deletions(-)
>>   create mode 100644 hw/block/fdc-internal.h
>>   create mode 100644 hw/block/fdc-isa.c
>>   create mode 100644 hw/block/fdc-sysbus.c
>>
> 
> Hi, tentatively staged:
> 
> https://gitlab.com/jsnow/qemu/-/commits/floppy/
> 
> pending CI:
> 
> https://gitlab.com/jsnow/qemu/-/pipelines/304308461

Not good enough:

qemu-system-sparc: ../hw/block/fdc.c:2356: fdctrl_realize_common:
Assertion `fdctrl->dma' failed.

Forget about it for your next pull request.


Re: [PATCH v4 0/9] hw/block/fdc: Allow Kconfig-selecting ISA bus/SysBus floppy controllers
Posted by John Snow 2 years, 11 months ago
On 5/17/21 4:50 PM, Philippe Mathieu-Daudé wrote:
> On 5/17/21 9:19 PM, John Snow wrote:
>> On 5/17/21 2:39 PM, Philippe Mathieu-Daudé wrote:
>>> Missing review: #1
>>>
>>> Hi,
>>>
>>> The floppy disc controllers pulls in irrelevant devices (sysbus in
>>> an ISA-only machine, ISA bus + isa devices on a sysbus-only machine).
>>>
>>> This series clean that by extracting each device in its own file,
>>> adding the corresponding Kconfig symbols: FDC_ISA and FDC_SYSBUS.
>>>
>>> Since v3:
>>> - Fix ISA_SUPERIO -> FDC Kconfig dependency (jsnow)
>>>
>>> Since v2:
>>> - rebased
>>>
>>> Since v1:
>>> - added missing "hw/block/block.h" header (jsnow)
>>> - inlined hardware specific calls (Mark)
>>> - added R-b/A-b tags
>>>
>>> Regards,
>>>
>>> Phil.
>>>
>>> Philippe Mathieu-Daudé (9):
>>>     hw/isa/Kconfig: Fix missing dependency ISA_SUPERIO -> FDC
>>>     hw/block/fdc: Replace disabled fprintf() by trace event
>>>     hw/block/fdc: Declare shared prototypes in fdc-internal.h
>>>     hw/block/fdc: Extract ISA floppy controllers to fdc-isa.c
>>>     hw/block/fdc: Extract SysBus floppy controllers to fdc-sysbus.c
>>>     hw/block/fdc: Add sysbus_fdc_init_drives() method
>>>     hw/sparc/sun4m: Inline sun4m_fdctrl_init()
>>>     hw/block/fdc-sysbus: Add 'dma-channel' property
>>>     hw/mips/jazz: Inline fdctrl_init_sysbus()
>>>
>>>    hw/block/fdc-internal.h | 156 +++++++++++
>>>    include/hw/block/fdc.h  |   7 +-
>>>    hw/block/fdc-isa.c      | 313 +++++++++++++++++++++
>>>    hw/block/fdc-sysbus.c   | 224 +++++++++++++++
>>>    hw/block/fdc.c          | 608 +---------------------------------------
>>>    hw/mips/jazz.c          |  16 ++
>>>    hw/sparc/sun4m.c        |  16 ++
>>>    MAINTAINERS             |   3 +
>>>    hw/block/Kconfig        |   8 +
>>>    hw/block/meson.build    |   2 +
>>>    hw/block/trace-events   |   3 +
>>>    hw/i386/Kconfig         |   2 +-
>>>    hw/isa/Kconfig          |   7 +-
>>>    hw/mips/Kconfig         |   2 +-
>>>    hw/sparc/Kconfig        |   2 +-
>>>    hw/sparc64/Kconfig      |   2 +-
>>>    16 files changed, 759 insertions(+), 612 deletions(-)
>>>    create mode 100644 hw/block/fdc-internal.h
>>>    create mode 100644 hw/block/fdc-isa.c
>>>    create mode 100644 hw/block/fdc-sysbus.c
>>>
>>
>> Hi, tentatively staged:
>>
>> https://gitlab.com/jsnow/qemu/-/commits/floppy/
>>
>> pending CI:
>>
>> https://gitlab.com/jsnow/qemu/-/pipelines/304308461
> 
> Not good enough:
> 
> qemu-system-sparc: ../hw/block/fdc.c:2356: fdctrl_realize_common:
> Assertion `fdctrl->dma' failed.
> 
> Forget about it for your next pull request.
> 

Yup, I see. Dropping it from the queue for now. Thanks!

--js


Re: [PATCH v4 0/9] hw/block/fdc: Allow Kconfig-selecting ISA bus/SysBus floppy controllers
Posted by Philippe Mathieu-Daudé 2 years, 11 months ago
Cc'ing Hervé, Aleksandar, Markus & Mark.

On 5/17/21 11:11 PM, John Snow wrote:
> On 5/17/21 4:50 PM, Philippe Mathieu-Daudé wrote:
>> On 5/17/21 9:19 PM, John Snow wrote:
>>> On 5/17/21 2:39 PM, Philippe Mathieu-Daudé wrote:

>>>> The floppy disc controllers pulls in irrelevant devices (sysbus in
>>>> an ISA-only machine, ISA bus + isa devices on a sysbus-only machine).
>>>>
>>>> This series clean that by extracting each device in its own file,
>>>> adding the corresponding Kconfig symbols: FDC_ISA and FDC_SYSBUS.

>> Not good enough:

> Yup, I see. Dropping it from the queue for now. Thanks!

The Jazz machines use the sysbus FDC model, but register a DMA channel.

The DMA transfer is done using:

    if (fdctrl->dma_chann != -1 && !(fdctrl->msr & FD_MSR_NONDMA)) {
        IsaDmaClass *k = ISADMA_GET_CLASS(fdctrl->dma);
        k->release_DREQ(fdctrl->dma, fdctrl->dma_chann);
    }

The IsaDmaTransferHandler is ISA specific...

I suppose the Jazz machines should use the ISA FDC model.

Hervé, Aleksandar, do you know?

Thanks,

Phil.


Re: [PATCH v4 0/9] hw/block/fdc: Allow Kconfig-selecting ISA bus/SysBus floppy controllers
Posted by Hervé Poussineau 2 years, 11 months ago
Le 18/05/2021 à 09:14, Philippe Mathieu-Daudé a écrit :
> Cc'ing Hervé, Aleksandar, Markus & Mark.
> 
> On 5/17/21 11:11 PM, John Snow wrote:
>> On 5/17/21 4:50 PM, Philippe Mathieu-Daudé wrote:
>>> On 5/17/21 9:19 PM, John Snow wrote:
>>>> On 5/17/21 2:39 PM, Philippe Mathieu-Daudé wrote:
> 
>>>>> The floppy disc controllers pulls in irrelevant devices (sysbus in
>>>>> an ISA-only machine, ISA bus + isa devices on a sysbus-only machine).
>>>>>
>>>>> This series clean that by extracting each device in its own file,
>>>>> adding the corresponding Kconfig symbols: FDC_ISA and FDC_SYSBUS.
> 
>>> Not good enough:
> 
>> Yup, I see. Dropping it from the queue for now. Thanks!
> 
> The Jazz machines use the sysbus FDC model, but register a DMA channel.
> 
> The DMA transfer is done using:
> 
>      if (fdctrl->dma_chann != -1 && !(fdctrl->msr & FD_MSR_NONDMA)) {
>          IsaDmaClass *k = ISADMA_GET_CLASS(fdctrl->dma);
>          k->release_DREQ(fdctrl->dma, fdctrl->dma_chann);
>      }
> 
> The IsaDmaTransferHandler is ISA specific...
> 
> I suppose the Jazz machines should use the ISA FDC model.
 >
 > Hervé, Aleksandar, do you know?

The Jazz machine uses the standard floppy controller (i82077)
FDC registers are accessible at memory-mapped address 0x80003000-0x80003007
FDC interrupt is using custom Jazz interrupt controller, with interrupt #1.
FDC DMA is using custom Jazz DMA channel controller, with channel #1.

You can find in jazz.c the following code:
     /* FIXME: we should enable DMA with a custom IsaDma device */
     fdctrl_init_sysbus(qdev_get_gpio_in(rc4030, 1), -1, 0x80003000, fds);

I had in the idea to wrap the custom Jazz interrupt controller in a IsaDma structure,
to use it with floppy controller, but I never took the time to do it + test it.

So, floppy drive never worked for me, neither with Windows NT 4, nor with Linux kernels.
You can do what you feel the best, as long as you keep the possibility to use the floppy
model with something else than a i8259 DMA controller.

Regards,

Hervé