Change in flashrom[master]: sb600spi.c: Generalise determin_generation() after Yangtze
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36426 ) Change subject: sb600spi.c: Generalise determin_generation() after Yangtze ...................................................................... sb600spi.c: Generalise determin_generation() after Yangtze Drop dead USE_YANGTZE_HEURISTICS code. Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 4 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/36426/1 diff --git a/sb600spi.c b/sb600spi.c index 48baf15..482c2f6 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -118,22 +118,6 @@ } else if (dev->device_id == 0x780e) { /* The PCI ID of the LPC bridge doesn't change between Hudson-2/3/4 and Yangtze (Kabini/Temash) * although they use different SPI interfaces. */ -#ifdef USE_YANGTZE_HEURISTICS - /* This heuristic accesses the SPI interface MMIO BAR at locations beyond those supported by - * Hudson in the hope of getting 0xff readback on older chipsets and non-0xff readback on - * Yangtze (and newer, compatible chipsets). */ - int i; - msg_pdbg("Checking for AMD Yangtze (Kabini/Temash) or later... "); - for (i = 0x20; i <= 0x4f; i++) { - if (mmio_readb(sb600_spibar + i) != 0xff) { - amd_gen = CHIPSET_YANGTZE; - msg_pdbg("found.\n"); - return 0; - } - } - msg_pdbg("not found. Assuming Hudson.\n"); - amd_gen = CHIPSET_HUDSON234; -#else int rev = find_smbus_dev(0x1022, 0x780B); if (rev < 0) return -1; @@ -165,7 +149,6 @@ } -#endif } else msg_pwarn("%s: Unknown LPC device %" PRIx16 ":%" PRIx16 ".\n" "Please report this to flashrom@flashrom.org and include this log and\n" @@ -595,7 +578,7 @@ msg_pdbg("SpiRomEnable=%i", (tmp >> 1) & 0x1); if (amd_gen == CHIPSET_SB7XX) msg_pdbg(", AltSpiCSEnable=%i, AbortEnable=%i", tmp & 0x1, (tmp >> 2) & 0x1); - else if (amd_gen == CHIPSET_YANGTZE) + else if (amd_gen >= CHIPSET_YANGTZE) msg_pdbg(", RouteTpm2Sp=%i", (tmp >> 3) & 0x1); tmp = pci_read_byte(dev, 0xba); @@ -631,13 +614,13 @@ */ tmp = mmio_readl(sb600_spibar + 0x00); msg_pdbg("(0x%08" PRIx32 ") SpiArbEnable=%i", tmp, (tmp >> 19) & 0x1); - if (amd_gen == CHIPSET_YANGTZE) + if (amd_gen >= CHIPSET_YANGTZE) msg_pdbg(", IllegalAccess=%i", (tmp >> 21) & 0x1); msg_pdbg(", SpiAccessMacRomEn=%i, SpiHostAccessRomEn=%i, ArbWaitCount=%i", (tmp >> 22) & 0x1, (tmp >> 23) & 0x1, (tmp >> 24) & 0x7); - if (amd_gen != CHIPSET_YANGTZE) + if (amd_gen < CHIPSET_YANGTZE) msg_pdbg(", SpiBridgeDisable=%i", (tmp >> 27) & 0x1); switch (amd_gen) { @@ -710,7 +693,7 @@ return ERROR_FATAL; /* Starting with Yangtze the SPI controller got a different interface with a much bigger buffer. */ - if (amd_gen != CHIPSET_YANGTZE) + if (amd_gen < CHIPSET_YANGTZE) register_spi_master(&spi_master_sb600); else register_spi_master(&spi_master_yangtze); -- To view, visit https://review.coreboot.org/c/flashrom/+/36426 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Gerrit-Change-Number: 36426 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: newchange
Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/36426 to look at the new patch set (#2). Change subject: sb600spi.c: Generalise determin_generation() after Yangtze ...................................................................... sb600spi.c: Generalise determin_generation() after Yangtze Drop dead USE_YANGTZE_HEURISTICS code and add Promontory support. Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 9 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/36426/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/36426 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Gerrit-Change-Number: 36426 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/36426 to look at the new patch set (#3). Change subject: sb600spi.c: Generalise determin_generation() after Yangtze ...................................................................... sb600spi.c: Generalise determin_generation() after Yangtze Drop dead USE_YANGTZE_HEURISTICS code and add Promontory support. Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 9 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/36426/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/36426 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Gerrit-Change-Number: 36426 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36426 ) Change subject: sb600spi.c: Generalise determin_generation() after Yangtze ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Is there a public datasheet one could read to check this? https://review.coreboot.org/c/flashrom/+/36426/3/sb600spi.c File sb600spi.c: https://review.coreboot.org/c/flashrom/+/36426/3/sb600spi.c@155 PS3, Line 155: Drop these empty lines as well? -- To view, visit https://review.coreboot.org/c/flashrom/+/36426 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Gerrit-Change-Number: 36426 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 14 Nov 2019 21:26:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/36426 ) Change subject: sb600spi.c: Generalise determin_generation() after Yangtze ...................................................................... sb600spi.c: Generalise determin_generation() after Yangtze Drop dead USE_YANGTZE_HEURISTICS code and add Promontory support. Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/36426 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de> --- M sb600spi.c 1 file changed, 9 insertions(+), 21 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/sb600spi.c b/sb600spi.c index 050d210..cb5c4a4 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -49,6 +49,7 @@ CHIPSET_HUDSON234, CHIPSET_BOLTON, CHIPSET_YANGTZE, + CHIPSET_PROMONTORY, }; static enum amd_chipset amd_gen = CHIPSET_AMD_UNKNOWN; @@ -118,22 +119,6 @@ } else if (dev->device_id == 0x780e) { /* The PCI ID of the LPC bridge doesn't change between Hudson-2/3/4 and Yangtze (Kabini/Temash) * although they use different SPI interfaces. */ -#ifdef USE_YANGTZE_HEURISTICS - /* This heuristic accesses the SPI interface MMIO BAR at locations beyond those supported by - * Hudson in the hope of getting 0xff readback on older chipsets and non-0xff readback on - * Yangtze (and newer, compatible chipsets). */ - int i; - msg_pdbg("Checking for AMD Yangtze (Kabini/Temash) or later... "); - for (i = 0x20; i <= 0x4f; i++) { - if (mmio_readb(sb600_spibar + i) != 0xff) { - amd_gen = CHIPSET_YANGTZE; - msg_pdbg("found.\n"); - return 0; - } - } - msg_pdbg("not found. Assuming Hudson.\n"); - amd_gen = CHIPSET_HUDSON234; -#else int rev = find_smbus_dev_rev(0x1022, 0x780B); if (rev < 0) return -1; @@ -158,6 +143,9 @@ if (rev == 0x4a) { amd_gen = CHIPSET_YANGTZE; msg_pdbg("Yangtze detected.\n"); + } else if (rev == 0x4b) { + amd_gen = CHIPSET_PROMONTORY; + msg_pdbg("Promontory detected.\n"); } else { msg_pwarn("FCH device found but SMBus revision 0x%02x does not match known values.\n" "Please report this to flashrom@flashrom.org and include this log and\n" @@ -165,7 +153,6 @@ } -#endif } else msg_pwarn("%s: Unknown LPC device %" PRIx16 ":%" PRIx16 ".\n" "Please report this to flashrom@flashrom.org and include this log and\n" @@ -595,7 +582,7 @@ msg_pdbg("SpiRomEnable=%i", (tmp >> 1) & 0x1); if (amd_gen == CHIPSET_SB7XX) msg_pdbg(", AltSpiCSEnable=%i, AbortEnable=%i", tmp & 0x1, (tmp >> 2) & 0x1); - else if (amd_gen == CHIPSET_YANGTZE) + else if (amd_gen >= CHIPSET_YANGTZE) msg_pdbg(", RouteTpm2Sp=%i", (tmp >> 3) & 0x1); tmp = pci_read_byte(dev, 0xba); @@ -631,13 +618,13 @@ */ tmp = mmio_readl(sb600_spibar + 0x00); msg_pdbg("(0x%08" PRIx32 ") SpiArbEnable=%i", tmp, (tmp >> 19) & 0x1); - if (amd_gen == CHIPSET_YANGTZE) + if (amd_gen >= CHIPSET_YANGTZE) msg_pdbg(", IllegalAccess=%i", (tmp >> 21) & 0x1); msg_pdbg(", SpiAccessMacRomEn=%i, SpiHostAccessRomEn=%i, ArbWaitCount=%i", (tmp >> 22) & 0x1, (tmp >> 23) & 0x1, (tmp >> 24) & 0x7); - if (amd_gen != CHIPSET_YANGTZE) + if (amd_gen < CHIPSET_YANGTZE) msg_pdbg(", SpiBridgeDisable=%i", (tmp >> 27) & 0x1); switch (amd_gen) { @@ -647,6 +634,7 @@ case CHIPSET_SB89XX: case CHIPSET_HUDSON234: case CHIPSET_YANGTZE: + case CHIPSET_PROMONTORY: msg_pdbg(", SpiBusy=%i", (tmp >> 31) & 0x1); default: break; } @@ -710,7 +698,7 @@ return ERROR_FATAL; /* Starting with Yangtze the SPI controller got a different interface with a much bigger buffer. */ - if (amd_gen != CHIPSET_YANGTZE) + if (amd_gen < CHIPSET_YANGTZE) register_spi_master(&spi_master_sb600); else register_spi_master(&spi_master_yangtze); -- To view, visit https://review.coreboot.org/c/flashrom/+/36426 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Gerrit-Change-Number: 36426 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: merged
participants (2)
-
Edward O'Callaghan (Code Review) -
Nico Huber (Code Review)