Change in coreboot[master]: mb/asus/p2b: list all unused Super I/O resources

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Keith Hui (Code Review)

16 May 2020 16 May '20

3:04 a.m.

Keith Hui has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

mb/asus/p2b: list all unused Super I/O resources

Some Super I/O resources were unused and not listed, causing warnings during resource allocation. Suppress these warnings by setting them to zero.

Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Signed-off-by: Keith Hui buurin@gmail.com --- M src/mainboard/asus/p2b/devicetree.cb 1 file changed, 4 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/41459/1

diff --git a/src/mainboard/asus/p2b/devicetree.cb b/src/mainboard/asus/p2b/devicetree.cb index 9f7f63e..7ee69e4 100644 --- a/src/mainboard/asus/p2b/devicetree.cb +++ b/src/mainboard/asus/p2b/devicetree.cb @@ -18,6 +18,7 @@ device pnp 3f0.1 on # Parallel port io 0x60 = 0x378 irq 0x70 = 7 + drq 0x74 = 0 end device pnp 3f0.2 on # COM1 io 0x60 = 0x3f8 @@ -34,6 +35,9 @@ irq 0x72 = 12 # PS/2 mouse interrupt end device pnp 3f0.7 on # GPIO 1 + io 0x60 = 0 + io 0x62 = 0 + irq 0x70 = 0 end device pnp 3f0.8 on # GPIO 2 end

Attachments:

attachment.htm (text/html — 3.0 KB)

Show replies by date

Patrick Rudolph (Code Review)

16 May 16 May

3:54 a.m.

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

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41459/1/src/mainboard/asus/p2b/devi... File src/mainboard/asus/p2b/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/41459/1/src/mainboard/asus/p2b/devi... PS1, Line 38: = If you don't need LDN7 please disable it.

Angel Pons (Code Review)

17 May 17 May

2:26 p.m.

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

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 1: Code-Review+1

Angel Pons (Code Review)

3 Aug 3 Aug

9:42 a.m.

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

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 3: Code-Review+2

Arthur Heymans (Code Review)

21 Mar 21 Mar

8:29 a.m.

Attention is currently required from: Keith Hui. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 7: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/41459 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Gerrit-Change-Number: 41459 Gerrit-PatchSet: 7 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Rudolph siro@das-labor.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Attention: Keith Hui buurin@gmail.com Gerrit-Comment-Date: Sun, 21 Mar 2021 08:29:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Keith Hui (Code Review)

27 Dec 27 Dec

4:56 a.m.

Attention is currently required from: Patrick Rudolph. Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 7:

(1 comment)

File src/mainboard/asus/p2b/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/41459/comment/1d8a91e9_aa77fb95 PS1, Line 38: =

...

If you don't need LDN7 please disable it.

I just double checked against vendor firmware. It does have LDN7 enabled, and LDN8 too - the latter contains a config register related to power LED that needs to be programmed.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41459 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Gerrit-Change-Number: 41459 Gerrit-PatchSet: 7 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Rudolph siro@das-labor.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Patrick Rudolph siro@das-labor.org Gerrit-Comment-Date: Mon, 27 Dec 2021 04:56:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph siro@das-labor.org Gerrit-MessageType: comment

Paul Menzel (Code Review)

9:06 a.m.

Attention is currently required from: Nico Huber, Keith Hui, Patrick Rudolph. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 7: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/41459 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Gerrit-Change-Number: 41459 Gerrit-PatchSet: 7 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Rudolph siro@das-labor.org Gerrit-Attention: Nico Huber nico.h@gmx.de Gerrit-Attention: Keith Hui buurin@gmail.com Gerrit-Attention: Patrick Rudolph siro@das-labor.org Gerrit-Comment-Date: Mon, 27 Dec 2021 09:06:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Felix Held (Code Review)

4 Jan 4 Jan

11:47 a.m.

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

mb/asus/p2b: list all unused Super I/O resources

Some Super I/O resources were unused and not listed, causing warnings during resource allocation. Suppress these warnings by setting them to zero.

Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Signed-off-by: Keith Hui buurin@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41459 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Paul Menzel paulepanter@mailbox.org Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/asus/p2b/devicetree.cb 1 file changed, 4 insertions(+), 0 deletions(-)

Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Arthur Heymans: Looks good to me, approved Angel Pons: Looks good to me, approved

Nico Huber (Code Review)

14 Jan 14 Jan

11:26 p.m.

Attention is currently required from: Keith Hui. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 8:

(1 comment)

File src/mainboard/asus/p2b/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/41459/comment/4332a2ee_135f5060 PS8, Line 38: io 0x60 = 0 : io 0x62 = 0 Does the code treat the 0 specially? If not this is conflicting 2x port 0.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41459 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Gerrit-Change-Number: 41459 Gerrit-PatchSet: 8 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Patrick Rudolph siro@das-labor.org Gerrit-Attention: Keith Hui buurin@gmail.com Gerrit-Comment-Date: Fri, 14 Jan 2022 23:26:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

17 Jan 17 Jan

3:45 p.m.

Attention is currently required from: Keith Hui. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 8:

(1 comment)

File src/mainboard/asus/p2b/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/41459/comment/0b8e4d58_4a4ea86b PS8, Line 38: io 0x60 = 0 : io 0x62 = 0

...

Does the code treat the 0 specially? If not this is conflicting 2x port 0.

I think it's just to silence the warnings about "resource not set". If 0 is treated specially, it's most likely done by the hardware.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41459 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Gerrit-Change-Number: 41459 Gerrit-PatchSet: 8 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Patrick Rudolph siro@das-labor.org Gerrit-Attention: Keith Hui buurin@gmail.com Gerrit-Comment-Date: Mon, 17 Jan 2022 15:45:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Nico Huber (Code Review)

23 Jan 23 Jan

6:28 p.m.

Attention is currently required from: Keith Hui. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 8:

(1 comment)

File src/mainboard/asus/p2b/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/41459/comment/3bb629b7_dcbaad6a PS8, Line 38: io 0x60 = 0 : io 0x62 = 0

...

I think it's just to silence the warnings about "resource not set". […]

It just sounds like something that happens to work because our infrastructure is not pedantic about it. But future changes to the allocator, for instance, could break it.

I can think of two solutions: a) add syntax to disable a resource; the chip driver would have to write an explicit 0 to the hardware then. b) the chip driver would tag a resource as 0-is-special or 0-is-off. Hmmm, or maybe a third, but I don't like it (just a bad feeling), c) we always treat 0 as disabled.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41459 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Gerrit-Change-Number: 41459 Gerrit-PatchSet: 8 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Patrick Rudolph siro@das-labor.org Gerrit-Attention: Keith Hui buurin@gmail.com Gerrit-Comment-Date: Sun, 23 Jan 2022 18:28:10 +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

Keith Hui (Code Review)

10:27 p.m.

Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 8:

(1 comment)

File src/mainboard/asus/p2b/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/41459/comment/e3cbebfe_fc7ef156 PS8, Line 38: io 0x60 = 0 : io 0x62 = 0

...

It just sounds like something that happens to work because our infrastructure […]

We already have syntax to disable that resource:

device pnp 3f0.7 off

However, on this board, when on OEM firmware, this device is enabled with an unassigned port (registers 0x30=1, 0x60=0, 0x62=0), hence the zero. The I/O port setting here is meant to provide a port where the chip's GPIO pins can be accessed directly, but the GPIO1x pins are completely unused as far as I can see, yet I'm unsure if disabling outright would break anything. It's therefore so configured to match OEM.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41459 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Gerrit-Change-Number: 41459 Gerrit-PatchSet: 8 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Patrick Rudolph siro@das-labor.org Gerrit-Comment-Date: Sun, 23 Jan 2022 22:27:04 +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

Nico Huber (Code Review)

11:31 p.m.

Attention is currently required from: Keith Hui. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 8:

(1 comment)

File src/mainboard/asus/p2b/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/41459/comment/f870c41d_f799e01e PS8, Line 38: io 0x60 = 0 : io 0x62 = 0

...

We already have syntax to disable that resource: […]

Understood (actually I knew that already). The problem is not what this code does, that's clear. It's that it is fragile (basically going a short cut that exists only by coincidence). Currently the framework code allows you to set two resources to the same value, but that could change. I just wanted to get the warning out.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41459 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Gerrit-Change-Number: 41459 Gerrit-PatchSet: 8 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Patrick Rudolph siro@das-labor.org Gerrit-Attention: Keith Hui buurin@gmail.com Gerrit-Comment-Date: Sun, 23 Jan 2022 23:31:35 +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 Comment-In-Reply-To: Keith Hui buurin@gmail.com Gerrit-MessageType: comment

Keith Hui (Code Review)

3 Feb 3 Feb

11 p.m.

Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41459 )

Change subject: mb/asus/p2b: list all unused Super I/O resources ......................................................................

Patch Set 8:

(1 comment)

File src/mainboard/asus/p2b/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/41459/comment/accde2f5_21cf20d2 PS8, Line 38: io 0x60 = 0 : io 0x62 = 0

...

Understood (actually I knew that already). The problem is not what this […]

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/41459 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28e37c3a58f3a6b5a613733f26ac18d6a7b3be2e Gerrit-Change-Number: 41459 Gerrit-PatchSet: 8 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Patrick Rudolph siro@das-labor.org Gerrit-Comment-Date: Thu, 03 Feb 2022 23:00:28 +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 Comment-In-Reply-To: Keith Hui buurin@gmail.com Gerrit-MessageType: comment

1029

days inactive

1657

days old

coreboot-gerrit@coreboot.org

13 comments

7 participants

tags (0)

participants (7)

Angel Pons (Code Review)
Arthur Heymans (Code Review)
Felix Held (Code Review)
Keith Hui (Code Review)
Nico Huber (Code Review)
Patrick Rudolph (Code Review)
Paul Menzel (Code Review)