Change in coreboot[master]: mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Angel Pons (Code Review)

8 Mar 2020 8 Mar '20

6:58 p.m.

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39384 )

Change subject: mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree ......................................................................

mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree

This mainboard has the FSB BSEL straps wired to SuperIO GPIOs. They are set up in romstage, so it makes no sense to clobber the registers with garbage in ramstage.

Tested, my Asus P5G41T-M LX still boots and it does not need a full reset on almost every reboot.

Change-Id: I6ea498119df44243ec42e3cb5c2903de32a17373 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb 1 file changed, 0 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/39384/1

diff --git a/src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb b/src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb index 919d409..2ea157f 100644 --- a/src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb +++ b/src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb @@ -23,9 +23,6 @@ chip superio/winbond/w83627dhg device pnp 2e.0 off end # Floppy device pnp 2e.1 on # Parallel port - # global - irq 0x2c = 0xf2 - # parallel port io 0x60 = 0x378 irq 0x70 = 7 drq 0x74 = 3 @@ -50,8 +47,6 @@ device pnp 2e.109 off end # GPIO3 device pnp 2e.209 on # GPIO4 irq 0xe8 = 0x80 - irq 0xf4 = 0xa4 - irq 0xf5 = 0x46 end device pnp 2e.309 on # GPIO5 irq 0xfa = 0xff

Attachments:

attachment.htm (text/html — 3.3 KB)

Show replies by date

Arthur Heymans (Code Review)

8 Mar 8 Mar

7:02 p.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39384 )

Change subject: mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree ......................................................................

Patch Set 1: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39384 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ea498119df44243ec42e3cb5c2903de32a17373 Gerrit-Change-Number: 39384 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Sun, 08 Mar 2020 19:02:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nico Huber (Code Review)

7:10 p.m.

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

Change subject: mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39384/1/src/mainboard/asus/p5qpl-am... File src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/39384/1/src/mainboard/asus/p5qpl-am... PS1, Line 53: irq 0xf4 = 0xa4 This differs from the early-init value? It doesn't seem wrong wrt. GPIOs 43 & 44. Are we sure the early value doesn't enable too many outputs?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39384 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ea498119df44243ec42e3cb5c2903de32a17373 Gerrit-Change-Number: 39384 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz 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-Comment-Date: Sun, 08 Mar 2020 19:10:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

7:42 p.m.

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

Change subject: mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39384/1/src/mainboard/asus/p5qpl-am... File src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/39384/1/src/mainboard/asus/p5qpl-am... PS1, Line 53: irq 0xf4 = 0xa4

...

This differs from the early-init value? It doesn't seem wrong wrt. GPIOs […]

I fixed the early-init code last, so these values might not make much sense.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39384 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ea498119df44243ec42e3cb5c2903de32a17373 Gerrit-Change-Number: 39384 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz 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-Comment-Date: Sun, 08 Mar 2020 19:42:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Angel Pons (Code Review)

7:53 p.m.

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

Change subject: mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39384/1/src/mainboard/asus/p5qpl-am... File src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/39384/1/src/mainboard/asus/p5qpl-am... PS1, Line 53: irq 0xf4 = 0xa4

...

I fixed the early-init code last, so these values might not make much sense.

Right, vendor uses that value. I'll fix it on a follow-up

-- To view, visit https://review.coreboot.org/c/coreboot/+/39384 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ea498119df44243ec42e3cb5c2903de32a17373 Gerrit-Change-Number: 39384 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz 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-Comment-Date: Sun, 08 Mar 2020 19:53:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

7:56 p.m.

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

Change subject: mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39384/1/src/mainboard/asus/p5qpl-am... File src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/39384/1/src/mainboard/asus/p5qpl-am... PS1, Line 53: irq 0xf4 = 0xa4

...

Right, vendor uses that value. […]

CB:39386

-- To view, visit https://review.coreboot.org/c/coreboot/+/39384 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ea498119df44243ec42e3cb5c2903de32a17373 Gerrit-Change-Number: 39384 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz 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-Comment-Date: Sun, 08 Mar 2020 19:56:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Patrick Georgi (Code Review)

9 Mar 9 Mar

9:25 p.m.

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

Change subject: mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree ......................................................................

mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree

This mainboard has the FSB BSEL straps wired to SuperIO GPIOs. They are set up in romstage, so it makes no sense to clobber the registers with garbage in ramstage.

Tested, my Asus P5G41T-M LX still boots and it does not need a full reset on almost every reboot.

Change-Id: I6ea498119df44243ec42e3cb5c2903de32a17373 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39384 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/asus/p5qpl-am/variants/p5g41t-m_lx/overridetree.cb 1 file changed, 0 insertions(+), 5 deletions(-)

Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved

-- To view, visit https://review.coreboot.org/c/coreboot/+/39384 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ea498119df44243ec42e3cb5c2903de32a17373 Gerrit-Change-Number: 39384 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz 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: merged

9elements QA (Code Review)

9:36 p.m.

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

Change subject: mb/asus/p5g41t-m_lx: Do not set BSEL GPIOs in devicetree ......................................................................

Patch Set 2:

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/1209 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1208 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1207

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

-- To view, visit https://review.coreboot.org/c/coreboot/+/39384 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ea498119df44243ec42e3cb5c2903de32a17373 Gerrit-Change-Number: 39384 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz 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: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 09 Mar 2020 21:36:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1531

days inactive

1532

days old

coreboot-gerrit@coreboot.org

7 comments

5 participants

tags (0)

participants (5)

9elements QA (Code Review)
Angel Pons (Code Review)
Arthur Heymans (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)