Change in ...coreboot[master]: mb/google/sarien: Fix SSD's power off sequence before going to S5

[Roy Mingi Park (Code Review)]

Roy Mingi Park has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33182

Change subject: mb/google/sarien: Fix SSD's power off sequence before going to S5
......................................................................

mb/google/sarien: Fix SSD's power off sequence before going to S5

BUG=b:133389422
TEST=check SSD's power off sequence to meet PCIE requirement.
     SSD's reset should be cleared before clearing SSD's power EN Pin.

Change-Id: Ia106b805deafb8a442b56bcce91b51135cb32988
Signed-off-by: Roy Mingi Park 
---
M src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl
M src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl
2 files changed, 4 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33182/1

diff --git a/src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl b/src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl
index 6eba2bc..8258f32 100644
--- a/src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl
+++ b/src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl
@@ -40,8 +40,9 @@

/* Clear SSD EN adn RST pin to avoid leakage */
    If (Arg0 == 5) {
-		_SB.PCI0.CTXS (SSD_EN)
    	_SB.PCI0.CTXS (SSD_RST)
+		sleep(1)
+		_SB.PCI0.CTXS (SSD_EN)
    }
 }

diff --git a/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl b/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl
index 6eba2bc..8258f32 100644
--- a/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl
+++ b/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl
@@ -40,8 +40,9 @@

/* Clear SSD EN adn RST pin to avoid leakage */
    If (Arg0 == 5) {
-		_SB.PCI0.CTXS (SSD_EN)
    	_SB.PCI0.CTXS (SSD_RST)
+		sleep(1)
+		_SB.PCI0.CTXS (SSD_EN)
    }
 }

[Roy Mingi Park (Code Review)]

Roy Mingi Park has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33182 )

Change subject: mb/google/sarien: Fix SSD's power off sequence before going to S5
......................................................................

Patch Set 1: Code-Review+1

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33182 )

Change subject: mb/google/sarien: Fix SSD's power off sequence before going to S5
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/33182/1/src/mainboard/google/sarien/variants... 
File src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl:

https://review.coreboot.org/#/c/33182/1/src/mainboard/google/sarien/variants... 
PS1, Line 44: s
Sleep (1)

(to match the coding style)

[Roy Mingi Park (Code Review)]

Hello EricR Lai, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Patrick Georgi,

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

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

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

Change subject: mb/google/sarien: Fix SSD's power off sequence before going to S5
......................................................................

mb/google/sarien: Fix SSD's power off sequence before going to S5

BUG=b:133389422
TEST=check SSD's power off sequence to meet PCIE requirement.
     SSD's reset should be cleared before clearing SSD's power EN Pin.

Change-Id: Ia106b805deafb8a442b56bcce91b51135cb32988
Signed-off-by: Roy Mingi Park 
---
M src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl
M src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl
2 files changed, 4 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/33182/2

[Roy Mingi Park (Code Review)]

Roy Mingi Park has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33182 )

Change subject: mb/google/sarien: Fix SSD's power off sequence before going to S5
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/#/c/33182/1/src/mainboard/google/sarien/variants... 
File src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl:

https://review.coreboot.org/#/c/33182/1/src/mainboard/google/sarien/variants... 
PS1, Line 44: s
> Sleep (1) […]

Done

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33182 )

Change subject: mb/google/sarien: Fix SSD's power off sequence before going to S5
......................................................................

Patch Set 2: Code-Review+2

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33182 )

Change subject: mb/google/sarien: Fix SSD's power off sequence before going to S5
......................................................................

Patch Set 2:

I want do this as well. But our HW is no concern about this power off timing.. :)

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33182 )

Change subject: mb/google/sarien: Fix SSD's power off sequence before going to S5
......................................................................

Patch Set 2: Code-Review+2

[Duncan Laurie (Code Review)]

Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33182 )

Change subject: mb/google/sarien: Fix SSD's power off sequence before going to S5
......................................................................

mb/google/sarien: Fix SSD's power off sequence before going to S5

BUG=b:133389422
TEST=check SSD's power off sequence to meet PCIE requirement.
     SSD's reset should be cleared before clearing SSD's power EN Pin.

Change-Id: Ia106b805deafb8a442b56bcce91b51135cb32988
Signed-off-by: Roy Mingi Park 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33182
Reviewed-by: EricR Lai 
Reviewed-by: Duncan Laurie 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl
M src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl
2 files changed, 4 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Duncan Laurie: Looks good to me, approved
  Roy Mingi Park: Looks good to me, but someone else must approve
  EricR Lai: Looks good to me, approved

diff --git a/src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl b/src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl
index 6eba2bc..4b05ba8 100644
--- a/src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl
+++ b/src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl
@@ -40,8 +40,9 @@

/* Clear SSD EN adn RST pin to avoid leakage */
    If (Arg0 == 5) {
-		_SB.PCI0.CTXS (SSD_EN)
    	_SB.PCI0.CTXS (SSD_RST)
+		Sleep(1)
+		_SB.PCI0.CTXS (SSD_EN)
    }
 }

diff --git a/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl b/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl
index 6eba2bc..4b05ba8 100644
--- a/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl
+++ b/src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl
@@ -40,8 +40,9 @@

/* Clear SSD EN adn RST pin to avoid leakage */
    If (Arg0 == 5) {
-		_SB.PCI0.CTXS (SSD_EN)
    	_SB.PCI0.CTXS (SSD_RST)
+		Sleep(1)
+		_SB.PCI0.CTXS (SSD_EN)
    }
 }