Change in coreboot[master]: sb/intel/bd82x6x/sata: Don't write RO register

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Felix Singer (Code Review)

6 Apr 2020 6 Apr '20

8:47 a.m.

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40229 )

Change subject: sb/intel/bd82x6x/sata: Don't write RO register ......................................................................

sb/intel/bd82x6x/sata: Don't write RO register

Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Signed-off-by: Felix Singer felix.singer@secunet.com --- M src/southbridge/intel/bd82x6x/sata.c 1 file changed, 0 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/40229/1

diff --git a/src/southbridge/intel/bd82x6x/sata.c b/src/southbridge/intel/bd82x6x/sata.c index e04f3ba..5bf6a2b 100644 --- a/src/southbridge/intel/bd82x6x/sata.c +++ b/src/southbridge/intel/bd82x6x/sata.c @@ -131,10 +131,6 @@ */ pci_write_config8(dev, 0x09, 0x8f);

- /* Set Interrupt Line */ - /* Interrupt Pin is set by D31IP.PIP */ - pci_write_config8(dev, INTR_LN, 0xff); - /* Set timings */ pci_write_config16(dev, IDE_TIM_PRI, IDE_DECODE_ENABLE | IDE_ISP_3_CLOCKS | IDE_RCT_1_CLOCKS |

Attachments:

attachment.htm (text/html — 2.5 KB)

Show replies by date

Nico Huber (Code Review)

6 Apr 6 Apr

9:24 a.m.

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

Change subject: sb/intel/bd82x6x/sata: Don't write RO register ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40229/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/40229/1//COMMIT_MSG@7 PS1, Line 7: sb/intel/bd82x6x/sata: Don't write RO register Ooops, now that I see it again, it's actually not trying to write the read-only part (interrupt pin would be ro, interrupt line is r/w)... Of course there is a(nother) reason I dropped the code:

The interrupt line registers are configured in a central place, pch_pirq_init() in `lpc.c`, according to the PIRQ configuration. Hardcoding values here makes no sense.

https://review.coreboot.org/c/coreboot/+/40229/1/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:

https://review.coreboot.org/c/coreboot/+/40229/1/src/southbridge/intel/bd82x... PS1, Line 63: pci_write_config8(dev, INTR_LN, 0x0a); Please also remove this and the definition of INTR_LN in `pch.h`.

-- To view, visit https://review.coreboot.org/c/coreboot/+/40229 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Gerrit-Change-Number: 40229 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 06 Apr 2020 09:24:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Felix Singer (Code Review)

9:49 a.m.

New subject: Change in coreboot[master]: sb/intel/bd82x6x/sata: Don't hard-code values

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40229 )

Change subject: sb/intel/bd82x6x/sata: Don't hard-code values ......................................................................

Patch Set 3:

(2 comments)

This change is ready for review.

https://review.coreboot.org/c/coreboot/+/40229/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/40229/1//COMMIT_MSG@7 PS1, Line 7: sb/intel/bd82x6x/sata: Don't write RO register

...

Ooops, now that I see it again, it's actually not trying to write the […]

Done

https://review.coreboot.org/c/coreboot/+/40229/1/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/sata.c:

https://review.coreboot.org/c/coreboot/+/40229/1/src/southbridge/intel/bd82x... PS1, Line 63: pci_write_config8(dev, INTR_LN, 0x0a);

...

Please also remove this and the definition of INTR_LN in `pch.h`.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/40229 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Gerrit-Change-Number: 40229 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 06 Apr 2020 09:49:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Felix Singer (Code Review)

9:54 a.m.

New subject: Change in coreboot[master]: sb/intel/bd82x6x/sata: Don't hard-code values

Hello build bot (Jenkins), Nico Huber, Patrick Rudolph,

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

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

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

Change subject: sb/intel/bd82x6x/sata: Don't hard-code values ......................................................................

sb/intel/bd82x6x/sata: Don't hard-code values

The interrupt line registers are configured in a central place, pch_pirq_init() in `lpc.c`, according to the PIRQ configuration. Hardcoding values here makes no sense.

Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Signed-off-by: Felix Singer felix.singer@secunet.com --- M src/southbridge/intel/bd82x6x/pch.h M src/southbridge/intel/bd82x6x/sata.c 2 files changed, 0 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/40229/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/40229 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Gerrit-Change-Number: 40229 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

11:32 a.m.

New subject: Change in coreboot[master]: sb/intel/bd82x6x/sata: Don't hard-code values

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40229 )

Change subject: sb/intel/bd82x6x/sata: Don't hard-code values ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/40229 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Gerrit-Change-Number: 40229 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 06 Apr 2020 11:32:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Menzel (Code Review)

2:27 p.m.

New subject: Change in coreboot[master]: sb/intel/bd82x6x/sata: Don't hard-code values

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

Change subject: sb/intel/bd82x6x/sata: Don't hard-code values ......................................................................

Patch Set 4: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/40229 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Gerrit-Change-Number: 40229 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 06 Apr 2020 14:27:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nico Huber (Code Review)

7 Apr 7 Apr

11:15 a.m.

New subject: Change in coreboot[master]: sb/intel/bd82x6x/sata: Don't hard-code values

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40229 )

Change subject: sb/intel/bd82x6x/sata: Don't hard-code values ......................................................................

sb/intel/bd82x6x/sata: Don't hard-code values

The interrupt line registers are configured in a central place, pch_pirq_init() in `lpc.c`, according to the PIRQ configuration. Hardcoding values here makes no sense.

Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Signed-off-by: Felix Singer felix.singer@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40229 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M src/southbridge/intel/bd82x6x/pch.h M src/southbridge/intel/bd82x6x/sata.c 2 files changed, 0 insertions(+), 9 deletions(-)

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

diff --git a/src/southbridge/intel/bd82x6x/pch.h b/src/southbridge/intel/bd82x6x/pch.h index 18383f6..0a236c6 100644 --- a/src/southbridge/intel/bd82x6x/pch.h +++ b/src/southbridge/intel/bd82x6x/pch.h @@ -167,7 +167,6 @@ #define PCH_IDE_DEV PCI_DEV(0, 0x1f, 1) #define PCH_SATA_DEV PCI_DEV(0, 0x1f, 2) #define PCH_SATA2_DEV PCI_DEV(0, 0x1f, 5) -#define INTR_LN 0x3c #define IDE_TIM_PRI 0x40 /* IDE timings, primary */ #define IDE_DECODE_ENABLE (1 << 15) #define IDE_SITRE (1 << 14) diff --git a/src/southbridge/intel/bd82x6x/sata.c b/src/southbridge/intel/bd82x6x/sata.c index e04f3ba..310d1a2 100644 --- a/src/southbridge/intel/bd82x6x/sata.c +++ b/src/southbridge/intel/bd82x6x/sata.c @@ -58,10 +58,6 @@

printk(BIOS_DEBUG, "SATA: Controller in AHCI mode.\n");

- /* Set Interrupt Line */ - /* Interrupt Pin is set by D31IP.PIP */ - pci_write_config8(dev, INTR_LN, 0x0a); - /* Set timings */ pci_write_config16(dev, IDE_TIM_PRI, IDE_DECODE_ENABLE | IDE_ISP_3_CLOCKS | IDE_RCT_1_CLOCKS | @@ -131,10 +127,6 @@ */ pci_write_config8(dev, 0x09, 0x8f);

-- To view, visit https://review.coreboot.org/c/coreboot/+/40229 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Gerrit-Change-Number: 40229 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: merged

9elements QA (Code Review)

12:01 p.m.

New subject: Change in coreboot[master]: sb/intel/bd82x6x/sata: Don't hard-code values

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

Change subject: sb/intel/bd82x6x/sata: Don't hard-code values ......................................................................

Patch Set 5:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2160 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2159 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2158

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

-- To view, visit https://review.coreboot.org/c/coreboot/+/40229 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide5f101b2e5bda84f3c2ff8c8ca636b8233bb948 Gerrit-Change-Number: 40229 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-Comment-Date: Tue, 07 Apr 2020 12:01:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1503

days inactive

1504

days old

coreboot-gerrit@coreboot.org

7 comments

5 participants

tags (0)

participants (5)

9elements QA (Code Review)
Felix Singer (Code Review)
Nico Huber (Code Review)
Patrick Rudolph (Code Review)
Paul Menzel (Code Review)