Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/59284 )
Change subject: internal: Hide internal_buses_supported state behind methods ......................................................................
internal: Hide internal_buses_supported state behind methods
Push the internal_buses_supported global state inwards to become a static local to internal.c and hide the manipulation of the state behind methods to allow reasoning about it.
BUG=none TEST=builds
Change-Id: I2f2e136b2d618f120a177f4c2ca15957783800c1 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M board_enable.c M chipset_enable.c M ichspi.c M internal.c M it85spi.c M it87spi.c M programmer.h 7 files changed, 44 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/59284/1
diff --git a/board_enable.c b/board_enable.c index 0141d1c..7832da5 100644 --- a/board_enable.c +++ b/board_enable.c @@ -601,7 +601,7 @@ uint8_t tmp; int ret = 0;
- if (!(internal_buses_supported & BUS_PARALLEL)) + if (!is_internal_buses_supported(BUS_PARALLEL, BUS_NONE)) return 1;
enter_conf_mode_ite(port); @@ -609,7 +609,7 @@ /* Check if at least one flash segment is enabled. */ if (tmp & 0xf0) { /* The IT8705F will respond to LPC cycles and translate them. */ - internal_buses_supported &= BUS_PARALLEL; + add_internal_buses_supported(BUS_PARALLEL); /* Flash ROM I/F Writes Enable */ tmp |= 0x04; msg_pdbg("Enabling IT8705F flash ROM interface write.\n"); diff --git a/chipset_enable.c b/chipset_enable.c index f48d311..145a570 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -67,13 +67,13 @@ tmp = pci_read_byte(dev, 0x40) & 0x3; switch (tmp) { case 3: - internal_buses_supported &= BUS_FWH; + add_internal_buses_supported(BUS_FWH); break; case 2: - internal_buses_supported &= BUS_LPC; + add_internal_buses_supported(BUS_LPC); break; default: - internal_buses_supported &= BUS_PARALLEL; + add_internal_buses_supported(BUS_PARALLEL); break; }
@@ -223,7 +223,7 @@ uint16_t old, new; uint16_t xbcs = 0x4e; /* X-Bus Chip Select register. */
- internal_buses_supported &= BUS_PARALLEL; + add_internal_buses_supported(BUS_PARALLEL);
old = pci_read_word(dev, xbcs);
@@ -558,7 +558,7 @@ if ((err = enable_flash_ich_fwh_decode(dev, ich_generation)) != 0) return err;
- internal_buses_supported &= BUS_FWH; + add_internal_buses_supported(BUS_FWH); return enable_flash_ich_bios_cntl_config_space(dev, ich_generation, bios_cntl); }
@@ -1013,7 +1013,7 @@ if (ret_fwh == ERROR_FATAL) return ret_fwh;
- internal_buses_supported &= BUS_FWH; + add_internal_buses_supported(BUS_FWH);
/* Get physical address of SPI Base Address and map it */ uint32_t sbase = pci_read_long(dev, 0x54) & 0xfffffe00; @@ -1167,7 +1167,7 @@ #define CS5530_ENABLE_SA2320 (1 << 2) #define CS5530_ENABLE_SA20 (1 << 6)
- internal_buses_supported &= BUS_PARALLEL; + add_internal_buses_supported(BUS_PARALLEL); /* Decode 0x000E0000-0x000FFFFF (128 kB), not just 64 kB, and * decode 0xFF000000-0xFFFFFFFF (16 MB), not just 256 kB. * FIXME: Should we really touch the low mapping below 1 MB? Flashrom @@ -1355,7 +1355,7 @@ msg_pdbg("done.\n"); }
- internal_buses_supported &= BUS_LPC | BUS_FWH; + add_internal_buses_supported(BUS_LPC | BUS_FWH);
ret = sb600_probe_spi(dev);
@@ -1503,7 +1503,7 @@ { uint8_t tmp;
- internal_buses_supported &= BUS_PARALLEL; + add_internal_buses_supported(BUS_PARALLEL);
tmp = INB(0xc06); tmp |= 0x1; @@ -1592,7 +1592,7 @@ switch ((val >> 5) & 0x3) { case 0x0: ret = enable_flash_mcp55(dev, name); - internal_buses_supported &= BUS_LPC; + add_internal_buses_supported(BUS_LPC); msg_pdbg("Flash bus type is LPC\n"); break; case 0x2: @@ -1600,12 +1600,12 @@ /* SPI is added in mcp6x_spi_init if it works. * Do we really want to disable LPC in this case? */ - internal_buses_supported = BUS_NONE; + clear_internal_buses_supported(); msg_pdbg("Flash bus type is SPI\n"); break; default: /* Should not happen. */ - internal_buses_supported = BUS_NONE; + clear_internal_buses_supported(); msg_pwarn("Flash bus type is unknown (none)\n"); msg_pinfo("Please send the log files created by "flashrom -p internal -o logfile" to\n" "flashrom@flashrom.org with "your board name: flashrom -V" as the subject to\n" @@ -2152,7 +2152,7 @@ "flashrom@flashrom.org including a verbose " "(-V) log.\nThank you!\n"); } - if (!(chipset_enables[i].buses & (internal_buses_supported | BUS_SPI))) { + if (!is_internal_buses_supported(chipset_enables[i].buses, BUS_SPI)) { msg_pdbg("Skipping chipset enable: No supported buses enabled.\n"); continue; } diff --git a/ichspi.c b/ichspi.c index d345af2..047a82a 100644 --- a/ichspi.c +++ b/ichspi.c @@ -2072,7 +2072,7 @@ /* Do we really need no write enable? Like the LPC one at D17F0 0x40 */
/* Not sure if it speaks all these bus protocols. */ - internal_buses_supported &= BUS_LPC | BUS_FWH; + add_internal_buses_supported(BUS_LPC | BUS_FWH); ich_generation = CHIPSET_ICH7; register_spi_master(&spi_master_via, NULL);
diff --git a/internal.c b/internal.c index 508906d..189a1e9 100644 --- a/internal.c +++ b/internal.c @@ -27,7 +27,27 @@ int force_boardenable = 0; int force_boardmismatch = 0;
-enum chipbustype internal_buses_supported = BUS_NONE; +static enum chipbustype internal_buses_supported = BUS_NONE; + +void clear_internal_buses_supported(void) +{ + internal_buses_supported = BUS_NONE; +} + +enum chipbustype get_internal_buses_supported(void) +{ + return internal_buses_supported; +} + +void add_internal_buses_supported(enum chipbustype flag) +{ + internal_buses_supported &= flag; +} + +bool is_internal_buses_supported(enum chipbustype flag, enum chipbustype mask) +{ + return (internal_buses_supported | mask) & flag; +}
#if defined(__i386__) || defined(__x86_64__) void probe_superio(void) diff --git a/it85spi.c b/it85spi.c index 17e4e10..cbdfcbd 100644 --- a/it85spi.c +++ b/it85spi.c @@ -297,13 +297,13 @@ unsigned int shm_io_base = 0;
msg_pdbg("%s():%d superio.vendor=0x%02x internal_buses_supported=0x%x\n", - __func__, __LINE__, s.vendor, internal_buses_supported); + __func__, __LINE__, s.vendor, get_internal_buses_supported());
/* Check for FWH because IT85 listens to FWH cycles. * FIXME: The big question is whether FWH cycles are necessary * for communication even if LPC_IO is defined. */ - if (!(internal_buses_supported & BUS_FWH)) { + if (!is_internal_buses_supported(BUS_FWH, BUS_NONE)) { msg_pdbg("%s():%d buses not support FWH\n", __func__, __LINE__); return 1; } diff --git a/it87spi.c b/it87spi.c index 5f6fb65..3655cbc 100644 --- a/it87spi.c +++ b/it87spi.c @@ -418,7 +418,7 @@ data->flashport = flashport; data->fast_spi = 1;
- if (internal_buses_supported & BUS_SPI) + if (is_internal_buses_supported(BUS_SPI, BUS_NONE)) msg_pdbg("Overriding chipset SPI with IT87 SPI.\n"); /* FIXME: Add the SPI bus or replace the other buses with it? */ return register_spi_master(&spi_master_it87xx, data); diff --git a/programmer.h b/programmer.h index 1f64d5a..d6dd68e 100644 --- a/programmer.h +++ b/programmer.h @@ -279,7 +279,10 @@ extern int force_boardmismatch; void probe_superio(void); int register_superio(struct superio s); -extern enum chipbustype internal_buses_supported; +void clear_internal_buses_supported(void); +enum chipbustype get_internal_buses_supported(void); +void add_internal_buses_supported(enum chipbustype flag); +bool is_internal_buses_supported(enum chipbustype flag, enum chipbustype mask); #endif
/* bitbang_spi.c */