Change in flashrom[master]: sb600spi.c: Fold up debug logic into determine_generation()
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36425 ) Change subject: sb600spi.c: Fold up debug logic into determine_generation() ...................................................................... sb600spi.c: Fold up debug logic into determine_generation() Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 12 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/36425/1 diff --git a/sb600spi.c b/sb600spi.c index c89d5a9..48baf15 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -91,7 +91,8 @@ return pci_read_byte(smbus_dev, PCI_REVISION_ID); } -static void determine_generation(struct pci_dev *dev) +/* Determine the chipset's version and identify the respective SMBUS device. */ +static int determine_generation(struct pci_dev *dev) { amd_gen = CHIPSET_AMD_UNKNOWN; msg_pdbg2("Trying to determine the generation of the SPI interface... "); @@ -101,7 +102,7 @@ } else if (dev->device_id == 0x439d) { int rev = find_smbus_dev(0x1002, 0x4385); if (rev < 0) - return; + return -1; if (rev >= 0x39 && rev <= 0x3D) { amd_gen = CHIPSET_SB7XX; msg_pdbg("SB7xx/SP5100 detected.\n"); @@ -127,7 +128,7 @@ if (mmio_readb(sb600_spibar + i) != 0xff) { amd_gen = CHIPSET_YANGTZE; msg_pdbg("found.\n"); - return; + return 0; } } msg_pdbg("not found. Assuming Hudson.\n"); @@ -135,7 +136,7 @@ #else int rev = find_smbus_dev(0x1022, 0x780B); if (rev < 0) - return; + return -1; if (rev >= 0x11 && rev <= 0x15) { amd_gen = CHIPSET_HUDSON234; msg_pdbg("Hudson-2/3/4 detected.\n"); @@ -153,7 +154,7 @@ } else if (dev->device_id == 0x790e) { int rev = find_smbus_dev(0x1022, 0x790B); if (rev < 0) - return; + return -1; if (rev == 0x4a) { amd_gen = CHIPSET_YANGTZE; msg_pdbg("Yangtze detected.\n"); @@ -170,6 +171,11 @@ "Please report this to flashrom@flashrom.org and include this log and\n" "the output of lspci -nnvx, thanks!\n", __func__, dev->vendor_id, dev->device_id); + if (amd_gen == CHIPSET_AMD_UNKNOWN) { + msg_perr("Could not determine chipset generation."); + return -1; + } + return 0; } static void reset_internal_fifo_pointer(void) @@ -565,11 +571,8 @@ */ sb600_spibar += tmp & 0xfff; - determine_generation(dev); - if (amd_gen == CHIPSET_AMD_UNKNOWN) { - msg_perr("Could not determine chipset generation."); + if (determine_generation(dev) < 0) return ERROR_NONFATAL; - } /* How to read the following table and similar ones in this file: * "?" means we have no datasheet for this chipset generation or it doesn't have any relevant info. -- To view, visit https://review.coreboot.org/c/flashrom/+/36425 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Gerrit-Change-Number: 36425 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: newchange
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36425 ) Change subject: sb600spi.c: Fold up debug logic into determine_generation() ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/36425 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Gerrit-Change-Number: 36425 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Tue, 29 Oct 2019 07:53:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Hello Arthur Heymans, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/36425 to look at the new patch set (#2). Change subject: sb600spi.c: Fold up debug logic into determine_generation() ...................................................................... sb600spi.c: Fold up debug logic into determine_generation() Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 21 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/36425/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/36425 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Gerrit-Change-Number: 36425 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Hello Arthur Heymans, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/36425 to look at the new patch set (#3). Change subject: sb600spi.c: Fold up debug logic into determine_generation() ...................................................................... sb600spi.c: Fold up debug logic into determine_generation() Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 12 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/36425/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/36425 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Gerrit-Change-Number: 36425 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> 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/+/36425 ) Change subject: sb600spi.c: Fold up debug logic into determine_generation() ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/36425 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Gerrit-Change-Number: 36425 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> 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:21:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/36425 ) Change subject: sb600spi.c: Fold up debug logic into determine_generation() ...................................................................... sb600spi.c: Fold up debug logic into determine_generation() Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/36425 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> --- M sb600spi.c 1 file changed, 12 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Arthur Heymans: Looks good to me, approved diff --git a/sb600spi.c b/sb600spi.c index 6466795..050d210 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -91,7 +91,8 @@ return pci_read_byte(smbus_dev, PCI_REVISION_ID); } -static void determine_generation(struct pci_dev *dev) +/* Determine the chipset's version and identify the respective SMBUS device. */ +static int determine_generation(struct pci_dev *dev) { amd_gen = CHIPSET_AMD_UNKNOWN; msg_pdbg2("Trying to determine the generation of the SPI interface... "); @@ -101,7 +102,7 @@ } else if (dev->device_id == 0x439d) { int rev = find_smbus_dev_rev(0x1002, 0x4385); if (rev < 0) - return; + return -1; if (rev >= 0x39 && rev <= 0x3D) { amd_gen = CHIPSET_SB7XX; msg_pdbg("SB7xx/SP5100 detected.\n"); @@ -127,7 +128,7 @@ if (mmio_readb(sb600_spibar + i) != 0xff) { amd_gen = CHIPSET_YANGTZE; msg_pdbg("found.\n"); - return; + return 0; } } msg_pdbg("not found. Assuming Hudson.\n"); @@ -135,7 +136,7 @@ #else int rev = find_smbus_dev_rev(0x1022, 0x780B); if (rev < 0) - return; + return -1; if (rev >= 0x11 && rev <= 0x15) { amd_gen = CHIPSET_HUDSON234; msg_pdbg("Hudson-2/3/4 detected.\n"); @@ -153,7 +154,7 @@ } else if (dev->device_id == 0x790e) { int rev = find_smbus_dev_rev(0x1022, 0x790B); if (rev < 0) - return; + return -1; if (rev == 0x4a) { amd_gen = CHIPSET_YANGTZE; msg_pdbg("Yangtze detected.\n"); @@ -170,6 +171,11 @@ "Please report this to flashrom@flashrom.org and include this log and\n" "the output of lspci -nnvx, thanks!\n", __func__, dev->vendor_id, dev->device_id); + if (amd_gen == CHIPSET_AMD_UNKNOWN) { + msg_perr("Could not determine chipset generation."); + return -1; + } + return 0; } static void reset_internal_fifo_pointer(void) @@ -565,11 +571,8 @@ */ sb600_spibar += tmp & 0xfff; - determine_generation(dev); - if (amd_gen == CHIPSET_AMD_UNKNOWN) { - msg_perr("Could not determine chipset generation."); + if (determine_generation(dev) < 0) return ERROR_NONFATAL; - } /* How to read the following table and similar ones in this file: * "?" means we have no datasheet for this chipset generation or it doesn't have any relevant info. -- To view, visit https://review.coreboot.org/c/flashrom/+/36425 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Gerrit-Change-Number: 36425 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: merged
participants (3)
-
Arthur Heymans (Code Review) -
Edward O'Callaghan (Code Review)
-
Nico Huber (Code Review)