Change in coreboot[master]: sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Change in coreboot[master]: [WIP]...

HAOUAS Elyes (Code Review)

28 Apr 2020 28 Apr '20

5:29 p.m.

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

Change subject: sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register ......................................................................

sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register

Change-Id: I779387fb0c9d3ad6e16d4ccfc39c38dfe6620345 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/amd/cimx/sb800/late.c 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/40806/1

diff --git a/src/southbridge/amd/cimx/sb800/late.c b/src/southbridge/amd/cimx/sb800/late.c index e4a1795..0993d2d 100644 --- a/src/southbridge/amd/cimx/sb800/late.c +++ b/src/southbridge/amd/cimx/sb800/late.c @@ -99,7 +99,7 @@ }

dev->command |= PCI_COMMAND_MASTER; - pci_write_config8(dev, PCI_COMMAND, dev->command); + pci_write_config16(dev, PCI_COMMAND, dev->command); printk(BIOS_DEBUG, "AHCI/RAID controller initialized\n"); }

Attachments:

attachment.htm (text/html — 2.4 KB)

Show replies by date

Nico Huber (Code Review)

4 Jun 4 Jun

9:28 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40806 )

Change subject: sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register ......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/40806/1/src/southbridge/amd/cimx/sb... File src/southbridge/amd/cimx/sb800/late.c:

https://review.coreboot.org/c/coreboot/+/40806/1/src/southbridge/amd/cimx/sb... PS1, Line 102: pci_write_config16(dev, PCI_COMMAND, dev->command); I hope you realize how hard it is to review if this doesn't change anything.

-- To view, visit https://review.coreboot.org/c/coreboot/+/40806 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I779387fb0c9d3ad6e16d4ccfc39c38dfe6620345 Gerrit-Change-Number: 40806 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 04 Jun 2020 21:28:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

5 Jun 5 Jun

6:52 a.m.

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

Change subject: sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40806/1/src/southbridge/amd/cimx/sb... File src/southbridge/amd/cimx/sb800/late.c:

https://review.coreboot.org/c/coreboot/+/40806/1/src/southbridge/amd/cimx/sb... PS1, Line 102: pci_write_config16(dev, PCI_COMMAND, dev->command);

...

I hope you realize how hard it is to review if this doesn't change anything.

Thank you so much. Here, it does. Old code do only a partial write of 16bit into 8.

-- To view, visit https://review.coreboot.org/c/coreboot/+/40806 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I779387fb0c9d3ad6e16d4ccfc39c38dfe6620345 Gerrit-Change-Number: 40806 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 05 Jun 2020 06:52:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Patrick Georgi (Code Review)

6 Jun 6 Jun

9:36 a.m.

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40806 )

Change subject: sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register ......................................................................

sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register

Change-Id: I779387fb0c9d3ad6e16d4ccfc39c38dfe6620345 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/coreboot/+/40806 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/southbridge/amd/cimx/sb800/late.c 1 file changed, 1 insertion(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved

diff --git a/src/southbridge/amd/cimx/sb800/late.c b/src/southbridge/amd/cimx/sb800/late.c index 1cf3ae8..de91a9a 100644 --- a/src/southbridge/amd/cimx/sb800/late.c +++ b/src/southbridge/amd/cimx/sb800/late.c @@ -98,7 +98,7 @@ }

Nico Huber (Code Review)

7 Jun 7 Jun

3:54 p.m.

Nico Huber has created a revert of this change. ( https://review.coreboot.org/c/coreboot/+/40806 )

Change subject: sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register ......................................................................

-- To view, visit https://review.coreboot.org/c/coreboot/+/40806 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I779387fb0c9d3ad6e16d4ccfc39c38dfe6620345 Gerrit-Change-Number: 40806 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr 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: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: revert

Angel Pons (Code Review)

3:55 p.m.

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

Change subject: sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40806/1/src/southbridge/amd/cimx/sb... File src/southbridge/amd/cimx/sb800/late.c:

https://review.coreboot.org/c/coreboot/+/40806/1/src/southbridge/amd/cimx/sb... PS1, Line 102: pci_write_config16(dev, PCI_COMMAND, dev->command);

...

Thank you so much. […]

As per some IRC talk, looks like dev->command is only 8 bits wide. So this is wrong.

-- To view, visit https://review.coreboot.org/c/coreboot/+/40806 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I779387fb0c9d3ad6e16d4ccfc39c38dfe6620345 Gerrit-Change-Number: 40806 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr 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: Angel Pons th3fanbus@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 07 Jun 2020 15:55:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Gerrit-MessageType: comment

9elements QA (Code Review)

5:38 p.m.

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40806 )

Change subject: sb/amd/cimx/sb800: Fix 16-bit read/write PCI_COMMAND register ......................................................................

Patch Set 2:

Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/5064 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/5063 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/5062 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/5061

Please note: This test is under development and might not be accurate at all!

-- To view, visit https://review.coreboot.org/c/coreboot/+/40806 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I779387fb0c9d3ad6e16d4ccfc39c38dfe6620345 Gerrit-Change-Number: 40806 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes ehaouas@noos.fr 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: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 07 Jun 2020 17:38:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1425

days inactive

1465

days old

coreboot-gerrit@coreboot.org

6 comments

5 participants

tags (0)

participants (5)

9elements QA (Code Review)
Angel Pons (Code Review)
HAOUAS Elyes (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)