diff mbox series

[4/8] hw/ide: Move MAX_IDE_BUS define to one header

Message ID eb14ddee41be26f38ee0fe5ff0ff42c79afe9731.1584134074.git.balaton@eik.bme.hu
State New
Headers show
Series Misc hw/ide legacy clean up | expand

Commit Message

BALATON Zoltan March 13, 2020, 9:14 p.m. UTC
There are several definitions of MAX_IDE_BUS in different boards (some
of them unused) with the same value. Move it to include/hw/ide/internal.h
to have it in a central place.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/alpha/dp264.c          | 2 --
 hw/hppa/machine.c         | 2 --
 hw/i386/pc_piix.c         | 2 --
 hw/mips/mips_fulong2e.c   | 1 -
 hw/mips/mips_malta.c      | 4 +---
 hw/mips/mips_r4k.c        | 4 +---
 hw/ppc/mac_newworld.c     | 1 -
 hw/ppc/mac_oldworld.c     | 1 -
 hw/ppc/prep.c             | 2 --
 hw/sparc64/sun4u.c        | 1 -
 include/hw/ide/internal.h | 2 ++
 11 files changed, 4 insertions(+), 18 deletions(-)

Comments

Markus Armbruster March 16, 2020, 6:53 a.m. UTC | #1
BALATON Zoltan <balaton@eik.bme.hu> writes:

> There are several definitions of MAX_IDE_BUS in different boards (some
> of them unused) with the same value. Move it to include/hw/ide/internal.h
> to have it in a central place.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

This one feels a bit questionable.

The number of (PATA) IDE buses provides by a host bus adapter depends on
the HBA.  It happens to be 2 for all HBAs we implement, but it could
really be anything.

Similar for SATA, where the common number is 6, but could really be
anything.  I can't see offhand whether any HBA we implement provides a
different number.

By moving MAX_IDE_BUS to include/hw/ide/internal.h, you bake the
accidental commonality into the interface to the IDE core.  I'd prefer
not to.
Philippe Mathieu-Daudé March 16, 2020, 8:23 a.m. UTC | #2
On 3/16/20 7:53 AM, Markus Armbruster wrote:
> BALATON Zoltan <balaton@eik.bme.hu> writes:
> 
>> There are several definitions of MAX_IDE_BUS in different boards (some
>> of them unused) with the same value. Move it to include/hw/ide/internal.h
>> to have it in a central place.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> 
> This one feels a bit questionable.
> 
> The number of (PATA) IDE buses provides by a host bus adapter depends on
> the HBA.  It happens to be 2 for all HBAs we implement, but it could
> really be anything.
> 
> Similar for SATA, where the common number is 6, but could really be
> anything.  I can't see offhand whether any HBA we implement provides a
> different number.
> 
> By moving MAX_IDE_BUS to include/hw/ide/internal.h, you bake the
> accidental commonality into the interface to the IDE core.  I'd prefer
> not to.

I agree with Markus here (I kept this commit tagged because I was 
thinking the same but didn't know how to express it correctly. Thanks 
Markus!).
diff mbox series

Patch

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 27595767e5..0f58b1b668 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -24,8 +24,6 @@ 
 #include "qemu/cutils.h"
 #include "net/net.h"
 
-#define MAX_IDE_BUS 2
-
 static uint64_t cpu_alpha_superpage_to_phys(void *opaque, uint64_t addr)
 {
     if (((addr >> 41) & 3) == 2) {
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 9175f4b790..1f9a390f99 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -24,8 +24,6 @@ 
 #include "qemu/log.h"
 #include "net/net.h"
 
-#define MAX_IDE_BUS 2
-
 static ISABus *hppa_isa_bus(void)
 {
     ISABus *isa_bus;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c399398739..b363a69e2e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -63,8 +63,6 @@ 
 #include "sysemu/numa.h"
 #include "hw/mem/nvdimm.h"
 
-#define MAX_IDE_BUS 2
-
 #ifdef CONFIG_IDE_ISA
 static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
 static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 639ba2a091..3690b76061 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -55,7 +55,6 @@ 
 
 /* fulong 2e has a 512k flash: Winbond W39L040AP70Z */
 #define BIOS_SIZE               (512 * KiB)
-#define MAX_IDE_BUS             2
 
 /*
  * PMON is not part of qemu and released with BSD license, anyone
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index d380f73d7b..6f51e33e7b 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -40,7 +40,7 @@ 
 #include "sysemu/arch_init.h"
 #include "qemu/log.h"
 #include "hw/mips/bios.h"
-#include "hw/ide.h"
+#include "hw/ide/internal.h"
 #include "hw/irq.h"
 #include "hw/loader.h"
 #include "elf.h"
@@ -68,8 +68,6 @@ 
 
 #define FLASH_SIZE          0x400000
 
-#define MAX_IDE_BUS         2
-
 typedef struct {
     MemoryRegion iomem;
     MemoryRegion iomem_lo; /* 0 - 0x900 */
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index ad8b75e286..2e5372bda0 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -25,7 +25,7 @@ 
 #include "hw/block/flash.h"
 #include "qemu/log.h"
 #include "hw/mips/bios.h"
-#include "hw/ide.h"
+#include "hw/ide/internal.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "hw/rtc/mc146818rtc.h"
@@ -37,8 +37,6 @@ 
 #include "sysemu/runstate.h"
 #include "qemu/error-report.h"
 
-#define MAX_IDE_BUS 2
-
 static const int ide_iobase[2] = { 0x1f0, 0x170 };
 static const int ide_iobase2[2] = { 0x3f6, 0x376 };
 static const int ide_irq[2] = { 14, 15 };
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index b8189bf7a4..daa1523feb 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -75,7 +75,6 @@ 
 #include "hw/sysbus.h"
 #include "trace.h"
 
-#define MAX_IDE_BUS 2
 #define CFG_ADDR 0xf0000510
 #define TBFREQ (100UL * 1000UL * 1000UL)
 #define CLOCKFREQ (900UL * 1000UL * 1000UL)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 440c406eb4..2478748c78 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -51,7 +51,6 @@ 
 #include "kvm_ppc.h"
 #include "exec/address-spaces.h"
 
-#define MAX_IDE_BUS 2
 #define CFG_ADDR 0xf0000510
 #define TBFREQ 16600000UL
 #define CLOCKFREQ 266000000UL
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 111cc80867..e1b1549e58 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -56,8 +56,6 @@ 
 /* SMP is not enabled, for now */
 #define MAX_CPUS 1
 
-#define MAX_IDE_BUS 2
-
 #define CFG_ADDR 0xf0000510
 
 #define KERNEL_LOAD_ADDR 0x01000000
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d33e84f831..74acfd39b3 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -66,7 +66,6 @@ 
 #define PBM_PCI_IO_BASE      (PBM_SPECIAL_BASE + 0x02000000ULL)
 #define PROM_FILENAME        "openbios-sparc64"
 #define NVRAM_SIZE           0x2000
-#define MAX_IDE_BUS          2
 #define BIOS_CFG_IOPORT      0x510
 #define FW_CFG_SPARC64_WIDTH (FW_CFG_ARCH_LOCAL + 0x00)
 #define FW_CFG_SPARC64_HEIGHT (FW_CFG_ARCH_LOCAL + 0x01)
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 1bc1fc73e5..1a49d35959 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -27,6 +27,8 @@  typedef struct IDEDMAOps IDEDMAOps;
 #define TYPE_IDE_BUS "IDE"
 #define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
 
+#define MAX_IDE_BUS 2
+
 /* Bits of HD_STATUS */
 #define ERR_STAT		0x01
 #define INDEX_STAT		0x02