Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup
Coalescing is not needed, as all PCIe ports are used. Also, update the comments to look more like the other two variants.
Change-Id: I3f4bf99ceec6c77f6e1eabea9b712245afee7d34 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb 1 file changed, 8 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/39742/1
diff --git a/src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb b/src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb index 08abc3e..b76200b 100644 --- a/src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb +++ b/src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb @@ -34,7 +34,6 @@ chip southbridge/intel/bd82x6x # Intel Series 6 Cougar Point PCH register "c2_latency" = "0x0065" register "gen1_dec" = "0x003c0a01" - register "pcie_port_coalesce" = "1" register "sata_interface_speed_support" = "0x3" register "sata_port_map" = "0x33" register "spi_lvscc" = "0x2005" @@ -46,14 +45,14 @@ device pci 19.0 off end # Intel Gigabit Ethernet device pci 1a.0 on end # USB2 EHCI #2 device pci 1b.0 on end # High Definition Audio Audio controller - device pci 1c.0 on end # PCIe x1 Port (PCIEX1) - device pci 1c.1 off end # Unused PCIe Port - device pci 1c.2 off end # Unused PCIe Port - device pci 1c.3 off end # Unused PCIe Port - device pci 1c.4 on end # Realtek RTL8111F Ethernet Controller - device pci 1c.5 on end # ITE IT8892F PCIe to PCI bridge - device pci 1c.6 off end # Unused PCIe Port - device pci 1c.7 off end # Unused PCIe Port + + device pci 1c.0 on end # RP #1: PCIe x1 Port (PCIEX1) + device pci 1c.1 off end # RP #2: + device pci 1c.2 off end # RP #3: + device pci 1c.3 off end # RP #4: + device pci 1c.4 on end # RP #5: Realtek RTL8111F GbE NIC + device pci 1c.5 on end # RP #6: ITE IT8892F PCIe to PCI bridge + device pci 1d.0 on end # USB2 EHCI #1 device pci 1e.0 off end # PCI bridge device pci 1f.0 on # LPC bridge
Angel Pons has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup
Coalescing is not needed, as all PCIe ports are used. Also, update the comments to look more like the other two variants.
Change-Id: I3f4bf99ceec6c77f6e1eabea9b712245afee7d34 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb 1 file changed, 8 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/39742/2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... File src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... PS2, Line 50: device pci 1c.1 off end # RP #2: so if this isn't unused, why is it off?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... File src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... PS2, Line 50: device pci 1c.1 off end # RP #2:
so if this isn't unused, why is it off?
It actually is unused, though? This board, unlike the other two variants, only has one PCIe x1 slot.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... File src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... PS2, Line 50: device pci 1c.1 off end # RP #2:
It actually is unused, though? This board, unlike the other two variants, only has one PCIe x1 slot.
Any updates?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... File src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... PS2, Line 50: device pci 1c.1 off end # RP #2:
Any updates?
Hint: It completely disagrees with the commit message. The commit message is the first thing reviewers see. If it's wrong, the review can get annoying.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... File src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... PS2, Line 50: device pci 1c.1 off end # RP #2:
Hint: It completely disagrees with the commit message. The commit message […]
Um, only now I realized that the commit message was wrong...
Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Arthur Heymans,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39742
to look at the new patch set (#3).
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup
Coalescing is not needed, as the root port #0 is enabled. Also, update the comments to look more like the other two variants.
Change-Id: I3f4bf99ceec6c77f6e1eabea9b712245afee7d34 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb 1 file changed, 8 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/39742/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... File src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... PS2, Line 50: device pci 1c.1 off end # RP #2:
Um, only now I realized that the commit message was wrong...
Shouldn't be a problem now, I guess...
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
Patch Set 3: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/39742/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39742/3//COMMIT_MSG@9 PS3, Line 9: 0 ports are counted from 1
https://review.coreboot.org/c/coreboot/+/39742/3//COMMIT_MSG@10 PS3, Line 10: the comments to look more like the other two variants. Please mention that H61 doesn't have 1c.6/7.
Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Arthur Heymans,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39742
to look at the new patch set (#4).
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup
Coalescing is not needed, as root port #1 is enabled. Also, update the comments to look more like the other two variants. Note that the Intel H61 PCH only has six root ports, so devices 1c.6 and 1c.7 do not exist.
Change-Id: I3f4bf99ceec6c77f6e1eabea9b712245afee7d34 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb 1 file changed, 8 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/39742/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39742/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39742/3//COMMIT_MSG@9 PS3, Line 9: 0
ports are counted from 1
oops
https://review.coreboot.org/c/coreboot/+/39742/3//COMMIT_MSG@10 PS3, Line 10: the comments to look more like the other two variants.
Please mention that H61 doesn't have 1c.6/7.
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... File src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39742/2/src/mainboard/gigabyte/ga-h... PS2, Line 50: device pci 1c.1 off end # RP #2:
Shouldn't be a problem now, I guess...
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39742 )
Change subject: mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup ......................................................................
mb/gigabyte/ga-h61m-s2pv: Correct PCIe port setup
Coalescing is not needed, as root port #1 is enabled. Also, update the comments to look more like the other two variants. Note that the Intel H61 PCH only has six root ports, so devices 1c.6 and 1c.7 do not exist.
Change-Id: I3f4bf99ceec6c77f6e1eabea9b712245afee7d34 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39742 Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb 1 file changed, 8 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb b/src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb index 08abc3e..b76200b 100644 --- a/src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb +++ b/src/mainboard/gigabyte/ga-h61m-s2pv/variants/ga-h61m-s2pv/devicetree.cb @@ -34,7 +34,6 @@ chip southbridge/intel/bd82x6x # Intel Series 6 Cougar Point PCH register "c2_latency" = "0x0065" register "gen1_dec" = "0x003c0a01" - register "pcie_port_coalesce" = "1" register "sata_interface_speed_support" = "0x3" register "sata_port_map" = "0x33" register "spi_lvscc" = "0x2005" @@ -46,14 +45,14 @@ device pci 19.0 off end # Intel Gigabit Ethernet device pci 1a.0 on end # USB2 EHCI #2 device pci 1b.0 on end # High Definition Audio Audio controller - device pci 1c.0 on end # PCIe x1 Port (PCIEX1) - device pci 1c.1 off end # Unused PCIe Port - device pci 1c.2 off end # Unused PCIe Port - device pci 1c.3 off end # Unused PCIe Port - device pci 1c.4 on end # Realtek RTL8111F Ethernet Controller - device pci 1c.5 on end # ITE IT8892F PCIe to PCI bridge - device pci 1c.6 off end # Unused PCIe Port - device pci 1c.7 off end # Unused PCIe Port + + device pci 1c.0 on end # RP #1: PCIe x1 Port (PCIEX1) + device pci 1c.1 off end # RP #2: + device pci 1c.2 off end # RP #3: + device pci 1c.3 off end # RP #4: + device pci 1c.4 on end # RP #5: Realtek RTL8111F GbE NIC + device pci 1c.5 on end # RP #6: ITE IT8892F PCIe to PCI bridge + device pci 1d.0 on end # USB2 EHCI #1 device pci 1e.0 off end # PCI bridge device pci 1f.0 on # LPC bridge
-
Angel Pons (Code Review) -
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)