Change in ...coreboot[master]: device/pci_rom.c: Fix indent for 'if' statement and remove unneeded '...

List overview All Threads
Download

newer

older

Change in ...coreboot[master]: test

Change in coreboot[master]:...

HAOUAS Elyes (Code Review)

9 Jun 2019 9 Jun '19

10:47 a.m.

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33330

Change subject: device/pci_rom.c: Fix indent for 'if' statement and remove unneeded 'else' ......................................................................

device/pci_rom.c: Fix indent for 'if' statement and remove unneeded 'else'

'else' is not needed after a 'break' or 'return'.

Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/device/pci_rom.c 1 file changed, 21 insertions(+), 22 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/33330/1

diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index 2dbfb51..f1f6c41 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -56,32 +56,31 @@ printk(BIOS_DEBUG, "In CBFS, ROM address for %s = %p\n", dev_path(dev), rom_header); } else if (!CONFIG(ON_DEVICE_ROM_LOAD)) { - printk(BIOS_DEBUG, "PCI Option ROM loading disabled " - "for %s\n", dev_path(dev)); + printk(BIOS_DEBUG, "PCI Option ROM loading disabled " + "for %s\n", dev_path(dev)); + return NULL; + } + uintptr_t rom_address; + + rom_address = pci_read_config32(dev, PCI_ROM_ADDRESS); + + if (rom_address == 0x00000000 || rom_address == 0xffffffff) { +#if CONFIG(BOARD_EMULATION_QEMU_X86) + if ((dev->class >> 8) == PCI_CLASS_DISPLAY_VGA) + rom_address = 0xc0000; + else +#endif return NULL; } else { - uintptr_t rom_address; - - rom_address = pci_read_config32(dev, PCI_ROM_ADDRESS); - - if (rom_address == 0x00000000 || rom_address == 0xffffffff) { -#if CONFIG(BOARD_EMULATION_QEMU_X86) - if ((dev->class >> 8) == PCI_CLASS_DISPLAY_VGA) - rom_address = 0xc0000; - else -#endif - return NULL; - } else { - /* Enable expansion ROM address decoding. */ - pci_write_config32(dev, PCI_ROM_ADDRESS, - rom_address|PCI_ROM_ADDRESS_ENABLE); - } - - printk(BIOS_DEBUG, "Option ROM address for %s = %lx\n", - dev_path(dev), (unsigned long)rom_address); - rom_header = (struct rom_header *)rom_address; + /* Enable expansion ROM address decoding. */ + pci_write_config32(dev, PCI_ROM_ADDRESS, + rom_address|PCI_ROM_ADDRESS_ENABLE); }

+ printk(BIOS_DEBUG, "Option ROM address for %s = %lx\n", + dev_path(dev), (unsigned long)rom_address); + rom_header = (struct rom_header *)rom_address; + printk(BIOS_SPEW, "PCI expansion ROM, signature 0x%04x, " "INIT size 0x%04x, data ptr 0x%04x\n", le32_to_cpu(rom_header->signature),

Attachments:

attachment.htm (text/html — 6.1 KB)

Show replies by date

Angel Pons (Code Review)

9 Jun 9 Jun

4:59 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement and remove unneeded 'else' ......................................................................

Patch Set 1: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/33330 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Gerrit-Change-Number: 33330 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sun, 09 Jun 2019 16:59:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Martin Roth (Code Review)

14 Jun 14 Jun

8:03 p.m.

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement and remove unneeded 'else' ......................................................................

Patch Set 1: Code-Review-1

(1 comment)

https://review.coreboot.org/#/c/33330/1/src/device/pci_rom.c File src/device/pci_rom.c:

https://review.coreboot.org/#/c/33330/1/src/device/pci_rom.c@58 PS1, Line 58: } else if (!CONFIG(ON_DEVICE_ROM_LOAD)) { This code flow isn't the same as before though. Before we had:

1) if (rom_header) 2) else if (!CONFIG()) 3) else then 4) more code

we would execute the code in code path 2 and return or paths 1 or 3, then always 4. Now we're executing code path 2 and returning, or executing path 1, then always 3 & 4.

1) if 2) else then 3) code and 4) code

Is that what we want?

-- To view, visit https://review.coreboot.org/c/coreboot/+/33330 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Gerrit-Change-Number: 33330 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 14 Jun 2019 20:03:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

9:20 p.m.

HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement and remove unneeded 'else' ......................................................................

Abandoned

HAOUAS Elyes (Code Review)

9:28 p.m.

HAOUAS Elyes has restored this change. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement and remove unneeded 'else' ......................................................................

Restored

HAOUAS Elyes (Code Review)

9:28 p.m.

New subject: Change in ...coreboot[master]: device/pci_rom.c: Fix indent for 'if' statement

Hello Angel Pons, build bot (Jenkins), Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33330

to look at the new patch set (#2).

Change subject: device/pci_rom.c: Fix indent for 'if' statement ......................................................................

device/pci_rom.c: Fix indent for 'if' statement

Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/device/pci_rom.c 1 file changed, 3 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/33330/2

Angel Pons (Code Review)

15 Jun 15 Jun

8:04 a.m.

New subject: Change in ...coreboot[master]: device/pci_rom.c: Fix indent for 'if' statement

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement ......................................................................

Patch Set 2: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/33330 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Gerrit-Change-Number: 33330 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 15 Jun 2019 08:04:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Jacob Garber (Code Review)

22 Jun 22 Jun

5:08 a.m.

New subject: Change in ...coreboot[master]: device/pci_rom.c: Fix indent for 'if' statement

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/33330/2/src/device/pci_rom.c File src/device/pci_rom.c:

https://review.coreboot.org/#/c/33330/2/src/device/pci_rom.c@60 PS2, Line 60: for %s\n Now that this is un-indented, we could combine this string with the above one to make it more grep-able.

-- To view, visit https://review.coreboot.org/c/coreboot/+/33330 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Gerrit-Change-Number: 33330 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jacob Garber jgarber1@ualberta.ca Gerrit-Comment-Date: Sat, 22 Jun 2019 05:08:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

5:13 a.m.

New subject: Change in ...coreboot[master]: device/pci_rom.c: Fix indent for 'if' statement

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/33330/2/src/device/pci_rom.c File src/device/pci_rom.c:

https://review.coreboot.org/#/c/33330/2/src/device/pci_rom.c@60 PS2, Line 60: for %s\n

...

Now that this is un-indented, we could combine this string with the above one to make it more grep-a […]

indeed, Thx

-- To view, visit https://review.coreboot.org/c/coreboot/+/33330 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Gerrit-Change-Number: 33330 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jacob Garber jgarber1@ualberta.ca Gerrit-Comment-Date: Sat, 22 Jun 2019 05:13:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jacob Garber jgarber1@ualberta.ca Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

5:13 a.m.

New subject: Change in ...coreboot[master]: device/pci_rom.c: Fix indent for 'if' statement

Hello Angel Pons, build bot (Jenkins), Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33330

to look at the new patch set (#3).

Change subject: device/pci_rom.c: Fix indent for 'if' statement ......................................................................

device/pci_rom.c: Fix indent for 'if' statement

Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/device/pci_rom.c 1 file changed, 3 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/33330/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/33330 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Gerrit-Change-Number: 33330 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Jacob Garber jgarber1@ualberta.ca Gerrit-MessageType: newpatchset

Jacob Garber (Code Review)

5:22 a.m.

New subject: Change in ...coreboot[master]: device/pci_rom.c: Fix indent for 'if' statement

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement ......................................................................

Patch Set 3: Code-Review+2

Let's try out my +2 powers

-- To view, visit https://review.coreboot.org/c/coreboot/+/33330 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Gerrit-Change-Number: 33330 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 22 Jun 2019 05:22:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Menzel (Code Review)

5 Jul 5 Jul

10:09 a.m.

New subject: Change in coreboot[master]: device/pci_rom.c: Fix indent for 'if' statement

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement ......................................................................

Patch Set 4: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/33330 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Gerrit-Change-Number: 33330 Gerrit-PatchSet: 4 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 05 Jul 2019 10:09:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

10:18 a.m.

New subject: Change in coreboot[master]: device/pci_rom.c: Fix indent for 'if' statement

Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33330 )

Change subject: device/pci_rom.c: Fix indent for 'if' statement ......................................................................

device/pci_rom.c: Fix indent for 'if' statement

Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/coreboot/+/33330 Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Jacob Garber jgarber1@ualberta.ca Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/device/pci_rom.c 1 file changed, 3 insertions(+), 3 deletions(-)

Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Jacob Garber: Looks good to me, approved

diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index 31fe115..34a9a81 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -56,9 +56,9 @@ printk(BIOS_DEBUG, "In CBFS, ROM address for %s = %p\n", dev_path(dev), rom_header); } else if (!CONFIG(ON_DEVICE_ROM_LOAD)) { - printk(BIOS_DEBUG, "PCI Option ROM loading disabled " - "for %s\n", dev_path(dev)); - return NULL; + printk(BIOS_DEBUG, "PCI Option ROM loading disabled for %s\n", + dev_path(dev)); + return NULL; } else { uintptr_t rom_address;

-- To view, visit https://review.coreboot.org/c/coreboot/+/33330 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie9adb60323742d379cc4ad0af069a793b9ddd79b Gerrit-Change-Number: 33330 Gerrit-PatchSet: 5 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: merged

1782

days inactive

1808

days old

coreboot-gerrit@coreboot.org

12 comments

6 participants

tags (0)

participants (6)

Angel Pons (Code Review)
HAOUAS Elyes (Code Review)
Jacob Garber (Code Review)
Kyösti Mälkki (Code Review)
Martin Roth (Code Review)
Paul Menzel (Code Review)