Change in flashrom[master]: internal: Hide internal_buses_supported state behind methods

[Edward O'Callaghan (Code Review)]

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 
---
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"
    		  "[e-mail] with "your board name: flashrom -V" as the subject to\n"
@@ -2152,7 +2152,7 @@
    			  "[e-mail] 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 */