Change in coreboot[master]: superio/aspeed/ast2400: Add eSPI auto detection

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Christian Walter (Code Review)

26 Jan 2020 26 Jan '20

11:20 p.m.

Christian Walter has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38586 )

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

superio/aspeed/ast2400: Add eSPI auto detection

Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/superio/aspeed/ast2400/superio.c 1 file changed, 11 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/38586/1

diff --git a/src/superio/aspeed/ast2400/superio.c b/src/superio/aspeed/ast2400/superio.c index 6f2cbcd..556dea9 100644 --- a/src/superio/aspeed/ast2400/superio.c +++ b/src/superio/aspeed/ast2400/superio.c @@ -31,7 +31,17 @@ if (!dev->enabled) return;

- if (conf && conf->use_espi) { + + // Detect eSPI Mode automatically + pnp_enter_conf_mode(dev); + pnp_set_logical_device(dev); + u8 val = pnp_read_config(dev, 0x70) & 0x07; + pnp_exit_conf_mode(dev); + + if (val == 0xC) { + if (conf) + conf->use_espi = 1; + pnp_enter_conf_mode(dev); pnp_set_logical_device(dev); /* In ESPI mode must write 0 to IRQ level on every LDN */

Attachments:

attachment.htm (text/html — 3.1 KB)

Show replies by date

build bot (Jenkins) (Code Review)

26 Jan 26 Jan

11:22 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38586 )

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38586/1/src/superio/aspeed/ast2400/... File src/superio/aspeed/ast2400/superio.c:

https://review.coreboot.org/c/coreboot/+/38586/1/src/superio/aspeed/ast2400/... PS1, Line 43: conf->use_espi = 1; code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/38586/1/src/superio/aspeed/ast2400/... PS1, Line 43: conf->use_espi = 1; please, no spaces at the start of a line

-- To view, visit https://review.coreboot.org/c/coreboot/+/38586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Gerrit-Change-Number: 38586 Gerrit-PatchSet: 1 Gerrit-Owner: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-CC: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sun, 26 Jan 2020 23:22:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Christian Walter (Code Review)

11:23 p.m.

Hello Felix Held,

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

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

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

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

superio/aspeed/ast2400: Add eSPI auto detection

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/38586/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/38586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Gerrit-Change-Number: 38586 Gerrit-PatchSet: 2 Gerrit-Owner: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-CC: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Felix Held (Code Review)

11:52 p.m.

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

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38586/2/src/superio/aspeed/ast2400/... File src/superio/aspeed/ast2400/superio.c:

https://review.coreboot.org/c/coreboot/+/38586/2/src/superio/aspeed/ast2400/... PS2, Line 38: 0x07 either this or the val == 0xc in line 41 is wrong; in the current state the condition below can't become true

https://review.coreboot.org/c/coreboot/+/38586/2/src/superio/aspeed/ast2400/... PS2, Line 43: conf->use_espi = 1; iirc ast2400_init gets called after enable_dev, so this won't have an effect below in enable_dev

-- To view, visit https://review.coreboot.org/c/coreboot/+/38586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Gerrit-Change-Number: 38586 Gerrit-PatchSet: 2 Gerrit-Owner: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sun, 26 Jan 2020 23:52:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Christian Walter (Code Review)

11:57 p.m.

Hello Felix Held, build bot (Jenkins),

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

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

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

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

superio/aspeed/ast2400: Add eSPI auto detection

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/38586/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/38586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Gerrit-Change-Number: 38586 Gerrit-PatchSet: 3 Gerrit-Owner: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Christian Walter (Code Review)

11:59 p.m.

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38586 )

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38586/2/src/superio/aspeed/ast2400/... File src/superio/aspeed/ast2400/superio.c:

https://review.coreboot.org/c/coreboot/+/38586/2/src/superio/aspeed/ast2400/... PS2, Line 38: 0x07

...

either this or the val == 0xc in line 41 is wrong; in the current state the condition below can't be […]

You are totally right. Sorry this is untested yet as I only have the chance to get my hands on real hardware tomorrow.

https://review.coreboot.org/c/coreboot/+/38586/2/src/superio/aspeed/ast2400/... PS2, Line 43: conf->use_espi = 1;

...

iirc ast2400_init gets called after enable_dev, so this won't have an effect below in enable_dev

Maybe like this?

-- To view, visit https://review.coreboot.org/c/coreboot/+/38586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Gerrit-Change-Number: 38586 Gerrit-PatchSet: 3 Gerrit-Owner: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sun, 26 Jan 2020 23:59:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held felix-coreboot@felixheld.de Gerrit-MessageType: comment

Christian Walter (Code Review)

27 Jan 27 Jan

midnight

Hello Felix Held, build bot (Jenkins),

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

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

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

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

superio/aspeed/ast2400: Add eSPI auto detection

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/38586/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/38586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Gerrit-Change-Number: 38586 Gerrit-PatchSet: 4 Gerrit-Owner: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Paul Menzel (Code Review)

10:34 a.m.

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

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

Patch Set 4:

(3 comments)

Tested how?

https://review.coreboot.org/c/coreboot/+/38586/4/src/superio/aspeed/ast2400/... File src/superio/aspeed/ast2400/superio.c:

https://review.coreboot.org/c/coreboot/+/38586/4/src/superio/aspeed/ast2400/... PS4, Line 110: // Detect eSPI Mode automatically C89 style for consistency?

https://review.coreboot.org/c/coreboot/+/38586/4/src/superio/aspeed/ast2400/... PS4, Line 113: 0x0F 0x0f

https://review.coreboot.org/c/coreboot/+/38586/4/src/superio/aspeed/ast2400/... PS4, Line 116: 0x0C 1. 0x0c 2. Add a macro for that value?

-- To view, visit https://review.coreboot.org/c/coreboot/+/38586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Gerrit-Change-Number: 38586 Gerrit-PatchSet: 4 Gerrit-Owner: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 27 Jan 2020 10:34:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Christian Walter (Code Review)

28 Jan 28 Jan

9:30 a.m.

Hello Felix Held, build bot (Jenkins),

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

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

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

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

superio/aspeed/ast2400: Add eSPI auto detection

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/38586/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/38586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Gerrit-Change-Number: 38586 Gerrit-PatchSet: 5 Gerrit-Owner: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Christian Walter (Code Review)

11 Mar 11 Mar

3:37 p.m.

Christian Walter has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/38586 )

Change subject: superio/aspeed/ast2400: Add eSPI auto detection ......................................................................

Abandoned

We always read the default values when it starts up - thus we can not rely on the values. Looks like it gets reconfigured later on.

-- To view, visit https://review.coreboot.org/c/coreboot/+/38586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide58f3210475d949d69d9a91da8b83fc5b2b38ac Gerrit-Change-Number: 38586 Gerrit-PatchSet: 5 Gerrit-Owner: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Christian Walter christian.walter@9elements.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: abandon

1722

days inactive

1767

days old

coreboot-gerrit@coreboot.org

9 comments

4 participants

tags (0)

participants (4)

build bot (Jenkins) (Code Review)
Christian Walter (Code Review)
Felix Held (Code Review)
Paul Menzel (Code Review)