Change in ...coreboot[master]: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller

List overview All Threads
Download

newer

older

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

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

Raul Rangel (Code Review)

23 Apr 2019 23 Apr '19

10:06 p.m.

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32433

Change subject: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller ......................................................................

soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller

We need to support entering D3Cold from the OS to workaround a bug in the SDHC where the data lines get stuck always reading zeros.

BUG=b:122749418 TEST=Verified the linux kernel can transition between D3 and D0

Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Signed-off-by: Raul E Rangel rrangel@chromium.org --- M src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl 1 file changed, 34 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32433/1

diff --git a/src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl b/src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl index 15e4d2f..2139f8d 100644 --- a/src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl +++ b/src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl @@ -56,6 +56,16 @@ /* 0:14.7 - SD Controller */ Device(SDCN) { Name(_ADR, 0x00140007) + + Method(_PS0) { + FDDC(24, 0) + } + Method(_PS3) { + FDDC(24, 3) + } + Method(_PSC) { + Return(SDTD) + } } /* end SDCN */

Name(CRES, ResourceTemplate() { @@ -286,8 +296,9 @@ offset (0x1e70), /* SD D3 Control */ SDTD, 2, , 1, + SDPD, 1, , 1, - , 2, + , 1, SDRT, 1, SDSC, 1,

@@ -433,7 +444,17 @@ /* todo Case(15) { STD0()} */ /* SATA */ Case(18) { U2D0()} /* EHCI */ Case(23) { U3D0()} /* XHCI */ -/* todo Case(24) { SDD0()} */ /* SD */ + Case(24) { + /* Enable Power */ + Store(One, SDPD) + Store(SDDS, Local0) + while(LNotEqual(Local0,0x7)) { + Store(SDDS, Local0) + } + + /* D0-uninitialized */ + Store(0x00, SDTD) + } } } else { /* put device into D3cold */ @@ -489,7 +510,17 @@ /* todo Case(15) { STD3()} */ /* SATA */ Case(18) { U2D3()} /* EHCI */ Case(23) { U3D3()} /* XHCI */ -/* todo Case(24) { SDD3()} */ /* SD */ + Case(24) { + /* Disable power */ + Store(Zero, SDPD) + Store(SDDS, Local0) + while(LNotEqual(Local0,0x0)) { + Store(SDDS, Local0) + } + + /* D3 */ + Store(0x03, SDTD) + } } /* Turn off Power */ if(LEqual(I0TD, 3)) {

Attachments:

attachment.htm (text/html — 5.8 KB)

Show replies by date

Raul Rangel (Code Review)

1 May 1 May

10:43 p.m.

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32433 )

Change subject: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller ......................................................................

Patch Set 2:

This change is ready for review.

-- To view, visit https://review.coreboot.org/c/coreboot/+/32433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Gerrit-Change-Number: 32433 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@google.com Gerrit-Reviewer: Marshall Dawson marshalldawson3rd@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Raul Rangel (Don't Use) Gerrit-Comment-Date: Wed, 01 May 2019 22:43:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Daniel Kurtz (Code Review)

11:39 p.m.

Daniel Kurtz has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32433 )

Change subject: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32433/2/src/soc/amd/stoneyridge/acpi/sb_pci0... File src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl:

https://review.coreboot.org/#/c/32433/2/src/soc/amd/stoneyridge/acpi/sb_pci0... PS2, Line 455: /* D0-uninitialized */ oh, hmm. sorry I didn't catch this earlier. It looks like, based on the "shoes-and-socks" principle, and on the other equivalent blocks above, that this next line should come first in the "D0" case. That is, for consistency, we first clear the state bits, then actually power on. and in the D3 case, we first disable power and then set the state bits.

-- To view, visit https://review.coreboot.org/c/coreboot/+/32433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Gerrit-Change-Number: 32433 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@google.com Gerrit-Reviewer: Marshall Dawson marshalldawson3rd@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Raul Rangel (Don't Use) Gerrit-Comment-Date: Wed, 01 May 2019 23:39:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Raul Rangel (Code Review)

13 May 13 May

9 p.m.

Hello Marshall Dawson, Daniel Kurtz, build bot (Jenkins), Daniel Kurtz, Martin Roth,

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

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

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

Change subject: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller ......................................................................

soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller

We need to support entering D3Cold from the OS to work around a bug in the SDHC where the data lines get stuck always reading zeros.

BUG=b:122749418 TEST=Verified the linux kernel can transition between D3 and D0. Also verified that the device can suspend and resume and continue to have a functioning SD controller after.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/32433/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/32433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Gerrit-Change-Number: 32433 Gerrit-PatchSet: 3 Gerrit-Owner: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@google.com Gerrit-Reviewer: Marshall Dawson marshalldawson3rd@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Raul Rangel (Don't Use) Gerrit-MessageType: newpatchset

Raul Rangel (Code Review)

9:01 p.m.

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32433 )

Change subject: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/32433/2/src/soc/amd/stoneyridge/acpi/sb_pci0... File src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl:

https://review.coreboot.org/#/c/32433/2/src/soc/amd/stoneyridge/acpi/sb_pci0... PS2, Line 455: /* D0-uninitialized */

...

oh, hmm. sorry I didn't catch this earlier. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/32433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Gerrit-Change-Number: 32433 Gerrit-PatchSet: 3 Gerrit-Owner: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@google.com Gerrit-Reviewer: Marshall Dawson marshalldawson3rd@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Raul Rangel (Don't Use) Gerrit-Comment-Date: Mon, 13 May 2019 21:01:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Daniel Kurtz djkurtz@google.com Gerrit-MessageType: comment

Raul Rangel (Code Review)

15 May 15 May

3:49 p.m.

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32433 )

Change subject: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller ......................................................................

Patch Set 3:

PTAL

-- To view, visit https://review.coreboot.org/c/coreboot/+/32433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Gerrit-Change-Number: 32433 Gerrit-PatchSet: 3 Gerrit-Owner: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@google.com Gerrit-Reviewer: Marshall Dawson marshalldawson3rd@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Raul Rangel (Don't Use) Gerrit-Comment-Date: Wed, 15 May 2019 15:49:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Martin Roth (Code Review)

4:01 p.m.

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32433 )

Change subject: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller ......................................................................

Patch Set 3: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/32433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Gerrit-Change-Number: 32433 Gerrit-PatchSet: 3 Gerrit-Owner: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@google.com Gerrit-Reviewer: Marshall Dawson marshalldawson3rd@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Raul Rangel (Don't Use) Gerrit-Comment-Date: Wed, 15 May 2019 16:01:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Martin Roth (Code Review)

4:01 p.m.

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32433 )

Change subject: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller ......................................................................

soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller

We need to support entering D3Cold from the OS to work around a bug in the SDHC where the data lines get stuck always reading zeros.

BUG=b:122749418 TEST=Verified the linux kernel can transition between D3 and D0. Also verified that the device can suspend and resume and continue to have a functioning SD controller after.

Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Signed-off-by: Raul E Rangel rrangel@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32433 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Martin Roth martinroth@google.com --- M src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl 1 file changed, 28 insertions(+), 3 deletions(-)

Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved

diff --git a/src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl b/src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl index 15e4d2f..1334df1 100644 --- a/src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl +++ b/src/soc/amd/stoneyridge/acpi/sb_pci0_fch.asl @@ -56,6 +56,16 @@ /* 0:14.7 - SD Controller */ Device(SDCN) { Name(_ADR, 0x00140007) + + Method(_PS0) { + FDDC(24, 0) + } + Method(_PS3) { + FDDC(24, 3) + } + Method(_PSC) { + Return(SDTD) + } } /* end SDCN */

Name(CRES, ResourceTemplate() { @@ -286,8 +296,9 @@ offset (0x1e70), /* SD D3 Control */ SDTD, 2, , 1, + SDPD, 1, , 1, - , 2, + , 1, SDRT, 1, SDSC, 1,

@@ -433,7 +444,14 @@ /* todo Case(15) { STD0()} */ /* SATA */ Case(18) { U2D0()} /* EHCI */ Case(23) { U3D0()} /* XHCI */ -/* todo Case(24) { SDD0()} */ /* SD */ + Case(24) { /* SD */ + Store(0x00, SDTD) + Store(One, SDPD) + Store(SDDS, Local0) + while(LNotEqual(Local0,0x7)) { + Store(SDDS, Local0) + } + } } } else { /* put device into D3cold */ @@ -489,7 +507,14 @@ /* todo Case(15) { STD3()} */ /* SATA */ Case(18) { U2D3()} /* EHCI */ Case(23) { U3D3()} /* XHCI */ -/* todo Case(24) { SDD3()} */ /* SD */ + Case(24) { /* SD */ + Store(Zero, SDPD) + Store(SDDS, Local0) + while(LNotEqual(Local0,0x0)) { + Store(SDDS, Local0) + } + Store(0x03, SDTD) + } } /* Turn off Power */ if(LEqual(I0TD, 3)) {

-- To view, visit https://review.coreboot.org/c/coreboot/+/32433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Gerrit-Change-Number: 32433 Gerrit-PatchSet: 4 Gerrit-Owner: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@google.com Gerrit-Reviewer: Marshall Dawson marshalldawson3rd@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Raul Rangel (Don't Use) Gerrit-MessageType: merged

Daniel Kurtz (Code Review)

4:04 p.m.

Daniel Kurtz has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32433 )

Change subject: soc/amd/stoneyridge: Add ACPI D3Cold support for SD Controller ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/32433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifbf48f20c03a752ce3ff773296b536e92db16a62 Gerrit-Change-Number: 32433 Gerrit-PatchSet: 4 Gerrit-Owner: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@chromium.org Gerrit-Reviewer: Daniel Kurtz djkurtz@google.com Gerrit-Reviewer: Marshall Dawson marshalldawson3rd@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Raul Rangel rrangel@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Raul Rangel (Don't Use) Gerrit-Comment-Date: Wed, 15 May 2019 16:04:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2054

days inactive

2076

days old

coreboot-gerrit@coreboot.org

8 comments

3 participants

tags (0)

participants (3)

Daniel Kurtz (Code Review)
Martin Roth (Code Review)
Raul Rangel (Code Review)