diff mbox series

[2/2] hw/pci-host/pam: Remove northbridge-specific PAM_REGIONS_COUNT

Message ID 20240309134056.1605-3-shentey@gmail.com
State New
Headers show
Series Make PAMMemoryRegion less Intel-specific | expand

Commit Message

Bernhard Beschow March 9, 2024, 1:40 p.m. UTC 
PAM_REGIONS_COUNT being 13 seems to be Intel-specific. There are VIA 82cXX
northbridges having only 10, for example. Communicate this by having
northbridge-specific constants.

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

--

Do we need the constants or can we omit them, given they're just used once?
---
 include/hw/pci-host/i440fx.h | 4 +++-
 include/hw/pci-host/pam.h    | 2 --
 include/hw/pci-host/q35.h    | 4 +++-
 hw/pci-host/q35.c            | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

Comments

Philippe Mathieu-Daudé March 9, 2024, 2:02 p.m. UTC | #1 
On 9/3/24 14:40, Bernhard Beschow wrote:
> PAM_REGIONS_COUNT being 13 seems to be Intel-specific. There are VIA 82cXX
> northbridges having only 10, for example. Communicate this by having
> northbridge-specific constants.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> 
> --
> 
> Do we need the constants or can we omit them, given they're just used once?

We don't need but prefer, since the definition is self-explicit
and we don't have to lookup datasheet for magic value.

> ---
>   include/hw/pci-host/i440fx.h | 4 +++-
>   include/hw/pci-host/pam.h    | 2 --
>   include/hw/pci-host/q35.h    | 4 +++-
>   hw/pci-host/q35.c            | 2 +-
>   4 files changed, 7 insertions(+), 5 deletions(-)

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

Patch

diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
index c988f70890..9e9b252660 100644
--- a/include/hw/pci-host/i440fx.h
+++ b/include/hw/pci-host/i440fx.h
@@ -22,12 +22,14 @@ 
 
 OBJECT_DECLARE_SIMPLE_TYPE(PCII440FXState, I440FX_PCI_DEVICE)
 
+#define I440FX_HOST_PAM_REGIONS_COUNT 13
+
 struct PCII440FXState {
     /*< private >*/
     PCIDevice parent_obj;
     /*< public >*/
 
-    PAMMemoryRegion pam_regions[PAM_REGIONS_COUNT];
+    PAMMemoryRegion pam_regions[I440FX_HOST_PAM_REGIONS_COUNT];
     MemoryRegion smram_region;
     MemoryRegion smram, low_smram;
 };
diff --git a/include/hw/pci-host/pam.h b/include/hw/pci-host/pam.h
index b9b33aecc8..25dbe6feaf 100644
--- a/include/hw/pci-host/pam.h
+++ b/include/hw/pci-host/pam.h
@@ -79,8 +79,6 @@ 
 #define SMRAM_C_BASE_SEG_MASK  ((uint8_t)0x7)
 #define SMRAM_C_BASE_SEG       ((uint8_t)0x2)  /* hardwired to b010 */
 
-#define PAM_REGIONS_COUNT       13
-
 typedef struct PAMMemoryRegion {
     MemoryRegion alias[4];  /* index = mode value */
     uint8_t mode;
diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index bafcbe6752..618ecf05f4 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -35,6 +35,8 @@  OBJECT_DECLARE_SIMPLE_TYPE(Q35PCIHost, Q35_HOST_DEVICE)
 #define TYPE_MCH_PCI_DEVICE "mch"
 OBJECT_DECLARE_SIMPLE_TYPE(MCHPCIState, MCH_PCI_DEVICE)
 
+#define MCH_HOST_PAM_REGIONS_COUNT 13
+
 struct MCHPCIState {
     /*< private >*/
     PCIDevice parent_obj;
@@ -44,7 +46,7 @@  struct MCHPCIState {
     MemoryRegion *pci_address_space;
     MemoryRegion *system_memory;
     MemoryRegion *address_space_io;
-    PAMMemoryRegion pam_regions[PAM_REGIONS_COUNT];
+    PAMMemoryRegion pam_regions[MCH_HOST_PAM_REGIONS_COUNT];
     MemoryRegion smram_region, open_high_smram;
     MemoryRegion smram, low_smram, high_smram;
     MemoryRegion tseg_blackhole, tseg_window;
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 947d9aa9c4..6782bf4216 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -329,7 +329,7 @@  static void mch_update_pam(MCHPCIState *mch)
     int i;
 
     memory_region_transaction_begin();
-    for (i = 0; i < 13; i++) {
+    for (i = 0; i < ARRAY_SIZE(mch->pam_regions); i++) {
         uint8_t reg = pd->config[MCH_HOST_BRIDGE_PAM0 + DIV_ROUND_UP(i, 2)];
         pam_update(&mch->pam_regions[i],
                    (reg >> ((!(i & 1)) * 4)) & MCH_HOST_BRIDGE_PAM_MASK);