Change in coreboot[master]: zork/variants/dalboz: Use HS200 for eMMC

[Eric Peers (Code Review)]

Eric Peers has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43572 )

Change subject: zork/variants/dalboz: Use HS200 for eMMC
......................................................................

zork/variants/dalboz: Use HS200 for eMMC

Earlier versions of Dalboz did not correctly handle HS400. One fix was
to add stitching vias, but these boards did not have them. b/156539551

Another possible fix is to add tuning parameters including drive
strength, but that is still a WIP. b/158959725

This should correct OS load failures in the meantime by running the bus
slower.

BUG=b:158845662
TEST=build, flash, boot sku 0x5a80000c to OS
BRANCH=None
Signed-off-by: Eric Peers 
Change-Id: Ia3e7a641bde04c5a7be29bf91c38dd8c110ed17a
---
M src/mainboard/google/zork/variants/dalboz/variant.c
1 file changed, 4 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/43572/1

diff --git a/src/mainboard/google/zork/variants/dalboz/variant.c b/src/mainboard/google/zork/variants/dalboz/variant.c
index ceef289..d57b537 100644
--- a/src/mainboard/google/zork/variants/dalboz/variant.c
+++ b/src/mainboard/google/zork/variants/dalboz/variant.c
@@ -127,9 +127,11 @@
    cfg = config_of_soc();

if (sku_has_emmc()) {
-		if (sku_id() == 0x5A800003)
-			/* rev0 boards have issues with HS400 */
+		if ((sku_id() == 0x5A800003) || (sku_id() == 0x5A80000C)) {
+			/* rev0, 1 boards have issues with HS400
+			   b/149941450 */
    		cfg->sd_emmc_config = SD_EMMC_EMMC_HS200;
+		}
    } else {
    	cfg->sd_emmc_config = SD_EMMC_DISABLE;
    }

[Angel Pons (Code Review)]

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

Change subject: zork/variants/dalboz: Use HS200 for eMMC
......................................................................

Patch Set 1: Code-Review+1

[Martin Roth (Code Review)]

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

Change subject: zork/variants/dalboz: Use HS200 for eMMC
......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/43572/1/src/mainboard/google/zork/v... 
File src/mainboard/google/zork/variants/dalboz/variant.c:

https://review.coreboot.org/c/coreboot/+/43572/1/src/mainboard/google/zork/v... 
PS1, Line 132: b/149941450
nit: Since the public can't see the bug, I wouldn't put it in the code.  If there's anything in the bug that needs to be added, just add it in the comment.

[Raul Rangel (Code Review)]

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

Change subject: zork/variants/dalboz: Use HS200 for eMMC
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43572/1/src/mainboard/google/zork/v... 
File src/mainboard/google/zork/variants/dalboz/variant.c:

https://review.coreboot.org/c/coreboot/+/43572/1/src/mainboard/google/zork/v... 
PS1, Line 131: 0, 1
[0, 1] or rev0 and rev1

[Eric Peers (Code Review)]

Hello build bot (Jenkins), Raul Rangel, Martin Roth, Furquan Shaikh, Angel Pons, Aaron Durbin,

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

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

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

Change subject: zork/variants/dalboz: Use HS200 for eMMC
......................................................................

zork/variants/dalboz: Use HS200 for eMMC

Earlier versions of Dalboz did not correctly handle HS400. One fix was
to add stitching vias, but these boards did not have them. b/156539551

Another possible fix is to add tuning parameters including drive
strength, but that is still a WIP. b/158959725

This should correct OS load failures in the meantime by running the bus
slower.

BUG=b:158845662
TEST=build, flash, boot sku 0x5a80000c to OS
BRANCH=None
Signed-off-by: Eric Peers 
Change-Id: Ia3e7a641bde04c5a7be29bf91c38dd8c110ed17a
---
M src/mainboard/google/zork/variants/dalboz/variant.c
1 file changed, 14 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/43572/2

[Eric Peers (Code Review)]

Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43572 )

Change subject: zork/variants/dalboz: Use HS200 for eMMC
......................................................................

Patch Set 2:

(2 comments)

Updated verbage in the command about the hs200 issues. rebuilt & tested on device.

https://review.coreboot.org/c/coreboot/+/43572/1/src/mainboard/google/zork/v... 
File src/mainboard/google/zork/variants/dalboz/variant.c:

https://review.coreboot.org/c/coreboot/+/43572/1/src/mainboard/google/zork/v... 
PS1, Line 131: 0, 1
> [0, 1] or rev0 and rev1

used the "and"

https://review.coreboot.org/c/coreboot/+/43572/1/src/mainboard/google/zork/v... 
PS1, Line 132: b/149941450
> nit: Since the public can't see the bug, I wouldn't put it in the code. […]

bug reference removed. Additional verbage added.

[Angel Pons (Code Review)]

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

Change subject: zork/variants/dalboz: Use HS200 for eMMC
......................................................................

Patch Set 2: Code-Review+2

[Raul Rangel (Code Review)]

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

Change subject: zork/variants/dalboz: Use HS200 for eMMC
......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/43572/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43572/2//COMMIT_MSG@7 
PS2, Line 7: zork
mb/google/zork

Change in coreboot[master]: mb/google/zork/variants/dalboz: Use HS200 for eMMC

[Eric Peers (Code Review)]

Hello build bot (Jenkins), Raul Rangel, Martin Roth, Furquan Shaikh, Angel Pons, Aaron Durbin,

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

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

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

Change subject: mb/google/zork/variants/dalboz: Use HS200 for eMMC
......................................................................

mb/google/zork/variants/dalboz: Use HS200 for eMMC

Earlier versions of Dalboz did not correctly handle HS400. One fix was
to add stitching vias, but these boards did not have them. b/156539551

Another possible fix is to add tuning parameters including drive
strength, but that is still a WIP. b/158959725

This should correct OS load failures in the meantime by running the bus
slower.

BUG=b:158845662
TEST=build, flash, boot sku 0x5a80000c to OS
BRANCH=None
Signed-off-by: Eric Peers 
Change-Id: Ia3e7a641bde04c5a7be29bf91c38dd8c110ed17a
---
M src/mainboard/google/zork/variants/dalboz/variant.c
1 file changed, 14 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/43572/3

Change in coreboot[master]: mb/google/zork/variants/dalboz: Use HS200 for eMMC

[Eric Peers (Code Review)]

Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43572 )

Change subject: mb/google/zork/variants/dalboz: Use HS200 for eMMC
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43572/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43572/2//COMMIT_MSG@7 
PS2, Line 7: zork
> mb/google/zork

changed

Change in coreboot[master]: mb/google/zork/variants/dalboz: Use HS200 for eMMC

[Furquan Shaikh (Code Review)]

Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43572 )

Change subject: mb/google/zork/variants/dalboz: Use HS200 for eMMC
......................................................................

mb/google/zork/variants/dalboz: Use HS200 for eMMC

Earlier versions of Dalboz did not correctly handle HS400. One fix was
to add stitching vias, but these boards did not have them. b/156539551

Another possible fix is to add tuning parameters including drive
strength, but that is still a WIP. b/158959725

This should correct OS load failures in the meantime by running the bus
slower.

BUG=b:158845662
TEST=build, flash, boot sku 0x5a80000c to OS
BRANCH=None
Signed-off-by: Eric Peers 
Change-Id: Ia3e7a641bde04c5a7be29bf91c38dd8c110ed17a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43572
Reviewed-by: Angel Pons 
Reviewed-by: Raul Rangel 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/google/zork/variants/dalboz/variant.c
1 file changed, 14 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Raul Rangel: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/mainboard/google/zork/variants/dalboz/variant.c b/src/mainboard/google/zork/variants/dalboz/variant.c
index 0212161..de4338e 100644
--- a/src/mainboard/google/zork/variants/dalboz/variant.c
+++ b/src/mainboard/google/zork/variants/dalboz/variant.c
@@ -127,9 +127,21 @@
    cfg = config_of_soc();

if (sku_has_emmc()) {
-		if (sku_id() == 0x5A800003)
-			/* rev0 boards have issues with HS400 */
+		if ((sku_id() == 0x5A800003) || (sku_id() == 0x5A80000C)) {
+			/*
+			 * rev0 and rev1 boards have issues with HS400
+			 *
+			 * There is a tuning fix with ES which shows promise
+			 * for some boards, and a HW fix with stitching vias.
+			 * There were also concerns that these boards did not
+			 * have good margins for certain skus.
+			 *
+			 * But these original boards have none of these fixes.
+			 * So we keep the speed low here, with the intent that
+			 * other variants implement these corrections.
+			 */
    		cfg->sd_emmc_config = SD_EMMC_EMMC_HS200;
+		}
    } else {
    	cfg->sd_emmc_config = SD_EMMC_DISABLE;
    }