diff mbox series

[26/32] hw/pci-host/piix: Move RCR_IOPORT register definition

Message ID 20191015162705.28087-27-philmd@redhat.com (mailing list archive)
State Superseded
Headers show
Series hw/i386/pc: Split PIIX3 southbridge from i440FX northbridge | expand

Commit Message

Philippe Mathieu-Daudé Oct. 15, 2019, 4:26 p.m. UTC 
From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The RCR_IOPORT register belongs to the PIIX chipset.
Move the definition to "piix.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/pci-host/piix.c            | 1 +
 include/hw/i386/pc.h          | 6 ------
 include/hw/southbridge/piix.h | 6 ++++++
 3 files changed, 7 insertions(+), 6 deletions(-)

Comments

Aleksandar Markovic Oct. 18, 2019, 9:19 a.m. UTC | #1 
On Tuesday, October 15, 2019, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> The RCR_IOPORT register belongs to the PIIX chipset.
> Move the definition to "piix.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/pci-host/piix.c            | 1 +
>  include/hw/i386/pc.h          | 6 ------
>  include/hw/southbridge/piix.h | 6 ++++++
>  3 files changed, 7 insertions(+), 6 deletions(-)
>
>
Does it make sense to add prefix PIIX_ or a similar one to the register
name?

In any case:

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>


> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 3292703de7..3770575c1a 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -27,6 +27,7 @@
>  #include "hw/irq.h"
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pci_host.h"
> +#include "hw/southbridge/piix.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/isa/isa.h"
>  #include "hw/sysbus.h"
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 183326d9fe..1c20b96571 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -257,12 +257,6 @@ typedef struct PCII440FXState PCII440FXState;
>
>  #define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "igd-passthrough-i440FX"
>
> -/*
> - * Reset Control Register: PCI-accessible ISA-Compatible Register at
> address
> - * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0,
> 8086:7000).
> - */
> -#define RCR_IOPORT 0xcf9
> -
>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                      PCII440FXState **pi440fx_state, int *piix_devfn,
>                      ISABus **isa_bus, qemu_irq *pic,
> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
> index add352456b..79ebe0089b 100644
> --- a/include/hw/southbridge/piix.h
> +++ b/include/hw/southbridge/piix.h
> @@ -18,6 +18,12 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t
> smb_io_base,
>                        qemu_irq sci_irq, qemu_irq smi_irq,
>                        int smm_enabled, DeviceState **piix4_pm);
>
> +/*
> + * Reset Control Register: PCI-accessible ISA-Compatible Register at
> address
> + * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0,
> 8086:7000).
> + */
> +#define RCR_IOPORT 0xcf9
> +
>  extern PCIDevice *piix4_dev;
>
>  DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus,
> --
> 2.21.0
>
>
>
Philippe Mathieu-Daudé Oct. 18, 2019, 10:13 a.m. UTC | #2 
On 10/18/19 11:19 AM, Aleksandar Markovic wrote:
> On Tuesday, October 15, 2019, Philippe Mathieu-Daudé <philmd@redhat.com 
> <mailto:philmd@redhat.com>> wrote:
> 
>     From: Philippe Mathieu-Daudé <f4bug@amsat.org <mailto:f4bug@amsat.org>>
> 
>     The RCR_IOPORT register belongs to the PIIX chipset.
>     Move the definition to "piix.h".
> 
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>     <mailto:philmd@redhat.com>>
>     ---
>       hw/pci-host/piix.c            | 1 +
>       include/hw/i386/pc.h          | 6 ------
>       include/hw/southbridge/piix.h | 6 ++++++
>       3 files changed, 7 insertions(+), 6 deletions(-)
> 
> 
> Does it make sense to add prefix PIIX_ or a similar one to the register 
> name?

Good idea, it will make the comment in hw/i386/acpi-build.c:213 cleaner:

     /* The above need not be conditional on machine type because the 
reset port
      * happens to be the same on PIIX (pc) and ICH9 (q35). */
     QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != RCR_IOPORT);

> 
> In any case:
> 
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com 
> <mailto:amarkovic@wavecomp.com>>

Thanks!

> 
>     diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>     index 3292703de7..3770575c1a 100644
>     --- a/hw/pci-host/piix.c
>     +++ b/hw/pci-host/piix.c
>     @@ -27,6 +27,7 @@
>       #include "hw/irq.h"
>       #include "hw/pci/pci.h"
>       #include "hw/pci/pci_host.h"
>     +#include "hw/southbridge/piix.h"
>       #include "hw/qdev-properties.h"
>       #include "hw/isa/isa.h"
>       #include "hw/sysbus.h"
>     diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>     index 183326d9fe..1c20b96571 100644
>     --- a/include/hw/i386/pc.h
>     +++ b/include/hw/i386/pc.h
>     @@ -257,12 +257,6 @@ typedef struct PCII440FXState PCII440FXState;
> 
>       #define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
>     "igd-passthrough-i440FX"
> 
>     -/*
>     - * Reset Control Register: PCI-accessible ISA-Compatible Register
>     at address
>     - * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0,
>     8086:7000).
>     - */
>     -#define RCR_IOPORT 0xcf9
>     -
>       PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                           PCII440FXState **pi440fx_state, int *piix_devfn,
>                           ISABus **isa_bus, qemu_irq *pic,
>     diff --git a/include/hw/southbridge/piix.h
>     b/include/hw/southbridge/piix.h
>     index add352456b..79ebe0089b 100644
>     --- a/include/hw/southbridge/piix.h
>     +++ b/include/hw/southbridge/piix.h
>     @@ -18,6 +18,12 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn,
>     uint32_t smb_io_base,
>                             qemu_irq sci_irq, qemu_irq smi_irq,
>                             int smm_enabled, DeviceState **piix4_pm);
> 
>     +/*
>     + * Reset Control Register: PCI-accessible ISA-Compatible Register
>     at address
>     + * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0,
>     8086:7000).
>     + */
>     +#define RCR_IOPORT 0xcf9
>     +
>       extern PCIDevice *piix4_dev;
> 
>       DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus,
>     -- 
>     2.21.0
> 
>
Aleksandar Markovic Oct. 18, 2019, 1:07 p.m. UTC | #3 
On Friday, October 18, 2019, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> On 10/18/19 11:19 AM, Aleksandar Markovic wrote:
>
>> On Tuesday, October 15, 2019, Philippe Mathieu-Daudé <philmd@redhat.com
>> <mailto:philmd@redhat.com>> wrote:
>>
>>     From: Philippe Mathieu-Daudé <f4bug@amsat.org <mailto:f4bug@amsat.org
>> >>
>>
>>     The RCR_IOPORT register belongs to the PIIX chipset.
>>     Move the definition to "piix.h".
>>
>>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>>     <mailto:philmd@redhat.com>>
>>     ---
>>       hw/pci-host/piix.c            | 1 +
>>       include/hw/i386/pc.h          | 6 ------
>>       include/hw/southbridge/piix.h | 6 ++++++
>>       3 files changed, 7 insertions(+), 6 deletions(-)
>>
>>
>> Does it make sense to add prefix PIIX_ or a similar one to the register
>> name?
>>
>
> Good idea, it will make the comment in hw/i386/acpi-build.c:213 cleaner:


Correct. Let's than add PIIX_ prefix.

Thanks in advance.

A.



>     /* The above need not be conditional on machine type because the reset
> port
>      * happens to be the same on PIIX (pc) and ICH9 (q35). */
>     QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != RCR_IOPORT);
>
>
>> In any case:
>>
>> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com <mailto:
>> amarkovic@wavecomp.com>>
>>
>
> Thanks!
>
>
>>     diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>     index 3292703de7..3770575c1a 100644
>>     --- a/hw/pci-host/piix.c
>>     +++ b/hw/pci-host/piix.c
>>     @@ -27,6 +27,7 @@
>>       #include "hw/irq.h"
>>       #include "hw/pci/pci.h"
>>       #include "hw/pci/pci_host.h"
>>     +#include "hw/southbridge/piix.h"
>>       #include "hw/qdev-properties.h"
>>       #include "hw/isa/isa.h"
>>       #include "hw/sysbus.h"
>>     diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>>     index 183326d9fe..1c20b96571 100644
>>     --- a/include/hw/i386/pc.h
>>     +++ b/include/hw/i386/pc.h
>>     @@ -257,12 +257,6 @@ typedef struct PCII440FXState PCII440FXState;
>>
>>       #define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
>>     "igd-passthrough-i440FX"
>>
>>     -/*
>>     - * Reset Control Register: PCI-accessible ISA-Compatible Register
>>     at address
>>     - * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0,
>>     8086:7000).
>>     - */
>>     -#define RCR_IOPORT 0xcf9
>>     -
>>       PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>                           PCII440FXState **pi440fx_state, int *piix_devfn,
>>                           ISABus **isa_bus, qemu_irq *pic,
>>     diff --git a/include/hw/southbridge/piix.h
>>     b/include/hw/southbridge/piix.h
>>     index add352456b..79ebe0089b 100644
>>     --- a/include/hw/southbridge/piix.h
>>     +++ b/include/hw/southbridge/piix.h
>>     @@ -18,6 +18,12 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn,
>>     uint32_t smb_io_base,
>>                             qemu_irq sci_irq, qemu_irq smi_irq,
>>                             int smm_enabled, DeviceState **piix4_pm);
>>
>>     +/*
>>     + * Reset Control Register: PCI-accessible ISA-Compatible Register
>>     at address
>>     + * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0,
>>     8086:7000).
>>     + */
>>     +#define RCR_IOPORT 0xcf9
>>     +
>>       extern PCIDevice *piix4_dev;
>>
>>       DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus,
>>     --     2.21.0
>>
>>
>>
diff mbox series

Patch

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 3292703de7..3770575c1a 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -27,6 +27,7 @@ 
 #include "hw/irq.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_host.h"
+#include "hw/southbridge/piix.h"
 #include "hw/qdev-properties.h"
 #include "hw/isa/isa.h"
 #include "hw/sysbus.h"
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 183326d9fe..1c20b96571 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -257,12 +257,6 @@  typedef struct PCII440FXState PCII440FXState;
 
 #define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "igd-passthrough-i440FX"
 
-/*
- * Reset Control Register: PCI-accessible ISA-Compatible Register at address
- * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0, 8086:7000).
- */
-#define RCR_IOPORT 0xcf9
-
 PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     PCII440FXState **pi440fx_state, int *piix_devfn,
                     ISABus **isa_bus, qemu_irq *pic,
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index add352456b..79ebe0089b 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -18,6 +18,12 @@  I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
                       qemu_irq sci_irq, qemu_irq smi_irq,
                       int smm_enabled, DeviceState **piix4_pm);
 
+/*
+ * Reset Control Register: PCI-accessible ISA-Compatible Register at address
+ * 0xcf9, provided by the PCI/ISA bridge (PIIX3 PCI function 0, 8086:7000).
+ */
+#define RCR_IOPORT 0xcf9
+
 extern PCIDevice *piix4_dev;
 
 DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus,