New patch to review for coreboot: 0dd9d5b ASRock E350M1: BiosCallOuts.c: Let `BiosGnbPcieSlotReset()` return `AGESA_UNSUPPORTED`

[Paul Menzel]

Paul Menzel ([e-mail]) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/2489

-gerrit

commit 0dd9d5b4d6b0a5ad3f0281e0961f84b06db69d64
Author: Paul Menzel 
Date:   Sat Feb 23 00:15:49 2013 +0100

ASRock E350M1: BiosCallOuts.c: Let `BiosGnbPcieSlotReset()` return `AGESA_UNSUPPORTED`

Quoting Jens Rottmann [1]:

Ok, to make it short:
    In my opinion this is safe. The original code is definitely wrong, and fixing it can only make it better.
    If you really want an explicit test, try to check if a PCIe device on 00:7.0 still works as it did before.
    And if the E350M1 board has a PCI bus then the whole BiosGnbPcieSlotReset() function has zero effect anyway and you can replace all of it with a single "return AGESA_UNSUPPORTED".

[…]

> 1 x PCI Express 2.0 x16 slot (@ x4) Is that what you meant?
    No, I meant (old) PCI, not PCIe. If the E350M1 had that then we'd _know_ the GPIOs 2,21,25 cannot be connected as assumed by BiosGnbPcieSlotReset().
    Nevertheless I still think this whole function is bogus for the E350M1. The function assumes GPIO21 is wired to reset APU PCIe lane 0+1 (PCIe x8, port 4+5 as Coreboot/AGESA calls it), GPIO25 resets lane 2 (PCIe x4) and GPIO02 lane 3.
    But the E350M1 has PCIe x16 i.e. probably APU lanes 0-3 bundled, completely different layout. They could have chosen GPIO21 to force resets, or 25 - or maybe 50 like on the Persimmon or any other they fancied or - and this is the most probable - none at all.
    Having BiosGnbPcieSlotReset() toggle some GPIOs without knowing what they do on the E350M1 (if anything at all) is nonsense. In my opinion this whole function should just "return AGESA_UNSUPPORTED" and good riddance.

[1] http://review.coreboot.org/#/c/2445/

Change-Id: Iac66da41182e838c7e6925250cc3982adbb3e4ec
    Reported-by: Jens Rottmann 
    Signed-off-by: Paul Menzel 
---
 src/mainboard/asrock/e350m1/BiosCallOuts.c | 77 +-----------------------------
 1 file changed, 1 insertion(+), 76 deletions(-)

diff --git a/src/mainboard/asrock/e350m1/BiosCallOuts.c b/src/mainboard/asrock/e350m1/BiosCallOuts.c
index 8aa4398..642a68e 100644
--- a/src/mainboard/asrock/e350m1/BiosCallOuts.c
+++ b/src/mainboard/asrock/e350m1/BiosCallOuts.c
@@ -525,80 +525,5 @@ AGESA_STATUS BiosHookBeforeExitSelfRefresh (UINT32 Func, UINT32 Data, VOID *Conf
 /* PCIE slot reset control */
 AGESA_STATUS BiosGnbPcieSlotReset (UINT32 Func, UINT32 Data, VOID *ConfigPtr)
 {
-  AGESA_STATUS Status;
-  UINTN                 FcnData;
-  PCIe_SLOT_RESET_INFO  *ResetInfo;
-
-  UINT32  GpioMmioAddr;
-  UINT32  AcpiMmioAddr;
-  UINT8   Data8;
-  UINT16  Data16;
-
-  FcnData   = Data;
-  ResetInfo = ConfigPtr;
-  // Get SB800 MMIO Base (AcpiMmioAddr)
-  WriteIo8(0xCD6, 0x27);
-  Data8 = ReadIo8(0xCD7);
-  Data16=Data8<<8;
-  WriteIo8(0xCD6, 0x26);
-  Data8 = ReadIo8(0xCD7);
-  Data16|=Data8;
-  AcpiMmioAddr = (UINT32)Data16 << 16;
-  Status = AGESA_UNSUPPORTED;
-  GpioMmioAddr = AcpiMmioAddr + GPIO_BASE;
-  switch (ResetInfo->ResetId)
-  {
-  case 4:
-      switch (ResetInfo->ResetControl)
-      {
-      case AssertSlotReset:
-        Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG21);
-        Data8 &= ~(UINT8)BIT6 ;
-        Write64Mem8(GpioMmioAddr+SB_GPIO_REG21, Data8);   // MXM_GPIO0. GPIO21
-        Status = AGESA_SUCCESS;
-        break;
-      case DeassertSlotReset:
-        Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG21);
-        Data8 |= BIT6 ;
-        Write64Mem8 (GpioMmioAddr+SB_GPIO_REG21, Data8);       // MXM_GPIO0. GPIO21
-        Status = AGESA_SUCCESS;
-        break;
-      }
-      break;
-  case 6:
-      switch (ResetInfo->ResetControl)
-      {
-      case AssertSlotReset:
-        Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG25);
-        Data8 &= ~(UINT8)BIT6 ;
-        Write64Mem8(GpioMmioAddr+SB_GPIO_REG25, Data8);   // PCIE_RST#_LAN, GPIO25
-        Status = AGESA_SUCCESS;
-        break;
-      case DeassertSlotReset:
-        Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG25);
-        Data8 |= BIT6 ;
-        Write64Mem8 (GpioMmioAddr+SB_GPIO_REG25, Data8);       // PCIE_RST#_LAN, GPIO25
-        Status = AGESA_SUCCESS;
-        break;
-      }
-      break;
-  case 7:
-      switch (ResetInfo->ResetControl)
-      {
-      case AssertSlotReset:
-        Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG02);
-        Data8 &= ~(UINT8)BIT6 ;
-        Write64Mem8(GpioMmioAddr+SB_GPIO_REG02, Data8);   // MPCIE_RST0, GPIO02
-        Status = AGESA_SUCCESS;
-        break;
-      case DeassertSlotReset:
-        Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG02);
-        Data8 |= BIT6 ;
-        Write64Mem8 (GpioMmioAddr+SB_GPIO_REG02, Data8);       // MPCIE_RST0, GPIO02
-        Status = AGESA_SUCCESS;
-        break;
-      }
-      break;
-  }
-  return  Status;
+  return AGESA_UNSUPPORTED;
 }