commit 8a9de2454aa5f2f0a2a928ff61ef53634eff6dee
author Willy Tu <wltu@google.com> Fri Oct 30 11:00:21 2020 -0700
committer Willy Tu <wltu@google.com> Tue Nov 03 15:38:01 2020 -0800
tree 024a9aead5b01b4f25b6f658edfea55894212db3
parent c893f43d5f8a3c0b05c0f6f67d1b232a8117cf71

Add option to skip p2a bridge disable

Add a new constructor for P2aDataHandler with skipBridgeDisable bool to
enable an option to skip disableBridge() in AspeedPciBridge and
NuvotonPciBridge.

Signed-off-by: Willy Tu <wltu@google.com>
Change-Id: I439bbaa2b7295adc54a8aa98157db60a7e820837

tools/test/tools_pci_unittest.cpp

diff --git a/tools/test/tools_pci_unittest.cpp b/tools/test/tools_pci_unittest.cpp
index 4a45468..7f69db2 100644
--- a/tools/test/tools_pci_unittest.cpp
+++ b/tools/test/tools_pci_unittest.cpp
@@ -71,7 +71,8 @@
     virtual struct pci_device getDevice() const = 0;
     virtual void expectSetup(PciAccessMock& pciMock,
                              const struct pci_device& dev) const {};
-    virtual std::unique_ptr<PciBridgeIntf> getBridge(PciAccess* pci) const = 0;
+    virtual std::unique_ptr<PciBridgeIntf>
+        getBridge(PciAccess* pci, bool skipBridgeDisable = false) const = 0;
     virtual std::string getName() const = 0;
 };
 
@@ -120,9 +121,10 @@
             .WillOnce(Return(0));
     }
 
-    std::unique_ptr<PciBridgeIntf> getBridge(PciAccess* pci) const override
+    std::unique_ptr<PciBridgeIntf>
+        getBridge(PciAccess* pci, bool skipBridgeDisable = false) const override
     {
-        return std::make_unique<NuvotonPciBridge>(pci);
+        return std::make_unique<NuvotonPciBridge>(pci, skipBridgeDisable);
     }
 
     std::string getName() const override
@@ -164,9 +166,10 @@
         return dev;
     }
 
-    std::unique_ptr<PciBridgeIntf> getBridge(PciAccess* pci) const override
+    std::unique_ptr<PciBridgeIntf>
+        getBridge(PciAccess* pci, bool skipBridgeDisable = false) const override
     {
-        return std::make_unique<AspeedPciBridge>(pci);
+        return std::make_unique<AspeedPciBridge>(pci, skipBridgeDisable);
     }
 
     std::string getName() const override
@@ -551,6 +554,33 @@
     nuvotonDevice.getBridge(&pciMock);
 }
 
+/* Make sure it skips the disable bridge call when skipBridgeDisable is true */
+TEST(NuvotonBridgeTest, SkipDisable)
+{
+    PciAccessMock pciMock;
+    struct pci_device dev;
+    std::vector<std::uint8_t> region(mockRegionSize);
+
+    constexpr std::uint8_t defaultVal = 0x40;
+
+    /* Only set standard expectations; not those from nuvotonDevice */
+    expectSetup(pciMock, dev, &nuvotonDevice, region.data(), false);
+
+    {
+        InSequence in;
+
+        /* Only expect call for enableBridge() */
+        EXPECT_CALL(pciMock, pci_device_cfg_read_u8(Eq(&dev), NotNull(),
+                                                    NuvotonDevice::config))
+            .WillOnce(DoAll(
+                SetArgPointee<1>(defaultVal | NuvotonDevice::bridgeEnabled),
+                Return(0)));
+    }
+
+    /* Setting skipBridgeDisable to true */
+    nuvotonDevice.getBridge(&pciMock, true);
+}
+
 TEST(AspeedWriteTest, TooLarge)
 {
     PciAccessMock pciMock;
@@ -644,6 +674,41 @@
     }
 }
 
+/* Make sure the config region remains the same even after cleanup if
+ * skipBridgeDisable is true */
+TEST(AspeedBridgeTest, SkipDisable)
+{
+    PciAccessMock pciMock;
+    struct pci_device dev;
+    std::vector<std::uint8_t> region(mockRegionSize);
+
+    constexpr std::uint8_t defaultVal = 0x42;
+
+    region[AspeedDevice::config] = defaultVal | AspeedDevice::bridgeEnabled;
+
+    expectSetup(pciMock, dev, &aspeedDevice, region.data());
+
+    /* Setting skipBridgeDisable to true */
+    std::unique_ptr<PciBridgeIntf> bridge =
+        aspeedDevice.getBridge(&pciMock, true);
+
+    {
+        std::vector<std::uint8_t> enabledRegion(mockRegionSize);
+        enabledRegion[AspeedDevice::config] =
+            defaultVal | AspeedDevice::bridgeEnabled;
+        EXPECT_THAT(region, ContainerEq(enabledRegion));
+    }
+
+    bridge.reset();
+
+    {
+        std::vector<std::uint8_t> disabledRegion(mockRegionSize);
+        disabledRegion[AspeedDevice::config] =
+            defaultVal | AspeedDevice::bridgeEnabled;
+        EXPECT_THAT(region, ContainerEq(disabledRegion));
+    }
+}
+
 /* Make sure the bridge gets enabled when needed */
 TEST(AspeedBridgeTest, NotEnabledSuccess)
 {