[PATCH 00/20] hw/ide: QOM/QDev housekeeping

[PATCH 00/20] hw/ide: QOM/QDev housekeeping

[Philippe Mathieu-Daudé]

Mostly trivial cleanups in preparation of a v2 of
"hw/ide: Untangle ISA/PCI abuses of ide_init_ioport()":
https://lore.kernel.org/qemu-devel/20230208000743.79415-1-philmd@linaro.org/

Bernhard Beschow (1):
  hw/ide/pci: Unexport bmdma_active_if()

John Snow (1):
  MAINTAINERS: Mark IDE and Floppy as "Odd Fixes"

Philippe Mathieu-Daudé (18):
  hw/ide/mmio: Use CamelCase for MMIO_IDE state name
  hw/ide/mmio: Extract TYPE_MMIO_IDE declarations to 'hw/ide/mmio.h'
  hw/ide/isa: Extract TYPE_ISA_IDE declarations to 'hw/ide/isa.h'
  hw/ide/isa: Remove intermediate ISAIDEState::irq variable
  hw/ide/atapi: Restrict 'scsi/constants.h' inclusion
  hw/ide: Remove unused 'qapi/qapi-types-run-state.h'
  hw/ide: Include 'exec/ioport.h' instead of 'hw/isa/isa.h'
  hw/ide: Un-inline ide_set_irq()
  hw/ide: Rename ide_set_irq() -> ide_bus_set_irq()
  hw/ide: Rename ide_create_drive() -> ide_bus_create_drive()
  hw/ide: Rename ide_register_restart_cb -> ide_bus_register_restart_cb
  hw/ide: Rename ide_exec_cmd() -> ide_bus_exec_cmd()
  hw/ide: Rename ide_init2() -> ide_bus_init_output_irq()
  hw/ide: Rename idebus_active_if() -> ide_bus_active_if()
  hw/ide/ioport: Remove unnecessary includes
  hw/ide/piix: Remove unused includes
  hw/ide/piix: Pass Error* to pci_piix_init_ports() for better error msg
  hw/ide/piix: Refactor pci_piix_init_ports as pci_piix_init_bus per bus

 MAINTAINERS               |  4 +-
 hw/arm/sbsa-ref.c         |  2 +-
 hw/i386/pc_piix.c         |  1 +
 hw/ide/ahci.c             |  9 +++--
 hw/ide/atapi.c            | 13 ++++---
 hw/ide/cmd646.c           |  4 +-
 hw/ide/core.c             | 80 +++++++++++++++++++++------------------
 hw/ide/ich.c              |  1 +
 hw/ide/ioport.c           | 10 -----
 hw/ide/isa.c              | 22 +++++------
 hw/ide/macio.c            | 15 ++++----
 hw/ide/microdrive.c       |  9 +++--
 hw/ide/mmio.c             | 37 ++++++++----------
 hw/ide/pci.c              | 11 +++++-
 hw/ide/piix.c             | 46 ++++++++++------------
 hw/ide/qdev.c             |  2 +-
 hw/ide/sii3112.c          |  4 +-
 hw/ide/trace-events       |  2 +-
 hw/ide/via.c              |  4 +-
 hw/misc/macio/gpio.c      |  1 +
 hw/sh4/r2d.c              |  2 +-
 hw/sparc64/sun4u.c        |  1 +
 include/hw/ide.h          |  8 ----
 include/hw/ide/internal.h | 25 ++++--------
 include/hw/ide/isa.h      | 20 ++++++++++
 include/hw/ide/mmio.h     | 26 +++++++++++++
 include/hw/ide/pci.h      |  6 ---
 27 files changed, 193 insertions(+), 172 deletions(-)
 create mode 100644 include/hw/ide/isa.h
 create mode 100644 include/hw/ide/mmio.h

-- 
2.38.1

[PATCH 01/20] MAINTAINERS: Mark IDE and Floppy as "Odd Fixes"

[Philippe Mathieu-Daudé]

From: John Snow <jsnow@redhat.com>

I have not been able to give these devices the love they need for a
while now. Update the maintainers file to reflect the truth of the
matter.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20230206182544.711117-1-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 96e25f62ac..841aa3e021 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1792,7 +1792,7 @@ F: hw/misc/edu.c
 IDE
 M: John Snow <jsnow@redhat.com>
 L: qemu-block@nongnu.org
-S: Supported
+S: Odd Fixes
 F: include/hw/ide.h
 F: include/hw/ide/
 F: hw/ide/
@@ -1817,7 +1817,7 @@ T: git https://github.com/cminyard/qemu.git master-ipmi-rebase
 Floppy
 M: John Snow <jsnow@redhat.com>
 L: qemu-block@nongnu.org
-S: Supported
+S: Odd Fixes
 F: hw/block/fdc.c
 F: hw/block/fdc-internal.h
 F: hw/block/fdc-isa.c
-- 
2.38.1

Re: [PATCH 01/20] MAINTAINERS: Mark IDE and Floppy as "Odd Fixes"

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> From: John Snow <jsnow@redhat.com>
>
> I have not been able to give these devices the love they need for a
> while now. Update the maintainers file to reflect the truth of the
> matter.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Message-Id: <20230206182544.711117-1-jsnow@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 01/20] MAINTAINERS: Mark IDE and Floppy as "Odd Fixes"

[Philippe Mathieu-Daudé]

On 15/2/23 12:26, Philippe Mathieu-Daudé wrote:
> From: John Snow <jsnow@redhat.com>
> 
> I have not been able to give these devices the love they need for a
> while now. Update the maintainers file to reflect the truth of the
> matter.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> Message-Id: <20230206182544.711117-1-jsnow@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   MAINTAINERS | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 02/20] hw/ide/mmio: Use CamelCase for MMIO_IDE state name

[Philippe Mathieu-Daudé]

Following docs/devel/style.rst guidelines, rename MMIOIDEState
as IdeMmioState.

Having the structure name and its typedef named equally,
we can manually convert from the old DECLARE_INSTANCE_CHECKER()
macro to the more recent OBJECT_DECLARE_SIMPLE_TYPE().

Note, due to that name mismatch, this macro wasn't automatically
converted during commit 8063396bf3 ("Use OBJECT_DECLARE_SIMPLE_TYPE
when possible").

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/mmio.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
index fb2ebd4847..f1c6e1479b 100644
--- a/hw/ide/mmio.c
+++ b/hw/ide/mmio.c
@@ -40,9 +40,7 @@
  */
 
 #define TYPE_MMIO_IDE "mmio-ide"
-typedef struct MMIOIDEState MMIOState;
-DECLARE_INSTANCE_CHECKER(MMIOState, MMIO_IDE,
-                         TYPE_MMIO_IDE)
+OBJECT_DECLARE_SIMPLE_TYPE(MMIOIDEState, MMIO_IDE)
 
 struct MMIOIDEState {
     /*< private >*/
@@ -58,7 +56,7 @@ struct MMIOIDEState {
 
 static void mmio_ide_reset(DeviceState *dev)
 {
-    MMIOState *s = MMIO_IDE(dev);
+    MMIOIDEState *s = MMIO_IDE(dev);
 
     ide_bus_reset(&s->bus);
 }
@@ -66,7 +64,7 @@ static void mmio_ide_reset(DeviceState *dev)
 static uint64_t mmio_ide_read(void *opaque, hwaddr addr,
                               unsigned size)
 {
-    MMIOState *s = opaque;
+    MMIOIDEState *s = opaque;
     addr >>= s->shift;
     if (addr & 7)
         return ide_ioport_read(&s->bus, addr);
@@ -77,7 +75,7 @@ static uint64_t mmio_ide_read(void *opaque, hwaddr addr,
 static void mmio_ide_write(void *opaque, hwaddr addr,
                            uint64_t val, unsigned size)
 {
-    MMIOState *s = opaque;
+    MMIOIDEState *s = opaque;
     addr >>= s->shift;
     if (addr & 7)
         ide_ioport_write(&s->bus, addr, val);
@@ -94,14 +92,14 @@ static const MemoryRegionOps mmio_ide_ops = {
 static uint64_t mmio_ide_status_read(void *opaque, hwaddr addr,
                                      unsigned size)
 {
-    MMIOState *s= opaque;
+    MMIOIDEState *s= opaque;
     return ide_status_read(&s->bus, 0);
 }
 
 static void mmio_ide_ctrl_write(void *opaque, hwaddr addr,
                                 uint64_t val, unsigned size)
 {
-    MMIOState *s = opaque;
+    MMIOIDEState *s = opaque;
     ide_ctrl_write(&s->bus, 0, val);
 }
 
@@ -116,8 +114,8 @@ static const VMStateDescription vmstate_ide_mmio = {
     .version_id = 3,
     .minimum_version_id = 0,
     .fields = (VMStateField[]) {
-        VMSTATE_IDE_BUS(bus, MMIOState),
-        VMSTATE_IDE_DRIVES(bus.ifs, MMIOState),
+        VMSTATE_IDE_BUS(bus, MMIOIDEState),
+        VMSTATE_IDE_DRIVES(bus.ifs, MMIOIDEState),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -125,7 +123,7 @@ static const VMStateDescription vmstate_ide_mmio = {
 static void mmio_ide_realizefn(DeviceState *dev, Error **errp)
 {
     SysBusDevice *d = SYS_BUS_DEVICE(dev);
-    MMIOState *s = MMIO_IDE(dev);
+    MMIOIDEState *s = MMIO_IDE(dev);
 
     ide_init2(&s->bus, s->irq);
 
@@ -140,14 +138,14 @@ static void mmio_ide_realizefn(DeviceState *dev, Error **errp)
 static void mmio_ide_initfn(Object *obj)
 {
     SysBusDevice *d = SYS_BUS_DEVICE(obj);
-    MMIOState *s = MMIO_IDE(obj);
+    MMIOIDEState *s = MMIO_IDE(obj);
 
     ide_bus_init(&s->bus, sizeof(s->bus), DEVICE(obj), 0, 2);
     sysbus_init_irq(d, &s->irq);
 }
 
 static Property mmio_ide_properties[] = {
-    DEFINE_PROP_UINT32("shift", MMIOState, shift, 0),
+    DEFINE_PROP_UINT32("shift", MMIOIDEState, shift, 0),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -164,7 +162,7 @@ static void mmio_ide_class_init(ObjectClass *oc, void *data)
 static const TypeInfo mmio_ide_type_info = {
     .name = TYPE_MMIO_IDE,
     .parent = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(MMIOState),
+    .instance_size = sizeof(MMIOIDEState),
     .instance_init = mmio_ide_initfn,
     .class_init = mmio_ide_class_init,
 };
@@ -176,7 +174,7 @@ static void mmio_ide_register_types(void)
 
 void mmio_ide_init_drives(DeviceState *dev, DriveInfo *hd0, DriveInfo *hd1)
 {
-    MMIOState *s = MMIO_IDE(dev);
+    MMIOIDEState *s = MMIO_IDE(dev);
 
     if (hd0 != NULL) {
         ide_create_drive(&s->bus, 0, hd0);
-- 
2.38.1

Re: [PATCH 02/20] hw/ide/mmio: Use CamelCase for MMIO_IDE state name

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Following docs/devel/style.rst guidelines, rename MMIOIDEState
> as IdeMmioState.

Erm the comment doesn't match what you've done s/MMIOState/MMIOIDEState/

>
> Having the structure name and its typedef named equally, we can
> manually convert from the old DECLARE_INSTANCE_CHECKER() macro to the
> more recent OBJECT_DECLARE_SIMPLE_TYPE().
>
> Note, due to that name mismatch, this macro wasn't automatically
> converted during commit 8063396bf3 ("Use OBJECT_DECLARE_SIMPLE_TYPE
> when possible").
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/ide/mmio.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
> index fb2ebd4847..f1c6e1479b 100644
> --- a/hw/ide/mmio.c
> +++ b/hw/ide/mmio.c
> @@ -40,9 +40,7 @@
>   */
>  
>  #define TYPE_MMIO_IDE "mmio-ide"
> -typedef struct MMIOIDEState MMIOState;
> -DECLARE_INSTANCE_CHECKER(MMIOState, MMIO_IDE,
> -                         TYPE_MMIO_IDE)
> +OBJECT_DECLARE_SIMPLE_TYPE(MMIOIDEState, MMIO_IDE)
>  
>  struct MMIOIDEState {
>      /*< private >*/
> @@ -58,7 +56,7 @@ struct MMIOIDEState {
>  
>  static void mmio_ide_reset(DeviceState *dev)
>  {
> -    MMIOState *s = MMIO_IDE(dev);
> +    MMIOIDEState *s = MMIO_IDE(dev);
>  
>      ide_bus_reset(&s->bus);
>  }
> @@ -66,7 +64,7 @@ static void mmio_ide_reset(DeviceState *dev)
>  static uint64_t mmio_ide_read(void *opaque, hwaddr addr,
>                                unsigned size)
>  {
> -    MMIOState *s = opaque;
> +    MMIOIDEState *s = opaque;
>      addr >>= s->shift;
>      if (addr & 7)
>          return ide_ioport_read(&s->bus, addr);
> @@ -77,7 +75,7 @@ static uint64_t mmio_ide_read(void *opaque, hwaddr addr,
>  static void mmio_ide_write(void *opaque, hwaddr addr,
>                             uint64_t val, unsigned size)
>  {
> -    MMIOState *s = opaque;
> +    MMIOIDEState *s = opaque;
>      addr >>= s->shift;
>      if (addr & 7)
>          ide_ioport_write(&s->bus, addr, val);
> @@ -94,14 +92,14 @@ static const MemoryRegionOps mmio_ide_ops = {
>  static uint64_t mmio_ide_status_read(void *opaque, hwaddr addr,
>                                       unsigned size)
>  {
> -    MMIOState *s= opaque;
> +    MMIOIDEState *s= opaque;
>      return ide_status_read(&s->bus, 0);
>  }
>  
>  static void mmio_ide_ctrl_write(void *opaque, hwaddr addr,
>                                  uint64_t val, unsigned size)
>  {
> -    MMIOState *s = opaque;
> +    MMIOIDEState *s = opaque;
>      ide_ctrl_write(&s->bus, 0, val);
>  }
>  
> @@ -116,8 +114,8 @@ static const VMStateDescription vmstate_ide_mmio = {
>      .version_id = 3,
>      .minimum_version_id = 0,
>      .fields = (VMStateField[]) {
> -        VMSTATE_IDE_BUS(bus, MMIOState),
> -        VMSTATE_IDE_DRIVES(bus.ifs, MMIOState),
> +        VMSTATE_IDE_BUS(bus, MMIOIDEState),
> +        VMSTATE_IDE_DRIVES(bus.ifs, MMIOIDEState),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -125,7 +123,7 @@ static const VMStateDescription vmstate_ide_mmio = {
>  static void mmio_ide_realizefn(DeviceState *dev, Error **errp)
>  {
>      SysBusDevice *d = SYS_BUS_DEVICE(dev);
> -    MMIOState *s = MMIO_IDE(dev);
> +    MMIOIDEState *s = MMIO_IDE(dev);
>  
>      ide_init2(&s->bus, s->irq);
>  
> @@ -140,14 +138,14 @@ static void mmio_ide_realizefn(DeviceState *dev, Error **errp)
>  static void mmio_ide_initfn(Object *obj)
>  {
>      SysBusDevice *d = SYS_BUS_DEVICE(obj);
> -    MMIOState *s = MMIO_IDE(obj);
> +    MMIOIDEState *s = MMIO_IDE(obj);
>  
>      ide_bus_init(&s->bus, sizeof(s->bus), DEVICE(obj), 0, 2);
>      sysbus_init_irq(d, &s->irq);
>  }
>  
>  static Property mmio_ide_properties[] = {
> -    DEFINE_PROP_UINT32("shift", MMIOState, shift, 0),
> +    DEFINE_PROP_UINT32("shift", MMIOIDEState, shift, 0),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> @@ -164,7 +162,7 @@ static void mmio_ide_class_init(ObjectClass *oc, void *data)
>  static const TypeInfo mmio_ide_type_info = {
>      .name = TYPE_MMIO_IDE,
>      .parent = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(MMIOState),
> +    .instance_size = sizeof(MMIOIDEState),
>      .instance_init = mmio_ide_initfn,
>      .class_init = mmio_ide_class_init,
>  };
> @@ -176,7 +174,7 @@ static void mmio_ide_register_types(void)
>  
>  void mmio_ide_init_drives(DeviceState *dev, DriveInfo *hd0, DriveInfo *hd1)
>  {
> -    MMIOState *s = MMIO_IDE(dev);
> +    MMIOIDEState *s = MMIO_IDE(dev);
>  
>      if (hd0 != NULL) {
>          ide_create_drive(&s->bus, 0, hd0);


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 03/20] hw/ide/mmio: Extract TYPE_MMIO_IDE declarations to 'hw/ide/mmio.h'

[Philippe Mathieu-Daudé]

"hw/ide.h" is a mixed bag of lost IDE declarations.

Extract mmio_ide_init_drives() and the TYPE_MMIO_IDE QOM
declarations to a new "hw/ide/mmio.h" header.

Document the SysBus interface.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/mmio.c         |  5 +----
 hw/sh4/r2d.c          |  2 +-
 include/hw/ide.h      |  3 ---
 include/hw/ide/mmio.h | 26 ++++++++++++++++++++++++++
 4 files changed, 28 insertions(+), 8 deletions(-)
 create mode 100644 include/hw/ide/mmio.h

diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
index f1c6e1479b..5153d19ac6 100644
--- a/hw/ide/mmio.c
+++ b/hw/ide/mmio.c
@@ -29,9 +29,9 @@
 #include "qemu/module.h"
 #include "sysemu/dma.h"
 
+#include "hw/ide/mmio.h"
 #include "hw/ide/internal.h"
 #include "hw/qdev-properties.h"
-#include "qom/object.h"
 
 /***********************************************************/
 /* MMIO based ide port
@@ -39,9 +39,6 @@
  * dedicated ide controller, which is often seen on embedded boards.
  */
 
-#define TYPE_MMIO_IDE "mmio-ide"
-OBJECT_DECLARE_SIMPLE_TYPE(MMIOIDEState, MMIO_IDE)
-
 struct MMIOIDEState {
     /*< private >*/
     SysBusDevice parent_obj;
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index 39fc4f19d9..b96c6a939a 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -38,7 +38,7 @@
 #include "hw/qdev-properties.h"
 #include "net/net.h"
 #include "sh7750_regs.h"
-#include "hw/ide.h"
+#include "hw/ide/mmio.h"
 #include "hw/irq.h"
 #include "hw/loader.h"
 #include "hw/usb.h"
diff --git a/include/hw/ide.h b/include/hw/ide.h
index 60f1f4f714..5f8c36b2aa 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -8,9 +8,6 @@
 ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
                         DriveInfo *hd0, DriveInfo *hd1);
 
-/* ide-mmio.c */
-void mmio_ide_init_drives(DeviceState *dev, DriveInfo *hd0, DriveInfo *hd1);
-
 int ide_get_geometry(BusState *bus, int unit,
                      int16_t *cyls, int8_t *heads, int8_t *secs);
 int ide_get_bios_chs_trans(BusState *bus, int unit);
diff --git a/include/hw/ide/mmio.h b/include/hw/ide/mmio.h
new file mode 100644
index 0000000000..d726a49848
--- /dev/null
+++ b/include/hw/ide/mmio.h
@@ -0,0 +1,26 @@
+/*
+ * QEMU IDE Emulation: mmio support (for embedded).
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef HW_IDE_MMIO_H
+#define HW_IDE_MMIO_H
+
+#include "qom/object.h"
+
+/*
+ * QEMU interface:
+ *  + sysbus IRQ 0: asserted by the IDE channel
+ *  + sysbus MMIO region 0: data registers
+ *  + sysbus MMIO region 1: status & control registers
+ */
+#define TYPE_MMIO_IDE "mmio-ide"
+OBJECT_DECLARE_SIMPLE_TYPE(MMIOIDEState, MMIO_IDE)
+
+void mmio_ide_init_drives(DeviceState *dev, DriveInfo *hd0, DriveInfo *hd1);
+
+#endif
-- 
2.38.1

Re: [PATCH 03/20] hw/ide/mmio: Extract TYPE_MMIO_IDE declarations to 'hw/ide/mmio.h'

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> "hw/ide.h" is a mixed bag of lost IDE declarations.
>
> Extract mmio_ide_init_drives() and the TYPE_MMIO_IDE QOM
> declarations to a new "hw/ide/mmio.h" header.
>
> Document the SysBus interface.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 03/20] hw/ide/mmio: Extract TYPE_MMIO_IDE declarations to 'hw/ide/mmio.h'

[Richard Henderson]

On 2/15/23 01:26, Philippe Mathieu-Daudé wrote:
> "hw/ide.h" is a mixed bag of lost IDE declarations.
> 
> Extract mmio_ide_init_drives() and the TYPE_MMIO_IDE QOM
> declarations to a new "hw/ide/mmio.h" header.
> 
> Document the SysBus interface.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/mmio.c         |  5 +----
>   hw/sh4/r2d.c          |  2 +-
>   include/hw/ide.h      |  3 ---
>   include/hw/ide/mmio.h | 26 ++++++++++++++++++++++++++
>   4 files changed, 28 insertions(+), 8 deletions(-)
>   create mode 100644 include/hw/ide/mmio.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 04/20] hw/ide/isa: Extract TYPE_ISA_IDE declarations to 'hw/ide/isa.h'

[Philippe Mathieu-Daudé]

"hw/ide.h" is a mixed bag of lost IDE declarations.

Extract isa_ide_init() and the TYPE_ISA_IDE QOM declarations
to a new "hw/ide/isa.h" header.

Rename ISAIDEState::isairq as 'irqnum' to emphasize this is
not a qemu_irq object but the number (index) of an ISA IRQ.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c    |  1 +
 hw/ide/isa.c         | 14 ++++++--------
 include/hw/ide.h     |  5 -----
 include/hw/ide/isa.h | 20 ++++++++++++++++++++
 4 files changed, 27 insertions(+), 13 deletions(-)
 create mode 100644 include/hw/ide/isa.h

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index df64dd8dcc..7085b4bc58 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -39,6 +39,7 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/usb.h"
 #include "net/net.h"
+#include "hw/ide/isa.h"
 #include "hw/ide/pci.h"
 #include "hw/ide/piix.h"
 #include "hw/irq.h"
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 8bedbd13f1..5c3e83a0fc 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -31,22 +31,20 @@
 #include "qemu/module.h"
 #include "sysemu/dma.h"
 
+#include "hw/ide/isa.h"
 #include "hw/ide/internal.h"
 #include "qom/object.h"
 
 /***********************************************************/
 /* ISA IDE definitions */
 
-#define TYPE_ISA_IDE "isa-ide"
-OBJECT_DECLARE_SIMPLE_TYPE(ISAIDEState, ISA_IDE)
-
 struct ISAIDEState {
     ISADevice parent_obj;
 
     IDEBus    bus;
     uint32_t  iobase;
     uint32_t  iobase2;
-    uint32_t  isairq;
+    uint32_t  irqnum;
     qemu_irq  irq;
 };
 
@@ -75,13 +73,13 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
 
     ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2);
     ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
-    s->irq = isa_get_irq(isadev, s->isairq);
+    s->irq = isa_get_irq(isadev, s->irqnum);
     ide_init2(&s->bus, s->irq);
     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
     ide_register_restart_cb(&s->bus);
 }
 
-ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
+ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int irqnum,
                         DriveInfo *hd0, DriveInfo *hd1)
 {
     DeviceState *dev;
@@ -92,7 +90,7 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
     dev = DEVICE(isadev);
     qdev_prop_set_uint32(dev, "iobase",  iobase);
     qdev_prop_set_uint32(dev, "iobase2", iobase2);
-    qdev_prop_set_uint32(dev, "irq",     isairq);
+    qdev_prop_set_uint32(dev, "irq",     irqnum);
     isa_realize_and_unref(isadev, bus, &error_fatal);
 
     s = ISA_IDE(dev);
@@ -108,7 +106,7 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
 static Property isa_ide_properties[] = {
     DEFINE_PROP_UINT32("iobase",  ISAIDEState, iobase,  0x1f0),
     DEFINE_PROP_UINT32("iobase2", ISAIDEState, iobase2, 0x3f6),
-    DEFINE_PROP_UINT32("irq",    ISAIDEState, isairq,  14),
+    DEFINE_PROP_UINT32("irq",     ISAIDEState, irqnum,  14),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/ide.h b/include/hw/ide.h
index 5f8c36b2aa..24a7aa2925 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -1,13 +1,8 @@
 #ifndef HW_IDE_H
 #define HW_IDE_H
 
-#include "hw/isa/isa.h"
 #include "exec/memory.h"
 
-/* ide-isa.c */
-ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
-                        DriveInfo *hd0, DriveInfo *hd1);
-
 int ide_get_geometry(BusState *bus, int unit,
                      int16_t *cyls, int8_t *heads, int8_t *secs);
 int ide_get_bios_chs_trans(BusState *bus, int unit);
diff --git a/include/hw/ide/isa.h b/include/hw/ide/isa.h
new file mode 100644
index 0000000000..1cd0ff1fa6
--- /dev/null
+++ b/include/hw/ide/isa.h
@@ -0,0 +1,20 @@
+/*
+ * QEMU IDE Emulation: ISA Bus support.
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+#ifndef HW_IDE_ISA_H
+#define HW_IDE_ISA_H
+
+#include "qom/object.h"
+
+#define TYPE_ISA_IDE "isa-ide"
+OBJECT_DECLARE_SIMPLE_TYPE(ISAIDEState, ISA_IDE)
+
+ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int irqnum,
+                        DriveInfo *hd0, DriveInfo *hd1);
+
+#endif
-- 
2.38.1

Re: [PATCH 04/20] hw/ide/isa: Extract TYPE_ISA_IDE declarations to 'hw/ide/isa.h'

[Richard Henderson]

On 2/15/23 01:26, Philippe Mathieu-Daudé wrote:
> "hw/ide.h" is a mixed bag of lost IDE declarations.
> 
> Extract isa_ide_init() and the TYPE_ISA_IDE QOM declarations
> to a new "hw/ide/isa.h" header.
> 
> Rename ISAIDEState::isairq as 'irqnum' to emphasize this is
> not a qemu_irq object but the number (index) of an ISA IRQ.

This is fine, but should be separate from the header creation.

With the split, for both patches,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 04/20] hw/ide/isa: Extract TYPE_ISA_IDE declarations to 'hw/ide/isa.h'

[Bernhard Beschow]

Am 15. Februar 2023 11:26:56 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>"hw/ide.h" is a mixed bag of lost IDE declarations.
>
>Extract isa_ide_init() and the TYPE_ISA_IDE QOM declarations
>to a new "hw/ide/isa.h" header.
>
>Rename ISAIDEState::isairq as 'irqnum' to emphasize this is
>not a qemu_irq object but the number (index) of an ISA IRQ.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/i386/pc_piix.c    |  1 +
> hw/ide/isa.c         | 14 ++++++--------
> include/hw/ide.h     |  5 -----
> include/hw/ide/isa.h | 20 ++++++++++++++++++++
> 4 files changed, 27 insertions(+), 13 deletions(-)
> create mode 100644 include/hw/ide/isa.h
>
>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>index df64dd8dcc..7085b4bc58 100644
>--- a/hw/i386/pc_piix.c
>+++ b/hw/i386/pc_piix.c
>@@ -39,6 +39,7 @@
> #include "hw/pci/pci_ids.h"
> #include "hw/usb.h"
> #include "net/net.h"
>+#include "hw/ide/isa.h"
> #include "hw/ide/pci.h"
> #include "hw/ide/piix.h"
> #include "hw/irq.h"
>diff --git a/hw/ide/isa.c b/hw/ide/isa.c
>index 8bedbd13f1..5c3e83a0fc 100644
>--- a/hw/ide/isa.c
>+++ b/hw/ide/isa.c
>@@ -31,22 +31,20 @@
> #include "qemu/module.h"
> #include "sysemu/dma.h"
> 
>+#include "hw/ide/isa.h"
> #include "hw/ide/internal.h"
> #include "qom/object.h"
> 
> /***********************************************************/
> /* ISA IDE definitions */
> 
>-#define TYPE_ISA_IDE "isa-ide"
>-OBJECT_DECLARE_SIMPLE_TYPE(ISAIDEState, ISA_IDE)
>-
> struct ISAIDEState {
>     ISADevice parent_obj;
> 
>     IDEBus    bus;
>     uint32_t  iobase;
>     uint32_t  iobase2;
>-    uint32_t  isairq;
>+    uint32_t  irqnum;
>     qemu_irq  irq;
> };
> 
>@@ -75,13 +73,13 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
> 
>     ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2);
>     ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
>-    s->irq = isa_get_irq(isadev, s->isairq);
>+    s->irq = isa_get_irq(isadev, s->irqnum);
>     ide_init2(&s->bus, s->irq);
>     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
>     ide_register_restart_cb(&s->bus);
> }
> 
>-ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
>+ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int irqnum,
>                         DriveInfo *hd0, DriveInfo *hd1)
> {
>     DeviceState *dev;
>@@ -92,7 +90,7 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
>     dev = DEVICE(isadev);
>     qdev_prop_set_uint32(dev, "iobase",  iobase);
>     qdev_prop_set_uint32(dev, "iobase2", iobase2);
>-    qdev_prop_set_uint32(dev, "irq",     isairq);
>+    qdev_prop_set_uint32(dev, "irq",     irqnum);
>     isa_realize_and_unref(isadev, bus, &error_fatal);
> 
>     s = ISA_IDE(dev);
>@@ -108,7 +106,7 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
> static Property isa_ide_properties[] = {
>     DEFINE_PROP_UINT32("iobase",  ISAIDEState, iobase,  0x1f0),
>     DEFINE_PROP_UINT32("iobase2", ISAIDEState, iobase2, 0x3f6),
>-    DEFINE_PROP_UINT32("irq",    ISAIDEState, isairq,  14),
>+    DEFINE_PROP_UINT32("irq",     ISAIDEState, irqnum,  14),
>     DEFINE_PROP_END_OF_LIST(),
> };
> 
>diff --git a/include/hw/ide.h b/include/hw/ide.h
>index 5f8c36b2aa..24a7aa2925 100644
>--- a/include/hw/ide.h
>+++ b/include/hw/ide.h
>@@ -1,13 +1,8 @@
> #ifndef HW_IDE_H
> #define HW_IDE_H
> 
>-#include "hw/isa/isa.h"
> #include "exec/memory.h"
> 
>-/* ide-isa.c */
>-ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
>-                        DriveInfo *hd0, DriveInfo *hd1);
>-
> int ide_get_geometry(BusState *bus, int unit,
>                      int16_t *cyls, int8_t *heads, int8_t *secs);
> int ide_get_bios_chs_trans(BusState *bus, int unit);
>diff --git a/include/hw/ide/isa.h b/include/hw/ide/isa.h
>new file mode 100644
>index 0000000000..1cd0ff1fa6
>--- /dev/null
>+++ b/include/hw/ide/isa.h
>@@ -0,0 +1,20 @@
>+/*
>+ * QEMU IDE Emulation: ISA Bus support.
>+ *
>+ * Copyright (c) 2003 Fabrice Bellard
>+ * Copyright (c) 2006 Openedhand Ltd.
>+ *
>+ * SPDX-License-Identifier: MIT
>+ */
>+#ifndef HW_IDE_ISA_H
>+#define HW_IDE_ISA_H
>+
>+#include "qom/object.h"
>+
>+#define TYPE_ISA_IDE "isa-ide"
>+OBJECT_DECLARE_SIMPLE_TYPE(ISAIDEState, ISA_IDE)
>+
>+ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int irqnum,
>+                        DriveInfo *hd0, DriveInfo *hd1);
>+
>+#endif

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

[PATCH 05/20] hw/ide/isa: Remove intermediate ISAIDEState::irq variable

[Philippe Mathieu-Daudé]

The intermediate ISAIDEState::irq variable just add noise, remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/isa.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 5c3e83a0fc..ad47e0899e 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -45,7 +45,6 @@ struct ISAIDEState {
     uint32_t  iobase;
     uint32_t  iobase2;
     uint32_t  irqnum;
-    qemu_irq  irq;
 };
 
 static void isa_ide_reset(DeviceState *d)
@@ -73,8 +72,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
 
     ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2);
     ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
-    s->irq = isa_get_irq(isadev, s->irqnum);
-    ide_init2(&s->bus, s->irq);
+    ide_init2(&s->bus, isa_get_irq(isadev, s->irqnum));
     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
     ide_register_restart_cb(&s->bus);
 }
-- 
2.38.1

Re: [PATCH 05/20] hw/ide/isa: Remove intermediate ISAIDEState::irq variable

[Bernhard Beschow]

Am 15. Februar 2023 11:26:57 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>The intermediate ISAIDEState::irq variable just add noise, remove it.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/ide/isa.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/hw/ide/isa.c b/hw/ide/isa.c
>index 5c3e83a0fc..ad47e0899e 100644
>--- a/hw/ide/isa.c
>+++ b/hw/ide/isa.c
>@@ -45,7 +45,6 @@ struct ISAIDEState {
>     uint32_t  iobase;
>     uint32_t  iobase2;
>     uint32_t  irqnum;
>-    qemu_irq  irq;
> };
> 
> static void isa_ide_reset(DeviceState *d)
>@@ -73,8 +72,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
> 
>     ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2);
>     ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
>-    s->irq = isa_get_irq(isadev, s->irqnum);
>-    ide_init2(&s->bus, s->irq);
>+    ide_init2(&s->bus, isa_get_irq(isadev, s->irqnum));
>     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
>     ide_register_restart_cb(&s->bus);
> }

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

Re: [PATCH 05/20] hw/ide/isa: Remove intermediate ISAIDEState::irq variable

[Richard Henderson]

On 2/15/23 01:26, Philippe Mathieu-Daudé wrote:
> The intermediate ISAIDEState::irq variable just add noise, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/isa.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 06/20] hw/ide/atapi: Restrict 'scsi/constants.h' inclusion

[Philippe Mathieu-Daudé]

Only atapi.c requires the SCSI constants. No need to include
it in all files including "hw/ide/internal.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/atapi.c            | 1 +
 include/hw/ide/internal.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 0a9aa6f009..0c36bd0afd 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -27,6 +27,7 @@
 #include "hw/ide/internal.h"
 #include "hw/scsi/scsi.h"
 #include "sysemu/block-backend.h"
+#include "scsi/constants.h"
 #include "trace.h"
 
 #define ATAPI_SECTOR_BITS (2 + BDRV_SECTOR_BITS)
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index fc0aa81a88..29a8e79817 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -13,7 +13,6 @@
 #include "hw/isa/isa.h"
 #include "sysemu/dma.h"
 #include "hw/block/block.h"
-#include "scsi/constants.h"
 
 /* debug IDE devices */
 #define USE_DMA_CDROM
-- 
2.38.1

Re: [PATCH 06/20] hw/ide/atapi: Restrict 'scsi/constants.h' inclusion

[Richard Henderson]

On 2/15/23 01:26, Philippe Mathieu-Daudé wrote:
> Only atapi.c requires the SCSI constants. No need to include
> it in all files including "hw/ide/internal.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/atapi.c            | 1 +
>   include/hw/ide/internal.h | 1 -
>   2 files changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 07/20] hw/ide: Remove unused 'qapi/qapi-types-run-state.h'

[Philippe Mathieu-Daudé]

Missed in commit d7458e7754 ("hw/ide/internal: Remove unused
DMARestartFunc typedef") which removed the single use of RunState.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/ide/internal.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 29a8e79817..e6deb1c5dc 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -7,7 +7,6 @@
  * non-internal declarations are in hw/ide.h
  */
 
-#include "qapi/qapi-types-run-state.h"
 #include "hw/ide.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
-- 
2.38.1

Re: [PATCH 07/20] hw/ide: Remove unused 'qapi/qapi-types-run-state.h'

[Richard Henderson]

On 2/15/23 01:26, Philippe Mathieu-Daudé wrote:
> Missed in commit d7458e7754 ("hw/ide/internal: Remove unused
> DMARestartFunc typedef") which removed the single use of RunState.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/hw/ide/internal.h | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 08/20] hw/ide: Include 'exec/ioport.h' instead of 'hw/isa/isa.h'

[Philippe Mathieu-Daudé]

The IDEBus structure has PortioList fields, so we need its
declarations from "exec/ioport.h". "hw/isa/isa.h" is not required.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/ide/internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index e6deb1c5dc..84d3722d67 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -9,9 +9,9 @@
 
 #include "hw/ide.h"
 #include "hw/irq.h"
-#include "hw/isa/isa.h"
 #include "sysemu/dma.h"
 #include "hw/block/block.h"
+#include "exec/ioport.h"
 
 /* debug IDE devices */
 #define USE_DMA_CDROM
-- 
2.38.1

Re: [PATCH 08/20] hw/ide: Include 'exec/ioport.h' instead of 'hw/isa/isa.h'

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> The IDEBus structure has PortioList fields, so we need its
> declarations from "exec/ioport.h". "hw/isa/isa.h" is not required.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/hw/ide/internal.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 09/20] hw/ide: Un-inline ide_set_irq()

[Philippe Mathieu-Daudé]

Only include "hw/irq.h" where appropriate.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/ahci.c             | 1 +
 hw/ide/core.c             | 8 ++++++++
 hw/ide/ich.c              | 1 +
 hw/ide/macio.c            | 1 +
 hw/ide/microdrive.c       | 1 +
 hw/ide/pci.c              | 1 +
 hw/misc/macio/gpio.c      | 1 +
 hw/sparc64/sun4u.c        | 1 +
 include/hw/ide/internal.h | 9 +--------
 9 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 7ce001cacd..3e21f607fe 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -22,6 +22,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/irq.h"
 #include "hw/pci/msi.h"
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5d1039378f..1473b6057f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -24,6 +24,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/irq.h"
 #include "hw/isa/isa.h"
 #include "migration/vmstate.h"
 #include "qemu/error-report.h"
@@ -2782,6 +2783,13 @@ void ide_init2(IDEBus *bus, qemu_irq irq)
     bus->dma = &ide_dma_nop;
 }
 
+void ide_set_irq(IDEBus *bus)
+{
+    if (!(bus->cmd & IDE_CTRL_DISABLE_IRQ)) {
+        qemu_irq_raise(bus->irq);
+    }
+}
+
 void ide_exit(IDEState *s)
 {
     timer_free(s->sector_write_timer);
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 1007a51fcb..d61faab532 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -61,6 +61,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/irq.h"
 #include "hw/pci/msi.h"
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index e604466acb..15fd934831 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -24,6 +24,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/irq.h"
 #include "hw/ppc/mac_dbdma.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index 56c5be3655..b9822b939b 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -29,6 +29,7 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "sysemu/dma.h"
+#include "hw/irq.h"
 
 #include "hw/ide/internal.h"
 #include "qom/object.h"
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 84ba733548..ae638dee0d 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -24,6 +24,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/irq.h"
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
 #include "sysemu/dma.h"
diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index c8ac5633b2..4deb330471 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -28,6 +28,7 @@
 #include "migration/vmstate.h"
 #include "hw/misc/macio/macio.h"
 #include "hw/misc/macio/gpio.h"
+#include "hw/irq.h"
 #include "hw/nmi.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 387181ff77..cb2d0fbbe7 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -28,6 +28,7 @@
 #include "qapi/error.h"
 #include "qemu/datadir.h"
 #include "cpu.h"
+#include "hw/irq.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_bus.h"
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 84d3722d67..57a6278327 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -8,7 +8,6 @@
  */
 
 #include "hw/ide.h"
-#include "hw/irq.h"
 #include "sysemu/dma.h"
 #include "hw/block/block.h"
 #include "exec/ioport.h"
@@ -572,13 +571,6 @@ static inline IDEState *idebus_active_if(IDEBus *bus)
     return bus->ifs + bus->unit;
 }
 
-static inline void ide_set_irq(IDEBus *bus)
-{
-    if (!(bus->cmd & IDE_CTRL_DISABLE_IRQ)) {
-        qemu_irq_raise(bus->irq);
-    }
-}
-
 /* hw/ide/core.c */
 extern const VMStateDescription vmstate_ide_bus;
 
@@ -627,6 +619,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_exit(IDEState *s);
 int ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
+void ide_set_irq(IDEBus *bus);
 void ide_register_restart_cb(IDEBus *bus);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
-- 
2.38.1

Re: [PATCH 09/20] hw/ide: Un-inline ide_set_irq()

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> Only include "hw/irq.h" where appropriate.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/ahci.c             | 1 +
>   hw/ide/core.c             | 8 ++++++++
>   hw/ide/ich.c              | 1 +
>   hw/ide/macio.c            | 1 +
>   hw/ide/microdrive.c       | 1 +
>   hw/ide/pci.c              | 1 +
>   hw/misc/macio/gpio.c      | 1 +
>   hw/sparc64/sun4u.c        | 1 +
>   include/hw/ide/internal.h | 9 +--------
>   9 files changed, 16 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 10/20] hw/ide: Rename ide_set_irq() -> ide_bus_set_irq()

[Philippe Mathieu-Daudé]

ide_set_irq() operates on a IDEBus; rename it as
ide_bus_set_irq() to emphasize its first argument
is a IDEBus.

Mechanical change using:

  $ sed -i -e 's/ide_set_irq/ide_bus_set_irq/g' \
        $(git grep -l ide_set_irq)

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/atapi.c            | 12 +++++------
 hw/ide/core.c             | 44 +++++++++++++++++++--------------------
 hw/ide/macio.c            |  2 +-
 include/hw/ide/internal.h |  2 +-
 4 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 0c36bd0afd..dcc39df9a4 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -179,7 +179,7 @@ void ide_atapi_cmd_ok(IDEState *s)
     s->status = READY_STAT | SEEK_STAT;
     s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
     ide_transfer_stop(s);
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc)
@@ -191,7 +191,7 @@ void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc)
     s->sense_key = sense_key;
     s->asc = asc;
     ide_transfer_stop(s);
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 void ide_atapi_io_error(IDEState *s, int ret)
@@ -254,7 +254,7 @@ void ide_atapi_cmd_reply_end(IDEState *s)
         } else {
             /* a new transfer is needed */
             s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO;
-            ide_set_irq(s->bus);
+            ide_bus_set_irq(s->bus);
             byte_count_limit = atapi_byte_count_limit(s);
             trace_ide_atapi_cmd_reply_end_bcl(s, byte_count_limit);
             size = s->packet_transfer_size;
@@ -294,7 +294,7 @@ void ide_atapi_cmd_reply_end(IDEState *s)
     /* end of transfer */
     trace_ide_atapi_cmd_reply_end_eot(s, s->status);
     ide_atapi_cmd_ok(s);
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 /* send a reply of 'size' bytes in s->io_buffer to an ATAPI command */
@@ -340,7 +340,7 @@ static void ide_atapi_cmd_check_status(IDEState *s)
     s->error = MC_ERR | (UNIT_ATTENTION << 4);
     s->status = ERR_STAT;
     s->nsector = 0;
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 /* ATAPI DMA support */
 
@@ -384,7 +384,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
     if (s->packet_transfer_size <= 0) {
         s->status = READY_STAT | SEEK_STAT;
         s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
         goto eot;
     }
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1473b6057f..117e26cef1 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -654,7 +654,7 @@ void ide_set_sector(IDEState *s, int64_t sector_num)
 
 static void ide_rw_error(IDEState *s) {
     ide_abort_command(s);
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 static void ide_buffered_readv_cb(void *opaque, int ret)
@@ -773,7 +773,7 @@ static void ide_sector_read_cb(void *opaque, int ret)
     s->nsector -= n;
     /* Allow the guest to read the io_buffer */
     ide_transfer_start(s, s->io_buffer, n * BDRV_SECTOR_SIZE, ide_sector_read);
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 static void ide_sector_read(IDEState *s)
@@ -837,7 +837,7 @@ void ide_dma_error(IDEState *s)
     dma_buf_commit(s, 0);
     ide_abort_command(s);
     ide_set_inactive(s, false);
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 int ide_handle_rw_error(IDEState *s, int error, int op)
@@ -907,7 +907,7 @@ static void ide_dma_cb(void *opaque, int ret)
     /* end of transfer ? */
     if (s->nsector == 0) {
         s->status = READY_STAT | SEEK_STAT;
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
         goto eot;
     }
 
@@ -1007,7 +1007,7 @@ static void ide_sector_write(IDEState *s);
 static void ide_sector_write_timer_cb(void *opaque)
 {
     IDEState *s = opaque;
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 static void ide_sector_write_cb(void *opaque, int ret)
@@ -1055,7 +1055,7 @@ static void ide_sector_write_cb(void *opaque, int ret)
         timer_mod(s->sector_write_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
                   (NANOSECONDS_PER_SECOND / 1000));
     } else {
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
     }
 }
 
@@ -1106,7 +1106,7 @@ static void ide_flush_cb(void *opaque, int ret)
     }
     s->status = READY_STAT | SEEK_STAT;
     ide_cmd_done(s);
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 static void ide_flush_cache(IDEState *s)
@@ -1195,7 +1195,7 @@ static void ide_cd_change_cb(void *opaque, bool load, Error **errp)
     s->cdrom_changed = 1;
     s->events.new_media = true;
     s->events.eject_request = false;
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 static void ide_cd_eject_request_cb(void *opaque, bool force)
@@ -1206,7 +1206,7 @@ static void ide_cd_eject_request_cb(void *opaque, bool force)
     if (force) {
         s->tray_locked = false;
     }
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 }
 
 static void ide_cmd_lba48_transform(IDEState *s, int lba48)
@@ -1440,7 +1440,7 @@ static bool cmd_identify(IDEState *s, uint8_t cmd)
         }
         s->status = READY_STAT | SEEK_STAT;
         ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
         return false;
     } else {
         if (s->drive_kind == IDE_CD) {
@@ -1630,7 +1630,7 @@ static bool cmd_specify(IDEState *s, uint8_t cmd)
     if (s->blk && s->drive_kind != IDE_CD) {
         s->heads = (s->select & (ATA_DEV_HS)) + 1;
         s->sectors = s->nsector;
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
     } else {
         ide_abort_command(s);
     }
@@ -1731,7 +1731,7 @@ static bool cmd_identify_packet(IDEState *s, uint8_t cmd)
     ide_atapi_identify(s);
     s->status = READY_STAT | SEEK_STAT;
     ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
     return false;
 }
 
@@ -1756,7 +1756,7 @@ static bool cmd_exec_dev_diagnostic(IDEState *s, uint8_t cmd)
          * They are part of the regular output (this is why ERR_STAT isn't set)
          * Device 0 passed, Device 1 passed or not present. */
         s->error = 0x01;
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
     }
 
     return false;
@@ -1788,7 +1788,7 @@ static bool cmd_cfa_req_ext_error_code(IDEState *s, uint8_t cmd)
 {
     s->error = 0x09;    /* miscellaneous error */
     s->status = READY_STAT | SEEK_STAT;
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 
     return false;
 }
@@ -1827,7 +1827,7 @@ static bool cmd_cfa_translate_sector(IDEState *s, uint8_t cmd)
     s->io_buffer[0x1a] = 0x01;                      /* Hot count */
 
     ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 
     return false;
 }
@@ -1851,7 +1851,7 @@ static bool cmd_cfa_access_metadata_storage(IDEState *s, uint8_t cmd)
 
     ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
     s->status = 0x00; /* NOTE: READY is _not_ set */
-    ide_set_irq(s->bus);
+    ide_bus_set_irq(s->bus);
 
     return false;
 }
@@ -1934,7 +1934,7 @@ static bool cmd_smart(IDEState *s, uint8_t cmd)
 
         s->status = READY_STAT | SEEK_STAT;
         ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
         return false;
 
     case SMART_READ_DATA:
@@ -1975,7 +1975,7 @@ static bool cmd_smart(IDEState *s, uint8_t cmd)
 
         s->status = READY_STAT | SEEK_STAT;
         ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
         return false;
 
     case SMART_READ_LOG:
@@ -2014,7 +2014,7 @@ static bool cmd_smart(IDEState *s, uint8_t cmd)
         }
         s->status = READY_STAT | SEEK_STAT;
         ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
         return false;
 
     case SMART_EXECUTE_OFFLINE:
@@ -2146,7 +2146,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 
     if (!ide_cmd_permitted(s, val)) {
         ide_abort_command(s);
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
         return;
     }
 
@@ -2164,7 +2164,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         }
 
         ide_cmd_done(s);
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
     }
 }
 
@@ -2783,7 +2783,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq)
     bus->dma = &ide_dma_nop;
 }
 
-void ide_set_irq(IDEBus *bus)
+void ide_bus_set_irq(IDEBus *bus)
 {
     if (!(bus->cmd & IDE_CTRL_DISABLE_IRQ)) {
         qemu_irq_raise(bus->irq);
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 15fd934831..24fb7a3f9d 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -160,7 +160,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
         MACIO_DPRINTF("End of IDE transfer\n");
         qemu_sglist_destroy(&s->sg);
         s->status = READY_STAT | SEEK_STAT;
-        ide_set_irq(s->bus);
+        ide_bus_set_irq(s->bus);
         m->dma_active = false;
         goto done;
     }
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 57a6278327..7b4b71d0b0 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -619,7 +619,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_exit(IDEState *s);
 int ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
-void ide_set_irq(IDEBus *bus);
+void ide_bus_set_irq(IDEBus *bus);
 void ide_register_restart_cb(IDEBus *bus);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
-- 
2.38.1

Re: [PATCH 10/20] hw/ide: Rename ide_set_irq() -> ide_bus_set_irq()

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> ide_set_irq() operates on a IDEBus; rename it as
> ide_bus_set_irq() to emphasize its first argument
> is a IDEBus.
> 
> Mechanical change using:
> 
>    $ sed -i -e 's/ide_set_irq/ide_bus_set_irq/g' \
>          $(git grep -l ide_set_irq)
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/atapi.c            | 12 +++++------
>   hw/ide/core.c             | 44 +++++++++++++++++++--------------------
>   hw/ide/macio.c            |  2 +-
>   include/hw/ide/internal.h |  2 +-
>   4 files changed, 30 insertions(+), 30 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 11/20] hw/ide: Rename ide_create_drive() -> ide_bus_create_drive()

[Philippe Mathieu-Daudé]

ide_create_drive() operates on a IDEBus; rename it as
ide_bus_create_drive() to emphasize its first argument
is a IDEBus.

Mechanical change using:

  $ sed -i -e 's/ide_create_drive/ide_bus_create_drive/g' \
        $(git grep -wl ide_create_drive)

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/sbsa-ref.c         | 2 +-
 hw/ide/ahci.c             | 2 +-
 hw/ide/isa.c              | 4 ++--
 hw/ide/macio.c            | 2 +-
 hw/ide/microdrive.c       | 2 +-
 hw/ide/mmio.c             | 4 ++--
 hw/ide/pci.c              | 2 +-
 hw/ide/qdev.c             | 2 +-
 include/hw/ide/internal.h | 2 +-
 9 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index f778cb6d09..0b93558dde 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -554,7 +554,7 @@ static void create_ahci(const SBSAMachineState *sms)
         if (hd[i] == NULL) {
             continue;
         }
-        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
+        ide_bus_create_drive(&ahci->dev[i].port, 0, hd[i]);
     }
 }
 
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 3e21f607fe..90fea5d059 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1842,7 +1842,7 @@ void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd)
         if (hd[i] == NULL) {
             continue;
         }
-        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
+        ide_bus_create_drive(&ahci->dev[i].port, 0, hd[i]);
     }
 
 }
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index ad47e0899e..74f7b43137 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -93,10 +93,10 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int irqnum,
 
     s = ISA_IDE(dev);
     if (hd0) {
-        ide_create_drive(&s->bus, 0, hd0);
+        ide_bus_create_drive(&s->bus, 0, hd0);
     }
     if (hd1) {
-        ide_create_drive(&s->bus, 1, hd1);
+        ide_bus_create_drive(&s->bus, 1, hd1);
     }
     return isadev;
 }
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 24fb7a3f9d..7efbbc720a 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -501,7 +501,7 @@ void macio_ide_init_drives(MACIOIDEState *s, DriveInfo **hd_table)
 
     for (i = 0; i < 2; i++) {
         if (hd_table[i]) {
-            ide_create_drive(&s->bus, i, hd_table[i]);
+            ide_bus_create_drive(&s->bus, i, hd_table[i]);
         }
     }
 }
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index b9822b939b..08504b499f 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -566,7 +566,7 @@ PCMCIACardState *dscm1xxxx_init(DriveInfo *dinfo)
     qdev_realize(DEVICE(md), NULL, &error_fatal);
 
     if (dinfo != NULL) {
-        ide_create_drive(&md->bus, 0, dinfo);
+        ide_bus_create_drive(&md->bus, 0, dinfo);
     }
     md->bus.ifs[0].drive_kind = IDE_CFATA;
     md->bus.ifs[0].mdata_size = METADATA_SIZE;
diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
index 5153d19ac6..1f1527122e 100644
--- a/hw/ide/mmio.c
+++ b/hw/ide/mmio.c
@@ -174,10 +174,10 @@ void mmio_ide_init_drives(DeviceState *dev, DriveInfo *hd0, DriveInfo *hd1)
     MMIOIDEState *s = MMIO_IDE(dev);
 
     if (hd0 != NULL) {
-        ide_create_drive(&s->bus, 0, hd0);
+        ide_bus_create_drive(&s->bus, 0, hd0);
     }
     if (hd1 != NULL) {
-        ide_create_drive(&s->bus, 1, hd1);
+        ide_bus_create_drive(&s->bus, 1, hd1);
     }
 }
 
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index ae638dee0d..4223f5e64d 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -489,7 +489,7 @@ void pci_ide_create_devs(PCIDevice *dev)
     ide_drive_get(hd_table, ARRAY_SIZE(hd_table));
     for (i = 0; i < 4; i++) {
         if (hd_table[i]) {
-            ide_create_drive(d->bus + bus[i], unit[i], hd_table[i]);
+            ide_bus_create_drive(d->bus + bus[i], unit[i], hd_table[i]);
         }
     }
 }
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6f6c7462f3..1b3b4da01d 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -124,7 +124,7 @@ static void ide_qdev_realize(DeviceState *qdev, Error **errp)
     dc->realize(dev, errp);
 }
 
-IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
+IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
 {
     DeviceState *dev;
 
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 7b4b71d0b0..ccfe064643 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -645,7 +645,7 @@ void ide_atapi_cmd_reply_end(IDEState *s);
 /* hw/ide/qdev.c */
 void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
                   int bus_id, int max_units);
-IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
+IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
 
 int ide_handle_rw_error(IDEState *s, int error, int op);
 
-- 
2.38.1

Re: [PATCH 11/20] hw/ide: Rename ide_create_drive() -> ide_bus_create_drive()

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> ide_create_drive() operates on a IDEBus; rename it as
> ide_bus_create_drive() to emphasize its first argument
> is a IDEBus.
> 
> Mechanical change using:
> 
>    $ sed -i -e 's/ide_create_drive/ide_bus_create_drive/g' \
>          $(git grep -wl ide_create_drive)
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/sbsa-ref.c         | 2 +-
>   hw/ide/ahci.c             | 2 +-
>   hw/ide/isa.c              | 4 ++--
>   hw/ide/macio.c            | 2 +-
>   hw/ide/microdrive.c       | 2 +-
>   hw/ide/mmio.c             | 4 ++--
>   hw/ide/pci.c              | 2 +-
>   hw/ide/qdev.c             | 2 +-
>   include/hw/ide/internal.h | 2 +-
>   9 files changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 12/20] hw/ide: Rename ide_register_restart_cb -> ide_bus_register_restart_cb

[Philippe Mathieu-Daudé]

ide_register_restart_cb() operates on a IDEBus; rename it as
ide_bus_register_restart_cb() to emphasize its first argument
is a IDEBus.

Mechanical change using:

  $ sed -i -e 's/ide_register_restart_cb/ide_bus_register_restart_cb/g' \
    $(git grep -l ide_register_restart_cb)

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/ahci.c             | 2 +-
 hw/ide/cmd646.c           | 2 +-
 hw/ide/core.c             | 2 +-
 hw/ide/isa.c              | 2 +-
 hw/ide/piix.c             | 2 +-
 hw/ide/sii3112.c          | 2 +-
 hw/ide/via.c              | 2 +-
 include/hw/ide/internal.h | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 90fea5d059..430961d73b 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1560,7 +1560,7 @@ void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
         ad->port_no = i;
         ad->port.dma = &ad->dma;
         ad->port.dma->ops = &ahci_dma_ops;
-        ide_register_restart_cb(&ad->port);
+        ide_bus_register_restart_cb(&ad->port);
     }
     g_free(irqs);
 }
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 94c576262c..2865bc25fc 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -298,7 +298,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp)
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
         d->bmdma[i].bus = &d->bus[i];
-        ide_register_restart_cb(&d->bus[i]);
+        ide_bus_register_restart_cb(&d->bus[i]);
     }
 }
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 117e26cef1..5897411b95 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2759,7 +2759,7 @@ static void ide_restart_cb(void *opaque, bool running, RunState state)
     }
 }
 
-void ide_register_restart_cb(IDEBus *bus)
+void ide_bus_register_restart_cb(IDEBus *bus)
 {
     if (bus->dma->ops->restart_dma) {
         bus->vmstate = qemu_add_vm_change_state_handler(ide_restart_cb, bus);
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 74f7b43137..f8ed26b587 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -74,7 +74,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
     ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
     ide_init2(&s->bus, isa_get_irq(isadev, s->irqnum));
     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
-    ide_register_restart_cb(&s->bus);
+    ide_bus_register_restart_cb(&s->bus);
 }
 
 ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int irqnum,
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 267dbf37db..daeb9b605d 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -149,7 +149,7 @@ static int pci_piix_init_ports(PCIIDEState *d)
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
         d->bmdma[i].bus = &d->bus[i];
-        ide_register_restart_cb(&d->bus[i]);
+        ide_bus_register_restart_cb(&d->bus[i]);
     }
 
     return 0;
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 46204f10d7..c918370220 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -288,7 +288,7 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
 
         bmdma_init(&s->bus[i], &s->bmdma[i], s);
         s->bmdma[i].bus = &s->bus[i];
-        ide_register_restart_cb(&s->bus[i]);
+        ide_bus_register_restart_cb(&s->bus[i]);
     }
 }
 
diff --git a/hw/ide/via.c b/hw/ide/via.c
index e1a429405d..fd398226d4 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -195,7 +195,7 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
         d->bmdma[i].bus = &d->bus[i];
-        ide_register_restart_cb(&d->bus[i]);
+        ide_bus_register_restart_cb(&d->bus[i]);
     }
 }
 
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index ccfe064643..cc95cd47a0 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -620,7 +620,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_exit(IDEState *s);
 int ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
 void ide_bus_set_irq(IDEBus *bus);
-void ide_register_restart_cb(IDEBus *bus);
+void ide_bus_register_restart_cb(IDEBus *bus);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
 
-- 
2.38.1

Re: [PATCH 12/20] hw/ide: Rename ide_register_restart_cb -> ide_bus_register_restart_cb

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> ide_register_restart_cb() operates on a IDEBus; rename it as
> ide_bus_register_restart_cb() to emphasize its first argument
> is a IDEBus.
> 
> Mechanical change using:
> 
>    $ sed -i -e 's/ide_register_restart_cb/ide_bus_register_restart_cb/g' \
>      $(git grep -l ide_register_restart_cb)
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/ahci.c             | 2 +-
>   hw/ide/cmd646.c           | 2 +-
>   hw/ide/core.c             | 2 +-
>   hw/ide/isa.c              | 2 +-
>   hw/ide/piix.c             | 2 +-
>   hw/ide/sii3112.c          | 2 +-
>   hw/ide/via.c              | 2 +-
>   include/hw/ide/internal.h | 2 +-
>   8 files changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 13/20] hw/ide: Rename ide_exec_cmd() -> ide_bus_exec_cmd()

[Philippe Mathieu-Daudé]

ide_exec_cmd() operates on a IDEBus; rename it as
ide_bus_exec_cmd() to emphasize its first argument
is a IDEBus.

Mechanical change using:

  $ sed -i -e 's/ide_exec_cmd/ide_bus_exec_cmd/g' \
        $(git grep -wl ide_exec_cmd)

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/ahci.c             | 2 +-
 hw/ide/core.c             | 6 +++---
 hw/ide/trace-events       | 2 +-
 include/hw/ide/internal.h | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 430961d73b..7f67fb3119 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1269,7 +1269,7 @@ static void handle_reg_h2d_fis(AHCIState *s, int port,
     cmd->status = 0;
 
     /* We're ready to process the command in FIS byte 2. */
-    ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
+    ide_bus_exec_cmd(&s->dev[port].port, cmd_fis[2]);
 }
 
 static int handle_cmd(AHCIState *s, int port, uint8_t slot)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5897411b95..1be0731d1a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1327,7 +1327,7 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case ATA_IOPORT_WR_COMMAND:
         ide_clear_hob(bus);
         qemu_irq_lower(bus->irq);
-        ide_exec_cmd(bus, val);
+        ide_bus_exec_cmd(bus, val);
         break;
     }
 }
@@ -2123,13 +2123,13 @@ static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
         && (ide_cmd_table[cmd].flags & (1u << s->drive_kind));
 }
 
-void ide_exec_cmd(IDEBus *bus, uint32_t val)
+void ide_bus_exec_cmd(IDEBus *bus, uint32_t val)
 {
     IDEState *s;
     bool complete;
 
     s = idebus_active_if(bus);
-    trace_ide_exec_cmd(bus, s, val);
+    trace_ide_bus_exec_cmd(bus, s, val);
 
     /* ignore commands to non existent slave */
     if (s != bus->ifs && !s->blk) {
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index 15d7921f15..a394c05710 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -12,7 +12,7 @@ ide_data_writew(uint32_t addr, uint32_t val, void *bus, void *s)
 ide_data_readl(uint32_t addr, uint32_t val, void *bus, void *s)                    "IDE PIO rd @ 0x%"PRIx32" (Data: Long); val 0x%08"PRIx32"; bus %p; IDEState %p"
 ide_data_writel(uint32_t addr, uint32_t val, void *bus, void *s)                   "IDE PIO wr @ 0x%"PRIx32" (Data: Long); val 0x%08"PRIx32"; bus %p; IDEState %p"
 # misc
-ide_exec_cmd(void *bus, void *state, uint32_t cmd) "IDE exec cmd: bus %p; state %p; cmd 0x%02x"
+ide_bus_exec_cmd(void *bus, void *state, uint32_t cmd) "IDE exec cmd: bus %p; state %p; cmd 0x%02x"
 ide_cancel_dma_sync_buffered(void *fn, void *req) "invoking cb %p of buffered request %p with -ECANCELED"
 ide_cancel_dma_sync_remaining(void) "draining all remaining requests"
 ide_sector_read(int64_t sector_num, int nsectors) "sector=%"PRId64" nsectors=%d"
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index cc95cd47a0..11a4931ef9 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -622,7 +622,7 @@ int ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
 void ide_bus_set_irq(IDEBus *bus);
 void ide_bus_register_restart_cb(IDEBus *bus);
 
-void ide_exec_cmd(IDEBus *bus, uint32_t val);
+void ide_bus_exec_cmd(IDEBus *bus, uint32_t val);
 
 void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
                         EndTransferFunc *end_transfer_func);
-- 
2.38.1

Re: [PATCH 13/20] hw/ide: Rename ide_exec_cmd() -> ide_bus_exec_cmd()

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> ide_exec_cmd() operates on a IDEBus; rename it as
> ide_bus_exec_cmd() to emphasize its first argument
> is a IDEBus.
> 
> Mechanical change using:
> 
>    $ sed -i -e 's/ide_exec_cmd/ide_bus_exec_cmd/g' \
>          $(git grep -wl ide_exec_cmd)
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/ahci.c             | 2 +-
>   hw/ide/core.c             | 6 +++---
>   hw/ide/trace-events       | 2 +-
>   include/hw/ide/internal.h | 2 +-
>   4 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 14/20] hw/ide: Rename ide_init2() -> ide_bus_init_output_irq()

[Philippe Mathieu-Daudé]

ide_init2() initializes a IDEBus, and set its output IRQ.
To emphasize this, rename it as ide_bus_init_output_irq().

Mechanical change using:

  $ sed -i -e 's/ide_init2/ide_bus_init_output_irq/g' \
        $(git grep -l ide_init2)

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/ahci.c             | 2 +-
 hw/ide/cmd646.c           | 2 +-
 hw/ide/core.c             | 4 ++--
 hw/ide/isa.c              | 2 +-
 hw/ide/macio.c            | 2 +-
 hw/ide/microdrive.c       | 2 +-
 hw/ide/mmio.c             | 2 +-
 hw/ide/piix.c             | 3 ++-
 hw/ide/sii3112.c          | 2 +-
 hw/ide/via.c              | 2 +-
 include/hw/ide/internal.h | 4 ++--
 11 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 7f67fb3119..d79b70d8c5 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1554,7 +1554,7 @@ void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
         AHCIDevice *ad = &s->dev[i];
 
         ide_bus_init(&ad->port, sizeof(ad->port), qdev, i, 1);
-        ide_init2(&ad->port, irqs[i]);
+        ide_bus_init_output_irq(&ad->port, irqs[i]);
 
         ad->hba = s;
         ad->port_no = i;
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 2865bc25fc..26a90ed45f 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -294,7 +294,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp)
     qdev_init_gpio_in(ds, cmd646_set_irq, 2);
     for (i = 0; i < 2; i++) {
         ide_bus_init(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
-        ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
+        ide_bus_init_output_irq(&d->bus[i], qdev_get_gpio_in(ds, i));
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
         d->bmdma[i].bus = &d->bus[i];
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1be0731d1a..fd2215c506 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2771,7 +2771,7 @@ static IDEDMA ide_dma_nop = {
     .aiocb = NULL,
 };
 
-void ide_init2(IDEBus *bus, qemu_irq irq)
+void ide_bus_init_output_irq(IDEBus *bus, qemu_irq irq_out)
 {
     int i;
 
@@ -2779,7 +2779,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq)
         ide_init1(bus, i);
         ide_reset(&bus->ifs[i]);
     }
-    bus->irq = irq;
+    bus->irq = irq_out;
     bus->dma = &ide_dma_nop;
 }
 
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index f8ed26b587..95053e026f 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -72,7 +72,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
 
     ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2);
     ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
-    ide_init2(&s->bus, isa_get_irq(isadev, s->irqnum));
+    ide_bus_init_output_irq(&s->bus, isa_get_irq(isadev, s->irqnum));
     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
     ide_bus_register_restart_cb(&s->bus);
 }
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 7efbbc720a..6be29e44bc 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -420,7 +420,7 @@ static void macio_ide_realizefn(DeviceState *dev, Error **errp)
 {
     MACIOIDEState *s = MACIO_IDE(dev);
 
-    ide_init2(&s->bus, s->ide_irq);
+    ide_bus_init_output_irq(&s->bus, s->ide_irq);
 
     /* Register DMA callbacks */
     s->dma.ops = &dbdma_ops;
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index 08504b499f..84452ae4ef 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -599,7 +599,7 @@ static void microdrive_realize(DeviceState *dev, Error **errp)
 {
     MicroDriveState *md = MICRODRIVE(dev);
 
-    ide_init2(&md->bus, qemu_allocate_irq(md_set_irq, md, 0));
+    ide_bus_init_output_irq(&md->bus, qemu_allocate_irq(md_set_irq, md, 0));
 }
 
 static void microdrive_init(Object *obj)
diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
index 1f1527122e..b6c40bc028 100644
--- a/hw/ide/mmio.c
+++ b/hw/ide/mmio.c
@@ -122,7 +122,7 @@ static void mmio_ide_realizefn(DeviceState *dev, Error **errp)
     SysBusDevice *d = SYS_BUS_DEVICE(dev);
     MMIOIDEState *s = MMIO_IDE(dev);
 
-    ide_init2(&s->bus, s->irq);
+    ide_bus_init_output_irq(&s->bus, s->irq);
 
     memory_region_init_io(&s->iomem1, OBJECT(s), &mmio_ide_ops, s,
                           "ide-mmio.1", 16 << s->shift);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index daeb9b605d..2f71376b93 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -145,7 +145,8 @@ static int pci_piix_init_ports(PCIIDEState *d)
         if (ret) {
             return ret;
         }
-        ide_init2(&d->bus[i], isa_get_irq(NULL, port_info[i].isairq));
+        ide_bus_init_output_irq(&d->bus[i],
+                                isa_get_irq(NULL, port_info[i].isairq));
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
         d->bmdma[i].bus = &d->bus[i];
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index c918370220..f9becdff8e 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -284,7 +284,7 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
     qdev_init_gpio_in(ds, sii3112_set_irq, 2);
     for (i = 0; i < 2; i++) {
         ide_bus_init(&s->bus[i], sizeof(s->bus[i]), ds, i, 1);
-        ide_init2(&s->bus[i], qdev_get_gpio_in(ds, i));
+        ide_bus_init_output_irq(&s->bus[i], qdev_get_gpio_in(ds, i));
 
         bmdma_init(&s->bus[i], &s->bmdma[i], s);
         s->bmdma[i].bus = &s->bus[i];
diff --git a/hw/ide/via.c b/hw/ide/via.c
index fd398226d4..ab9e43e244 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -191,7 +191,7 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
     qdev_init_gpio_in(ds, via_ide_set_irq, 2);
     for (i = 0; i < 2; i++) {
         ide_bus_init(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
-        ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
+        ide_bus_init_output_irq(&d->bus[i], qdev_get_gpio_in(ds, i));
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
         d->bmdma[i].bus = &d->bus[i];
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 11a4931ef9..c687282a78 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -488,7 +488,7 @@ struct IDEBus {
     IDEDMA *dma;
     uint8_t unit;
     uint8_t cmd;
-    qemu_irq irq;
+    qemu_irq irq; /* bus output */
 
     int error_status;
     uint8_t retry_unit;
@@ -616,8 +616,8 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
                    uint64_t wwn,
                    uint32_t cylinders, uint32_t heads, uint32_t secs,
                    int chs_trans, Error **errp);
-void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_exit(IDEState *s);
+void ide_bus_init_output_irq(IDEBus *bus, qemu_irq irq_out);
 int ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
 void ide_bus_set_irq(IDEBus *bus);
 void ide_bus_register_restart_cb(IDEBus *bus);
-- 
2.38.1

Re: [PATCH 14/20] hw/ide: Rename ide_init2() -> ide_bus_init_output_irq()

[Philippe Mathieu-Daudé]

On 15/2/23 12:27, Philippe Mathieu-Daudé wrote:
> ide_init2() initializes a IDEBus, and set its output IRQ.

s/set/sets/

> To emphasize this, rename it as ide_bus_init_output_irq().
> 
> Mechanical change using:
> 
>    $ sed -i -e 's/ide_init2/ide_bus_init_output_irq/g' \
>          $(git grep -l ide_init2)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/ide/ahci.c             | 2 +-
>   hw/ide/cmd646.c           | 2 +-
>   hw/ide/core.c             | 4 ++--
>   hw/ide/isa.c              | 2 +-
>   hw/ide/macio.c            | 2 +-
>   hw/ide/microdrive.c       | 2 +-
>   hw/ide/mmio.c             | 2 +-
>   hw/ide/piix.c             | 3 ++-
>   hw/ide/sii3112.c          | 2 +-
>   hw/ide/via.c              | 2 +-
>   include/hw/ide/internal.h | 4 ++--
>   11 files changed, 14 insertions(+), 13 deletions(-)

Re: [PATCH 14/20] hw/ide: Rename ide_init2() -> ide_bus_init_output_irq()

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> ide_init2() initializes a IDEBus, and set its output IRQ.
> To emphasize this, rename it as ide_bus_init_output_irq().
> 
> Mechanical change using:
> 
>    $ sed -i -e 's/ide_init2/ide_bus_init_output_irq/g' \
>          $(git grep -l ide_init2)
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/ahci.c             | 2 +-
>   hw/ide/cmd646.c           | 2 +-
>   hw/ide/core.c             | 4 ++--
>   hw/ide/isa.c              | 2 +-
>   hw/ide/macio.c            | 2 +-
>   hw/ide/microdrive.c       | 2 +-
>   hw/ide/mmio.c             | 2 +-
>   hw/ide/piix.c             | 3 ++-
>   hw/ide/sii3112.c          | 2 +-
>   hw/ide/via.c              | 2 +-
>   include/hw/ide/internal.h | 4 ++--
>   11 files changed, 14 insertions(+), 13 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 14/20] hw/ide: Rename ide_init2() -> ide_bus_init_output_irq()

[Bernhard Beschow]

Am 15. Februar 2023 11:27:06 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>ide_init2() initializes a IDEBus, and set its output IRQ.

s/a IDEBus/an IDEBus/, see e.g. https://owl.purdue.edu/owl/general_writing/grammar/articles_a_versus_an.html .

>To emphasize this, rename it as ide_bus_init_output_irq().

The new name suggests that only the IRQ is initialized, while in fact the two IDE devices contained inside the bus are too. It's really hard to come up with a good name here because the code is so tangled.

>
>Mechanical change using:
>
>  $ sed -i -e 's/ide_init2/ide_bus_init_output_irq/g' \
>        $(git grep -l ide_init2)
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/ide/ahci.c             | 2 +-
> hw/ide/cmd646.c           | 2 +-
> hw/ide/core.c             | 4 ++--
> hw/ide/isa.c              | 2 +-
> hw/ide/macio.c            | 2 +-
> hw/ide/microdrive.c       | 2 +-
> hw/ide/mmio.c             | 2 +-
> hw/ide/piix.c             | 3 ++-
> hw/ide/sii3112.c          | 2 +-
> hw/ide/via.c              | 2 +-
> include/hw/ide/internal.h | 4 ++--
> 11 files changed, 14 insertions(+), 13 deletions(-)
>
>diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>index 7f67fb3119..d79b70d8c5 100644
>--- a/hw/ide/ahci.c
>+++ b/hw/ide/ahci.c
>@@ -1554,7 +1554,7 @@ void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
>         AHCIDevice *ad = &s->dev[i];
> 
>         ide_bus_init(&ad->port, sizeof(ad->port), qdev, i, 1);
>-        ide_init2(&ad->port, irqs[i]);
>+        ide_bus_init_output_irq(&ad->port, irqs[i]);
> 
>         ad->hba = s;
>         ad->port_no = i;
>diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
>index 2865bc25fc..26a90ed45f 100644
>--- a/hw/ide/cmd646.c
>+++ b/hw/ide/cmd646.c
>@@ -294,7 +294,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp)
>     qdev_init_gpio_in(ds, cmd646_set_irq, 2);
>     for (i = 0; i < 2; i++) {
>         ide_bus_init(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
>-        ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
>+        ide_bus_init_output_irq(&d->bus[i], qdev_get_gpio_in(ds, i));
> 
>         bmdma_init(&d->bus[i], &d->bmdma[i], d);
>         d->bmdma[i].bus = &d->bus[i];
>diff --git a/hw/ide/core.c b/hw/ide/core.c
>index 1be0731d1a..fd2215c506 100644
>--- a/hw/ide/core.c
>+++ b/hw/ide/core.c
>@@ -2771,7 +2771,7 @@ static IDEDMA ide_dma_nop = {
>     .aiocb = NULL,
> };
> 
>-void ide_init2(IDEBus *bus, qemu_irq irq)
>+void ide_bus_init_output_irq(IDEBus *bus, qemu_irq irq_out)
> {
>     int i;
> 
>@@ -2779,7 +2779,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq)
>         ide_init1(bus, i);
>         ide_reset(&bus->ifs[i]);
>     }
>-    bus->irq = irq;
>+    bus->irq = irq_out;
>     bus->dma = &ide_dma_nop;
> }
> 
>diff --git a/hw/ide/isa.c b/hw/ide/isa.c
>index f8ed26b587..95053e026f 100644
>--- a/hw/ide/isa.c
>+++ b/hw/ide/isa.c
>@@ -72,7 +72,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
> 
>     ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2);
>     ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
>-    ide_init2(&s->bus, isa_get_irq(isadev, s->irqnum));
>+    ide_bus_init_output_irq(&s->bus, isa_get_irq(isadev, s->irqnum));
>     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
>     ide_bus_register_restart_cb(&s->bus);
> }
>diff --git a/hw/ide/macio.c b/hw/ide/macio.c
>index 7efbbc720a..6be29e44bc 100644
>--- a/hw/ide/macio.c
>+++ b/hw/ide/macio.c
>@@ -420,7 +420,7 @@ static void macio_ide_realizefn(DeviceState *dev, Error **errp)
> {
>     MACIOIDEState *s = MACIO_IDE(dev);
> 
>-    ide_init2(&s->bus, s->ide_irq);
>+    ide_bus_init_output_irq(&s->bus, s->ide_irq);
> 
>     /* Register DMA callbacks */
>     s->dma.ops = &dbdma_ops;
>diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
>index 08504b499f..84452ae4ef 100644
>--- a/hw/ide/microdrive.c
>+++ b/hw/ide/microdrive.c
>@@ -599,7 +599,7 @@ static void microdrive_realize(DeviceState *dev, Error **errp)
> {
>     MicroDriveState *md = MICRODRIVE(dev);
> 
>-    ide_init2(&md->bus, qemu_allocate_irq(md_set_irq, md, 0));
>+    ide_bus_init_output_irq(&md->bus, qemu_allocate_irq(md_set_irq, md, 0));
> }
> 
> static void microdrive_init(Object *obj)
>diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
>index 1f1527122e..b6c40bc028 100644
>--- a/hw/ide/mmio.c
>+++ b/hw/ide/mmio.c
>@@ -122,7 +122,7 @@ static void mmio_ide_realizefn(DeviceState *dev, Error **errp)
>     SysBusDevice *d = SYS_BUS_DEVICE(dev);
>     MMIOIDEState *s = MMIO_IDE(dev);
> 
>-    ide_init2(&s->bus, s->irq);
>+    ide_bus_init_output_irq(&s->bus, s->irq);
> 
>     memory_region_init_io(&s->iomem1, OBJECT(s), &mmio_ide_ops, s,
>                           "ide-mmio.1", 16 << s->shift);
>diff --git a/hw/ide/piix.c b/hw/ide/piix.c
>index daeb9b605d..2f71376b93 100644
>--- a/hw/ide/piix.c
>+++ b/hw/ide/piix.c
>@@ -145,7 +145,8 @@ static int pci_piix_init_ports(PCIIDEState *d)
>         if (ret) {
>             return ret;
>         }
>-        ide_init2(&d->bus[i], isa_get_irq(NULL, port_info[i].isairq));
>+        ide_bus_init_output_irq(&d->bus[i],
>+                                isa_get_irq(NULL, port_info[i].isairq));
> 
>         bmdma_init(&d->bus[i], &d->bmdma[i], d);
>         d->bmdma[i].bus = &d->bus[i];
>diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
>index c918370220..f9becdff8e 100644
>--- a/hw/ide/sii3112.c
>+++ b/hw/ide/sii3112.c
>@@ -284,7 +284,7 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
>     qdev_init_gpio_in(ds, sii3112_set_irq, 2);
>     for (i = 0; i < 2; i++) {
>         ide_bus_init(&s->bus[i], sizeof(s->bus[i]), ds, i, 1);
>-        ide_init2(&s->bus[i], qdev_get_gpio_in(ds, i));
>+        ide_bus_init_output_irq(&s->bus[i], qdev_get_gpio_in(ds, i));
> 
>         bmdma_init(&s->bus[i], &s->bmdma[i], s);
>         s->bmdma[i].bus = &s->bus[i];
>diff --git a/hw/ide/via.c b/hw/ide/via.c
>index fd398226d4..ab9e43e244 100644
>--- a/hw/ide/via.c
>+++ b/hw/ide/via.c
>@@ -191,7 +191,7 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>     qdev_init_gpio_in(ds, via_ide_set_irq, 2);
>     for (i = 0; i < 2; i++) {
>         ide_bus_init(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
>-        ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
>+        ide_bus_init_output_irq(&d->bus[i], qdev_get_gpio_in(ds, i));
> 
>         bmdma_init(&d->bus[i], &d->bmdma[i], d);
>         d->bmdma[i].bus = &d->bus[i];
>diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
>index 11a4931ef9..c687282a78 100644
>--- a/include/hw/ide/internal.h
>+++ b/include/hw/ide/internal.h
>@@ -488,7 +488,7 @@ struct IDEBus {
>     IDEDMA *dma;
>     uint8_t unit;
>     uint8_t cmd;
>-    qemu_irq irq;
>+    qemu_irq irq; /* bus output */
> 
>     int error_status;
>     uint8_t retry_unit;
>@@ -616,8 +616,8 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
>                    uint64_t wwn,
>                    uint32_t cylinders, uint32_t heads, uint32_t secs,
>                    int chs_trans, Error **errp);
>-void ide_init2(IDEBus *bus, qemu_irq irq);
> void ide_exit(IDEState *s);
>+void ide_bus_init_output_irq(IDEBus *bus, qemu_irq irq_out);
> int ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
> void ide_bus_set_irq(IDEBus *bus);
> void ide_bus_register_restart_cb(IDEBus *bus);

[PATCH 15/20] hw/ide: Rename idebus_active_if() -> ide_bus_active_if()

[Philippe Mathieu-Daudé]

idebus_active_if() operates on a IDEBus; rename it as
ide_bus_active_if() to emphasize its first argument
is a IDEBus.

Mechanical change using:

  $ sed -i -e 's/idebus_active_if/ide_bus_active_if/g' \
        $(git grep -l idebus_active_if)

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/core.c             | 18 +++++++++---------
 hw/ide/macio.c            |  8 ++++----
 hw/ide/microdrive.c       |  4 ++--
 hw/ide/pci.c              |  2 +-
 include/hw/ide/internal.h |  2 +-
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index fd2215c506..2d034731cf 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1265,7 +1265,7 @@ const char *ATA_IOPORT_WR_lookup[ATA_IOPORT_WR_NUM_REGISTERS] = {
 void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
     IDEBus *bus = opaque;
-    IDEState *s = idebus_active_if(bus);
+    IDEState *s = ide_bus_active_if(bus);
     int reg_num = addr & 7;
 
     trace_ide_ioport_write(addr, ATA_IOPORT_WR_lookup[reg_num], val, bus, s);
@@ -2128,7 +2128,7 @@ void ide_bus_exec_cmd(IDEBus *bus, uint32_t val)
     IDEState *s;
     bool complete;
 
-    s = idebus_active_if(bus);
+    s = ide_bus_active_if(bus);
     trace_ide_bus_exec_cmd(bus, s, val);
 
     /* ignore commands to non existent slave */
@@ -2195,7 +2195,7 @@ const char *ATA_IOPORT_RR_lookup[ATA_IOPORT_RR_NUM_REGISTERS] = {
 uint32_t ide_ioport_read(void *opaque, uint32_t addr)
 {
     IDEBus *bus = opaque;
-    IDEState *s = idebus_active_if(bus);
+    IDEState *s = ide_bus_active_if(bus);
     uint32_t reg_num;
     int ret, hob;
 
@@ -2281,7 +2281,7 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr)
 uint32_t ide_status_read(void *opaque, uint32_t addr)
 {
     IDEBus *bus = opaque;
-    IDEState *s = idebus_active_if(bus);
+    IDEState *s = ide_bus_active_if(bus);
     int ret;
 
     if ((!bus->ifs[0].blk && !bus->ifs[1].blk) ||
@@ -2370,7 +2370,7 @@ static bool ide_is_pio_out(IDEState *s)
 void ide_data_writew(void *opaque, uint32_t addr, uint32_t val)
 {
     IDEBus *bus = opaque;
-    IDEState *s = idebus_active_if(bus);
+    IDEState *s = ide_bus_active_if(bus);
     uint8_t *p;
 
     trace_ide_data_writew(addr, val, bus, s);
@@ -2406,7 +2406,7 @@ void ide_data_writew(void *opaque, uint32_t addr, uint32_t val)
 uint32_t ide_data_readw(void *opaque, uint32_t addr)
 {
     IDEBus *bus = opaque;
-    IDEState *s = idebus_active_if(bus);
+    IDEState *s = ide_bus_active_if(bus);
     uint8_t *p;
     int ret;
 
@@ -2444,7 +2444,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr)
 void ide_data_writel(void *opaque, uint32_t addr, uint32_t val)
 {
     IDEBus *bus = opaque;
-    IDEState *s = idebus_active_if(bus);
+    IDEState *s = ide_bus_active_if(bus);
     uint8_t *p;
 
     trace_ide_data_writel(addr, val, bus, s);
@@ -2472,7 +2472,7 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val)
 uint32_t ide_data_readl(void *opaque, uint32_t addr)
 {
     IDEBus *bus = opaque;
-    IDEState *s = idebus_active_if(bus);
+    IDEState *s = ide_bus_active_if(bus);
     uint8_t *p;
     int ret;
 
@@ -2711,7 +2711,7 @@ static void ide_restart_bh(void *opaque)
         return;
     }
 
-    s = idebus_active_if(bus);
+    s = ide_bus_active_if(bus);
     is_read = (bus->error_status & IDE_RETRY_READ) != 0;
 
     /* The error status must be cleared before resubmitting the request: The
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 6be29e44bc..dca1cc9efc 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -60,7 +60,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
 {
     DBDMA_io *io = opaque;
     MACIOIDEState *m = io->opaque;
-    IDEState *s = idebus_active_if(&m->bus);
+    IDEState *s = ide_bus_active_if(&m->bus);
     int64_t offset;
 
     MACIO_DPRINTF("pmac_ide_atapi_transfer_cb\n");
@@ -136,7 +136,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
 {
     DBDMA_io *io = opaque;
     MACIOIDEState *m = io->opaque;
-    IDEState *s = idebus_active_if(&m->bus);
+    IDEState *s = ide_bus_active_if(&m->bus);
     int64_t offset;
 
     MACIO_DPRINTF("pmac_ide_transfer_cb\n");
@@ -220,7 +220,7 @@ done:
 static void pmac_ide_transfer(DBDMA_io *io)
 {
     MACIOIDEState *m = io->opaque;
-    IDEState *s = idebus_active_if(&m->bus);
+    IDEState *s = ide_bus_active_if(&m->bus);
 
     MACIO_DPRINTF("\n");
 
@@ -251,7 +251,7 @@ static void pmac_ide_transfer(DBDMA_io *io)
 static void pmac_ide_flush(DBDMA_io *io)
 {
     MACIOIDEState *m = io->opaque;
-    IDEState *s = idebus_active_if(&m->bus);
+    IDEState *s = ide_bus_active_if(&m->bus);
 
     if (s->bus->dma->aiocb) {
         blk_drain(s->blk);
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index 84452ae4ef..f1017f7333 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -250,14 +250,14 @@ static uint16_t md_common_read(PCMCIACardState *card, uint32_t at)
     case 0xd:	/* Error */
         return ide_ioport_read(&s->bus, 0x1);
     case 0xe:	/* Alternate Status */
-        ifs = idebus_active_if(&s->bus);
+        ifs = ide_bus_active_if(&s->bus);
         if (ifs->blk) {
             return ifs->status;
         } else {
             return 0;
         }
     case 0xf:	/* Device Address */
-        ifs = idebus_active_if(&s->bus);
+        ifs = ide_bus_active_if(&s->bus);
         return 0xc2 | ((~ifs->select << 2) & 0x3c);
     default:
         return ide_ioport_read(&s->bus, at);
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 4223f5e64d..2ddcb49b27 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -296,7 +296,7 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val)
     /* Ignore writes to SSBM if it keeps the old value */
     if ((val & BM_CMD_START) != (bm->cmd & BM_CMD_START)) {
         if (!(val & BM_CMD_START)) {
-            ide_cancel_dma_sync(idebus_active_if(bm->bus));
+            ide_cancel_dma_sync(ide_bus_active_if(bm->bus));
             bm->status &= ~BM_STATUS_DMAING;
         } else {
             bm->cur_addr = bm->addr;
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index c687282a78..c2b794150f 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -566,7 +566,7 @@ static inline uint8_t ide_dma_cmd_to_retry(uint8_t dma_cmd)
     return 0;
 }
 
-static inline IDEState *idebus_active_if(IDEBus *bus)
+static inline IDEState *ide_bus_active_if(IDEBus *bus)
 {
     return bus->ifs + bus->unit;
 }
-- 
2.38.1

Re: [PATCH 15/20] hw/ide: Rename idebus_active_if() -> ide_bus_active_if()

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> idebus_active_if() operates on a IDEBus; rename it as
> ide_bus_active_if() to emphasize its first argument
> is a IDEBus.
> 
> Mechanical change using:
> 
>    $ sed -i -e 's/idebus_active_if/ide_bus_active_if/g' \
>          $(git grep -l idebus_active_if)
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/core.c             | 18 +++++++++---------
>   hw/ide/macio.c            |  8 ++++----
>   hw/ide/microdrive.c       |  4 ++--
>   hw/ide/pci.c              |  2 +-
>   include/hw/ide/internal.h |  2 +-
>   5 files changed, 17 insertions(+), 17 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 16/20] hw/ide/ioport: Remove unnecessary includes

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/ioport.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/hw/ide/ioport.c b/hw/ide/ioport.c
index e6caa537fa..e2ecc6230c 100644
--- a/hw/ide/ioport.c
+++ b/hw/ide/ioport.c
@@ -25,16 +25,6 @@
 
 #include "qemu/osdep.h"
 #include "hw/isa/isa.h"
-#include "qemu/error-report.h"
-#include "qemu/timer.h"
-#include "sysemu/blockdev.h"
-#include "sysemu/dma.h"
-#include "hw/block/block.h"
-#include "sysemu/block-backend.h"
-#include "qapi/error.h"
-#include "qemu/cutils.h"
-#include "sysemu/replay.h"
-
 #include "hw/ide/internal.h"
 #include "trace.h"
 
-- 
2.38.1

Re: [PATCH 16/20] hw/ide/ioport: Remove unnecessary includes

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/ioport.c | 10 ----------
>   1 file changed, 10 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 17/20] hw/ide/pci: Unexport bmdma_active_if()

[Philippe Mathieu-Daudé]

From: Bernhard Beschow <shentey@gmail.com>

The function is only used inside ide/pci.c, so doesn't need to be exported.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/pci.c         | 6 ++++++
 include/hw/ide/pci.h | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 2ddcb49b27..fc9224bbc9 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -104,6 +104,12 @@ const MemoryRegionOps pci_ide_data_le_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static IDEState *bmdma_active_if(BMDMAState *bmdma)
+{
+    assert(bmdma->bus->retry_unit != (uint8_t)-1);
+    return bmdma->bus->ifs + bmdma->bus->retry_unit;
+}
+
 static void bmdma_start_dma(const IDEDMA *dma, IDEState *s,
                             BlockCompletionFunc *dma_cb)
 {
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 2a6284acac..7b5e3f6e1c 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -55,12 +55,6 @@ struct PCIIDEState {
     MemoryRegion data_bar[2];
 };
 
-static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
-{
-    assert(bmdma->bus->retry_unit != (uint8_t)-1);
-    return bmdma->bus->ifs + bmdma->bus->retry_unit;
-}
-
 void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
 void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
 extern MemoryRegionOps bmdma_addr_ioport_ops;
-- 
2.38.1

Re: [PATCH 17/20] hw/ide/pci: Unexport bmdma_active_if()

[Bernhard Beschow]

Am 15. Februar 2023 11:27:09 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>From: Bernhard Beschow <shentey@gmail.com>
>
>The function is only used inside ide/pci.c, so doesn't need to be exported.
>
>Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/ide/pci.c         | 6 ++++++
> include/hw/ide/pci.h | 6 ------
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/hw/ide/pci.c b/hw/ide/pci.c
>index 2ddcb49b27..fc9224bbc9 100644
>--- a/hw/ide/pci.c
>+++ b/hw/ide/pci.c
>@@ -104,6 +104,12 @@ const MemoryRegionOps pci_ide_data_le_ops = {
>     .endianness = DEVICE_LITTLE_ENDIAN,
> };
> 
>+static IDEState *bmdma_active_if(BMDMAState *bmdma)
>+{
>+    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>+    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>+}
>+
> static void bmdma_start_dma(const IDEDMA *dma, IDEState *s,
>                             BlockCompletionFunc *dma_cb)
> {
>diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
>index 2a6284acac..7b5e3f6e1c 100644
>--- a/include/hw/ide/pci.h
>+++ b/include/hw/ide/pci.h
>@@ -55,12 +55,6 @@ struct PCIIDEState {
>     MemoryRegion data_bar[2];
> };
> 
>-static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
>-{
>-    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>-    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>-}
>-
> void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
> void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
> extern MemoryRegionOps bmdma_addr_ioport_ops;

Cool, where did you find this? ;)

This patch, your other patches doing s/ide/ide_bus/, and the fact that IDEBus instantiates two IDE devices itself make me wonder whether the IDE devices should really be instantiated in the usual QOM way. Then perhaps it could turn out that all the s/ide/ide_bus/ patches aren't really needed since the functions could operate on the IDE device directly. Not sure though...

This might all be a rabbit hole, but since you already started looking into it... ;)

Best regards,
Bernhard

Re: [PATCH 17/20] hw/ide/pci: Unexport bmdma_active_if()

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2848 bytes --]

On Wed, 15 Feb 2023, Bernhard Beschow wrote:
> Am 15. Februar 2023 11:27:09 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> From: Bernhard Beschow <shentey@gmail.com>
>>
>> The function is only used inside ide/pci.c, so doesn't need to be exported.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/ide/pci.c         | 6 ++++++
>> include/hw/ide/pci.h | 6 ------
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/ide/pci.c b/hw/ide/pci.c
>> index 2ddcb49b27..fc9224bbc9 100644
>> --- a/hw/ide/pci.c
>> +++ b/hw/ide/pci.c
>> @@ -104,6 +104,12 @@ const MemoryRegionOps pci_ide_data_le_ops = {
>>     .endianness = DEVICE_LITTLE_ENDIAN,
>> };
>>
>> +static IDEState *bmdma_active_if(BMDMAState *bmdma)
>> +{
>> +    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>> +    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>> +}
>> +
>> static void bmdma_start_dma(const IDEDMA *dma, IDEState *s,
>>                             BlockCompletionFunc *dma_cb)
>> {
>> diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
>> index 2a6284acac..7b5e3f6e1c 100644
>> --- a/include/hw/ide/pci.h
>> +++ b/include/hw/ide/pci.h
>> @@ -55,12 +55,6 @@ struct PCIIDEState {
>>     MemoryRegion data_bar[2];
>> };
>>
>> -static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
>> -{
>> -    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>> -    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>> -}
>> -
>> void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
>> void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
>> extern MemoryRegionOps bmdma_addr_ioport_ops;
>
> Cool, where did you find this? ;)
>
> This patch, your other patches doing s/ide/ide_bus/, and the fact that 
> IDEBus instantiates two IDE devices itself make me wonder whether the 
> IDE devices should really be instantiated in the usual QOM way. Then 
> perhaps it could turn out that all the s/ide/ide_bus/ patches aren't 
> really needed since the functions could operate on the IDE device 
> directly. Not sure though...
>
> This might all be a rabbit hole, but since you already started looking 
> into it... ;)

If you want some more, we've also found another problem with ports that 
should not have anythnig connected but still appear to have a non-working 
device that causes some delay during guest boot (and I think this is the 
reason the pegasos2 firmware prints an "ATA device not present or not 
responding" error while detecting IDE devices. This was discussed here:

https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg02468.html

Just to spread knowledge about it. I don't exoect this to be fixed as it 
does not cause much trouble just if you wanted to dig into it in the 
future I thought I'd let you know.

Regards,
BALATON Zoltan

Re: [PATCH 17/20] hw/ide/pci: Unexport bmdma_active_if()

[Bernhard Beschow]

Am 15. Februar 2023 21:09:10 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Wed, 15 Feb 2023, Bernhard Beschow wrote:
>> Am 15. Februar 2023 11:27:09 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> From: Bernhard Beschow <shentey@gmail.com>
>>> 
>>> The function is only used inside ide/pci.c, so doesn't need to be exported.
>>> 
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> hw/ide/pci.c         | 6 ++++++
>>> include/hw/ide/pci.h | 6 ------
>>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/hw/ide/pci.c b/hw/ide/pci.c
>>> index 2ddcb49b27..fc9224bbc9 100644
>>> --- a/hw/ide/pci.c
>>> +++ b/hw/ide/pci.c
>>> @@ -104,6 +104,12 @@ const MemoryRegionOps pci_ide_data_le_ops = {
>>>     .endianness = DEVICE_LITTLE_ENDIAN,
>>> };
>>> 
>>> +static IDEState *bmdma_active_if(BMDMAState *bmdma)
>>> +{
>>> +    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>>> +    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>>> +}
>>> +
>>> static void bmdma_start_dma(const IDEDMA *dma, IDEState *s,
>>>                             BlockCompletionFunc *dma_cb)
>>> {
>>> diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
>>> index 2a6284acac..7b5e3f6e1c 100644
>>> --- a/include/hw/ide/pci.h
>>> +++ b/include/hw/ide/pci.h
>>> @@ -55,12 +55,6 @@ struct PCIIDEState {
>>>     MemoryRegion data_bar[2];
>>> };
>>> 
>>> -static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
>>> -{
>>> -    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>>> -    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>>> -}
>>> -
>>> void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
>>> void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
>>> extern MemoryRegionOps bmdma_addr_ioport_ops;
>> 
>> Cool, where did you find this? ;)
>> 
>> This patch, your other patches doing s/ide/ide_bus/, and the fact that IDEBus instantiates two IDE devices itself make me wonder whether the IDE devices should really be instantiated in the usual QOM way. Then perhaps it could turn out that all the s/ide/ide_bus/ patches aren't really needed since the functions could operate on the IDE device directly. Not sure though...
>> 
>> This might all be a rabbit hole, but since you already started looking into it... ;)
>
>If you want some more, we've also found another problem with ports that should not have anythnig connected but still appear to have a non-working device that causes some delay during guest boot (and I think this is the reason the pegasos2 firmware prints an "ATA device not present or not responding" error while detecting IDE devices.

Yes, that would make sense and is indeed the "phantom" behavior I would be expecting.

>This was discussed here:
>
>https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg02468.html
>
>Just to spread knowledge about it. I don't exoect this to be fixed as it does not cause much trouble just if you wanted to dig into it in the future I thought I'd let you know.

Interesting read... Thanks for sharing!

Best regards,
Bernhard
>
>Regards,
>BALATON Zoltan

Re: [PATCH 17/20] hw/ide/pci: Unexport bmdma_active_if()

[Bernhard Beschow]

Am 16. Februar 2023 00:18:47 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 15. Februar 2023 21:09:10 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>>On Wed, 15 Feb 2023, Bernhard Beschow wrote:
>>> Am 15. Februar 2023 11:27:09 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> From: Bernhard Beschow <shentey@gmail.com>
>>>> 
>>>> The function is only used inside ide/pci.c, so doesn't need to be exported.
>>>> 
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> hw/ide/pci.c         | 6 ++++++
>>>> include/hw/ide/pci.h | 6 ------
>>>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>>> 
>>>> diff --git a/hw/ide/pci.c b/hw/ide/pci.c
>>>> index 2ddcb49b27..fc9224bbc9 100644
>>>> --- a/hw/ide/pci.c
>>>> +++ b/hw/ide/pci.c
>>>> @@ -104,6 +104,12 @@ const MemoryRegionOps pci_ide_data_le_ops = {
>>>>     .endianness = DEVICE_LITTLE_ENDIAN,
>>>> };
>>>> 
>>>> +static IDEState *bmdma_active_if(BMDMAState *bmdma)
>>>> +{
>>>> +    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>>>> +    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>>>> +}
>>>> +
>>>> static void bmdma_start_dma(const IDEDMA *dma, IDEState *s,
>>>>                             BlockCompletionFunc *dma_cb)
>>>> {
>>>> diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
>>>> index 2a6284acac..7b5e3f6e1c 100644
>>>> --- a/include/hw/ide/pci.h
>>>> +++ b/include/hw/ide/pci.h
>>>> @@ -55,12 +55,6 @@ struct PCIIDEState {
>>>>     MemoryRegion data_bar[2];
>>>> };
>>>> 
>>>> -static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
>>>> -{
>>>> -    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>>>> -    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>>>> -}
>>>> -
>>>> void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
>>>> void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
>>>> extern MemoryRegionOps bmdma_addr_ioport_ops;
>>> 
>>> Cool, where did you find this? ;)
>>> 
>>> This patch, your other patches doing s/ide/ide_bus/, and the fact that IDEBus instantiates two IDE devices itself make me wonder whether the IDE devices should really be instantiated in the usual QOM way. Then perhaps it could turn out that all the s/ide/ide_bus/ patches aren't really needed since the functions could operate on the IDE device directly. Not sure though...
>>> 
>>> This might all be a rabbit hole, but since you already started looking into it... ;)
>>
>>If you want some more, we've also found another problem with ports that should not have anythnig connected but still appear to have a non-working device that causes some delay during guest boot (and I think this is the reason the pegasos2 firmware prints an "ATA device not present or not responding" error while detecting IDE devices.
>
>Yes, that would make sense and is indeed the "phantom" behavior I would be expecting.

Just an idea: Perhaps pci_ide_create_devs() rather than ide_init2() could create the IDEState objects -- of course only if the respective drives are configured.

>
>>This was discussed here:
>>
>>https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg02468.html
>>
>>Just to spread knowledge about it. I don't exoect this to be fixed as it does not cause much trouble just if you wanted to dig into it in the future I thought I'd let you know.
>
>Interesting read... Thanks for sharing!
>
>Best regards,
>Bernhard
>>
>>Regards,
>>BALATON Zoltan

Re: [PATCH 17/20] hw/ide/pci: Unexport bmdma_active_if()

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> From: Bernhard Beschow<shentey@gmail.com>
> 
> The function is only used inside ide/pci.c, so doesn't need to be exported.
> 
> Signed-off-by: Bernhard Beschow<shentey@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/pci.c         | 6 ++++++
>   include/hw/ide/pci.h | 6 ------
>   2 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 18/20] hw/ide/piix: Remove unused includes

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/piix.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 2f71376b93..6354ae740b 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -28,14 +28,9 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/pci/pci.h"
 #include "migration/vmstate.h"
 #include "qapi/error.h"
-#include "qemu/module.h"
-#include "sysemu/block-backend.h"
-#include "sysemu/blockdev.h"
-#include "sysemu/dma.h"
-
+#include "hw/pci/pci.h"
 #include "hw/ide/piix.h"
 #include "hw/ide/pci.h"
 #include "trace.h"
-- 
2.38.1

Re: [PATCH 18/20] hw/ide/piix: Remove unused includes

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ide/piix.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 19/20] hw/ide/piix: Pass Error* to pci_piix_init_ports() for better error msg

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/piix.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 6354ae740b..f10bdf39ff 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -121,7 +121,7 @@ static void piix_ide_reset(DeviceState *dev)
     pci_set_byte(pci_conf + 0x20, 0x01);  /* BMIBA: 20-23h */
 }
 
-static int pci_piix_init_ports(PCIIDEState *d)
+static bool pci_piix_init_ports(PCIIDEState *d, Error **errp)
 {
     static const struct {
         int iobase;
@@ -138,7 +138,9 @@ static int pci_piix_init_ports(PCIIDEState *d)
         ret = ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
                               port_info[i].iobase2);
         if (ret) {
-            return ret;
+            error_setg_errno(errp, -ret, "Failed to realize %s port %u",
+                             object_get_typename(OBJECT(d)), i);
+            return false;
         }
         ide_bus_init_output_irq(&d->bus[i],
                                 isa_get_irq(NULL, port_info[i].isairq));
@@ -148,14 +150,13 @@ static int pci_piix_init_ports(PCIIDEState *d)
         ide_bus_register_restart_cb(&d->bus[i]);
     }
 
-    return 0;
+    return true;
 }
 
 static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
 {
     PCIIDEState *d = PCI_IDE(dev);
     uint8_t *pci_conf = dev->config;
-    int rc;
 
     pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
 
@@ -164,10 +165,8 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
 
     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
 
-    rc = pci_piix_init_ports(d);
-    if (rc) {
-        error_setg_errno(errp, -rc, "Failed to realize %s",
-                         object_get_typename(OBJECT(dev)));
+    if (!pci_piix_init_ports(d, errp)) {
+        return;
     }
 }
 
-- 
2.38.1

Re: [PATCH 19/20] hw/ide/piix: Pass Error* to pci_piix_init_ports() for better error msg

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> -    rc = pci_piix_init_ports(d);
> -    if (rc) {
> -        error_setg_errno(errp, -rc, "Failed to realize %s",
> -                         object_get_typename(OBJECT(dev)));
> +    if (!pci_piix_init_ports(d, errp)) {
> +        return;
>       }

The if is unnecessary.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 20/20] hw/ide/piix: Refactor pci_piix_init_ports as pci_piix_init_bus per bus

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/piix.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index f10bdf39ff..41d60921e3 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -121,7 +121,7 @@ static void piix_ide_reset(DeviceState *dev)
     pci_set_byte(pci_conf + 0x20, 0x01);  /* BMIBA: 20-23h */
 }
 
-static bool pci_piix_init_ports(PCIIDEState *d, Error **errp)
+static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, Error **errp)
 {
     static const struct {
         int iobase;
@@ -131,24 +131,21 @@ static bool pci_piix_init_ports(PCIIDEState *d, Error **errp)
         {0x1f0, 0x3f6, 14},
         {0x170, 0x376, 15},
     };
-    int i, ret;
+    int ret;
 
-    for (i = 0; i < 2; i++) {
-        ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
-        ret = ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
-                              port_info[i].iobase2);
-        if (ret) {
-            error_setg_errno(errp, -ret, "Failed to realize %s port %u",
-                             object_get_typename(OBJECT(d)), i);
-            return false;
-        }
-        ide_bus_init_output_irq(&d->bus[i],
-                                isa_get_irq(NULL, port_info[i].isairq));
-
-        bmdma_init(&d->bus[i], &d->bmdma[i], d);
-        d->bmdma[i].bus = &d->bus[i];
-        ide_bus_register_restart_cb(&d->bus[i]);
+    ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
+    ret = ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
+                          port_info[i].iobase2);
+    if (ret) {
+        error_setg_errno(errp, -ret, "Failed to realize %s port %u",
+                         object_get_typename(OBJECT(d)), i);
+        return false;
     }
+    ide_bus_init_output_irq(&d->bus[i], isa_get_irq(NULL, port_info[i].isairq));
+
+    bmdma_init(&d->bus[i], &d->bmdma[i], d);
+    d->bmdma[i].bus = &d->bus[i];
+    ide_bus_register_restart_cb(&d->bus[i]);
 
     return true;
 }
@@ -165,8 +162,10 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
 
     vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
 
-    if (!pci_piix_init_ports(d, errp)) {
-        return;
+    for (unsigned i = 0; i < 2; i++) {
+        if (!pci_piix_init_bus(d, i, errp)) {
+            return;
+        }
     }
 }
 
-- 
2.38.1

Re: [PATCH 20/20] hw/ide/piix: Refactor pci_piix_init_ports as pci_piix_init_bus per bus

[Richard Henderson]

On 2/15/23 01:27, Philippe Mathieu-Daudé wrote:
> @@ -165,8 +162,10 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
>   
>       vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
>   
> -    if (!pci_piix_init_ports(d, errp)) {
> -        return;
> +    for (unsigned i = 0; i < 2; i++) {
> +        if (!pci_piix_init_bus(d, i, errp)) {
> +            return;
> +        }

Oh, I see.  You can disregard my comment vs patch 19.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~