Change in flashrom[master]: sb600spi.c: Consolidate smbus dev reversion derivation
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... sb600spi.c: Consolidate smbus dev reversion derivation Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 17 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/36420/1 diff --git a/sb600spi.c b/sb600spi.c index 3026836..c89d5a9 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -80,6 +80,17 @@ .write_aai = default_spi_write_aai, }; +static int find_smbus_dev(uint16_t vendor, uint16_t device) +{ + struct pci_dev *smbus_dev = pci_dev_find(vendor, device); + if (!smbus_dev) { + msg_pdbg("No SMBus device with ID %04X:%04X found.\n", vendor, device); + msg_perr("ERROR: SMBus device not found. Not enabling SPI.\n"); + return -1; + } + return pci_read_byte(smbus_dev, PCI_REVISION_ID); +} + static void determine_generation(struct pci_dev *dev) { amd_gen = CHIPSET_AMD_UNKNOWN; @@ -88,10 +99,9 @@ amd_gen = CHIPSET_SB6XX; msg_pdbg("SB6xx detected.\n"); } else if (dev->device_id == 0x439d) { - struct pci_dev *smbus_dev = pci_dev_find(0x1002, 0x4385); - if (smbus_dev == NULL) + int rev = find_smbus_dev(0x1002, 0x4385); + if (rev < 0) return; - uint8_t rev = pci_read_byte(smbus_dev, PCI_REVISION_ID); if (rev >= 0x39 && rev <= 0x3D) { amd_gen = CHIPSET_SB7XX; msg_pdbg("SB7xx/SP5100 detected.\n"); @@ -123,12 +133,9 @@ msg_pdbg("not found. Assuming Hudson.\n"); amd_gen = CHIPSET_HUDSON234; #else - struct pci_dev *smbus_dev = pci_dev_find(0x1022, 0x780B); - if (smbus_dev == NULL) { - msg_pdbg("No SMBus device with ID 1022:780B found.\n"); + int rev = find_smbus_dev(0x1022, 0x780B); + if (rev < 0) return; - } - uint8_t rev = pci_read_byte(smbus_dev, PCI_REVISION_ID); if (rev >= 0x11 && rev <= 0x15) { amd_gen = CHIPSET_HUDSON234; msg_pdbg("Hudson-2/3/4 detected.\n"); @@ -144,12 +151,9 @@ "the output of lspci -nnvx, thanks!.\n", rev); } } else if (dev->device_id == 0x790e) { - struct pci_dev *smbus_dev = pci_dev_find(0x1022, 0x790B); - if (smbus_dev == NULL) { - msg_pdbg("No SMBus device with ID 1022:790B found.\n"); + int rev = find_smbus_dev(0x1022, 0x790B); + if (rev < 0) return; - } - uint8_t rev = pci_read_byte(smbus_dev, PCI_REVISION_ID); if (rev == 0x4a) { amd_gen = CHIPSET_YANGTZE; msg_pdbg("Yangtze detected.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 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/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/36420/1/sb600spi.c File sb600spi.c: https://review.coreboot.org/c/flashrom/+/36420/1/sb600spi.c@83 PS1, Line 83: find_smbus_dev call it smbus_rev? -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Tue, 29 Oct 2019 07:52:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/36420/1/sb600spi.c File sb600spi.c: https://review.coreboot.org/c/flashrom/+/36420/1/sb600spi.c@83 PS1, Line 83: find_smbus_dev
call it smbus_rev? find_smbus_dev_rev() ?
-- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Tue, 29 Oct 2019 08:20:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/36420/1/sb600spi.c File sb600spi.c: https://review.coreboot.org/c/flashrom/+/36420/1/sb600spi.c@83 PS1, Line 83: find_smbus_dev
find_smbus_dev_rev() ?
sounds good. -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Tue, 29 Oct 2019 09:36:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: comment
Hello build bot (Jenkins), Nico Huber, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/36420 to look at the new patch set (#2). Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... sb600spi.c: Consolidate smbus dev reversion derivation V.2: Rename 'find_smbus_dev()' -> 'find_smbus_dev_rev()'. Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 17 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/36420/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: newpatchset
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/flashrom/+/36420/1/sb600spi.c File sb600spi.c: https://review.coreboot.org/c/flashrom/+/36420/1/sb600spi.c@83 PS1, Line 83: find_smbus_dev
find_smbus_dev_rev() ? […] Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Tue, 29 Oct 2019 11:22:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: comment
Hello build bot (Jenkins), Nico Huber, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/36420 to look at the new patch set (#3). Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... sb600spi.c: Consolidate smbus dev reversion derivation V.2: Rename 'find_smbus_dev()' -> 'find_smbus_dev_rev()'. Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 17 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/36420/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: newpatchset
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/c/flashrom/+/36420/3/sb600spi.c File sb600spi.c: https://review.coreboot.org/c/flashrom/+/36420/3/sb600spi.c@103 PS3, Line 103: if (rev < 0) : return; That's the result from the u8 returned by pci_read_byte. When is this supposed to trigger? -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: Patrick Georgi <pgeorgi@google.com> Gerrit-Comment-Date: Wed, 06 Nov 2019 14:27:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/c/flashrom/+/36420/3/sb600spi.c File sb600spi.c: https://review.coreboot.org/c/flashrom/+/36420/3/sb600spi.c@103 PS3, Line 103: if (rev < 0) : return;
That's the result from the u8 returned by pci_read_byte. […] I think this is actually checking for the `if (!smbus_dev) {` branch taken and thus the returned -1 not the fall though as this is checking strictly less than zero.
-- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-CC: Patrick Georgi <pgeorgi@google.com> Gerrit-Comment-Date: Thu, 07 Nov 2019 08:43:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <pgeorgi@google.com> Gerrit-MessageType: comment
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... Patch Set 3: Code-Review+2 (1 comment) https://review.coreboot.org/c/flashrom/+/36420/3/sb600spi.c File sb600spi.c: https://review.coreboot.org/c/flashrom/+/36420/3/sb600spi.c@103 PS3, Line 103: if (rev < 0) : return;
I think this is actually checking for the `if (!smbus_dev) {` branch taken and thus the returned -1 […] Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Thu, 14 Nov 2019 12:14:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Patrick Georgi <pgeorgi@google.com> Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev reversion derivation ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Might want to fix the commit message before merge. https://review.coreboot.org/c/flashrom/+/36420/3//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/36420/3//COMMIT_MSG@7 PS3, Line 7: reversion revision? -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Thu, 14 Nov 2019 21:16:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/36420 to look at the new patch set (#4). Change subject: sb600spi.c: Consolidate smbus dev revision derivation ...................................................................... sb600spi.c: Consolidate smbus dev revision derivation V.2: Rename 'find_smbus_dev()' -> 'find_smbus_dev_rev()'. Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> --- M sb600spi.c 1 file changed, 17 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/36420/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: newpatchset
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev revision derivation ...................................................................... Patch Set 4: Code-Review+2 Carry Nico's +2 forwards. -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Thu, 14 Nov 2019 21:31:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev revision derivation ...................................................................... Patch Set 4: Verified+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Thu, 14 Nov 2019 22:41:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/36420 ) Change subject: sb600spi.c: Consolidate smbus dev revision derivation ...................................................................... sb600spi.c: Consolidate smbus dev revision derivation V.2: Rename 'find_smbus_dev()' -> 'find_smbus_dev_rev()'. Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Signed-off-by: Edward O'Callaghan <quasisec@chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/36420 Reviewed-by: Patrick Georgi <pgeorgi@google.com> Reviewed-by: Nico Huber <nico.h@gmx.de> Tested-by: Nico Huber <nico.h@gmx.de> --- M sb600spi.c 1 file changed, 17 insertions(+), 13 deletions(-) Approvals: Nico Huber: Verified; Looks good to me, approved Patrick Georgi: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/sb600spi.c b/sb600spi.c index 3026836..6466795 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -80,6 +80,17 @@ .write_aai = default_spi_write_aai, }; +static int find_smbus_dev_rev(uint16_t vendor, uint16_t device) +{ + struct pci_dev *smbus_dev = pci_dev_find(vendor, device); + if (!smbus_dev) { + msg_pdbg("No SMBus device with ID %04X:%04X found.\n", vendor, device); + msg_perr("ERROR: SMBus device not found. Not enabling SPI.\n"); + return -1; + } + return pci_read_byte(smbus_dev, PCI_REVISION_ID); +} + static void determine_generation(struct pci_dev *dev) { amd_gen = CHIPSET_AMD_UNKNOWN; @@ -88,10 +99,9 @@ amd_gen = CHIPSET_SB6XX; msg_pdbg("SB6xx detected.\n"); } else if (dev->device_id == 0x439d) { - struct pci_dev *smbus_dev = pci_dev_find(0x1002, 0x4385); - if (smbus_dev == NULL) + int rev = find_smbus_dev_rev(0x1002, 0x4385); + if (rev < 0) return; - uint8_t rev = pci_read_byte(smbus_dev, PCI_REVISION_ID); if (rev >= 0x39 && rev <= 0x3D) { amd_gen = CHIPSET_SB7XX; msg_pdbg("SB7xx/SP5100 detected.\n"); @@ -123,12 +133,9 @@ msg_pdbg("not found. Assuming Hudson.\n"); amd_gen = CHIPSET_HUDSON234; #else - struct pci_dev *smbus_dev = pci_dev_find(0x1022, 0x780B); - if (smbus_dev == NULL) { - msg_pdbg("No SMBus device with ID 1022:780B found.\n"); + int rev = find_smbus_dev_rev(0x1022, 0x780B); + if (rev < 0) return; - } - uint8_t rev = pci_read_byte(smbus_dev, PCI_REVISION_ID); if (rev >= 0x11 && rev <= 0x15) { amd_gen = CHIPSET_HUDSON234; msg_pdbg("Hudson-2/3/4 detected.\n"); @@ -144,12 +151,9 @@ "the output of lspci -nnvx, thanks!.\n", rev); } } else if (dev->device_id == 0x790e) { - struct pci_dev *smbus_dev = pci_dev_find(0x1022, 0x790B); - if (smbus_dev == NULL) { - msg_pdbg("No SMBus device with ID 1022:790B found.\n"); + int rev = find_smbus_dev_rev(0x1022, 0x790B); + if (rev < 0) return; - } - uint8_t rev = pci_read_byte(smbus_dev, PCI_REVISION_ID); if (rev == 0x4a) { amd_gen = CHIPSET_YANGTZE; msg_pdbg("Yangtze detected.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/36420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I766b29cc1c7d01aa0bcf6cb9ff5ab73fa1995dcd Gerrit-Change-Number: 36420 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: merged
participants (4)
-
Arthur Heymans (Code Review) -
Edward O'Callaghan (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)