Patrik Tesarik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40403 )
Change subject: src/mainboard: Fix eMMC for UP2 with EDK2 ......................................................................
src/mainboard: Fix eMMC for UP2 with EDK2
Historically the UP2's eMMC maximal host speed was reduced to DDR50, because HS200 showed I/O errorsi in the host kernel. Lately we found out that with EDK2 the correct Host Speed could not be set properly during EDK2 platform initialization. Therefore eMMC would not show up for boot device selection.
This commit sets the eMMC MaxHostSpeed to the designed max value of the used eMMC on the UP2 board and furthermore moves the setting from the ramstage.c to the devicetree.cb file for better maintainability.
Signed-off-by: Patrik Tesarik mail@patrik-tesarik.de Change-Id: I8d53204d8a776efd560fbdea918f83e180813179 --- M src/mainboard/up/squared/devicetree.cb M src/mainboard/up/squared/ramstage.c 2 files changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/40403/1
diff --git a/src/mainboard/up/squared/devicetree.cb b/src/mainboard/up/squared/devicetree.cb index b4d5810..383a26b 100644 --- a/src/mainboard/up/squared/devicetree.cb +++ b/src/mainboard/up/squared/devicetree.cb @@ -16,6 +16,8 @@
register "enable_vtd" = "1"
+ # Override eMMC MaxHostSpeed + # 0:HS400(Default) 1:HS200 2:DDR50 register "emmc_host_max_speed" = "1"
device cpu_cluster 0 on diff --git a/src/mainboard/up/squared/ramstage.c b/src/mainboard/up/squared/ramstage.c index 8295634..a5cc5d4 100644 --- a/src/mainboard/up/squared/ramstage.c +++ b/src/mainboard/up/squared/ramstage.c @@ -30,7 +30,6 @@ silconfig->IoApicBdfValid = 0x1; // 0x0 silconfig->IoApicDeviceNumber = 0x1F; // 0xf silconfig->LPSS_S0ixEnable = 0x1; // 0x0 - silconfig->eMMCHostMaxSpeed = 0x2; // 0x0 silconfig->Usb30Mode = 0x1; // 0x0 silconfig->HdAudioDspUaaCompliance = 0x1; // 0x0 silconfig->InitS3Cpu = 0x1; // 0x0
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40403
to look at the new patch set (#2).
Change subject: src/mainboard: Make eMMC for UP2 with EDK2 ......................................................................
src/mainboard: Make eMMC for UP2 with EDK2
Historically the UP2's eMMC maximum host speed was reduced to DDR50, because HS200 showed I/O errors in the host kernel. Lately we found out that with EDK2 the correct Host Speed could not be set properly during EDK2 platform initialization. Therefore eMMC would not show up for boot device selection.
This commit sets the eMMC MaxHostSpeed to the designed max value of the used eMMC on the UP2 board and furthermore moves the setting from the ramstage.c to the devicetree.cb file for better maintainability.
Though CRC errors are still visible in EDK II debug logs, no other negative effect have been observed.
Signed-off-by: Patrik Tesarik mail@patrik-tesarik.de Change-Id: I8d53204d8a776efd560fbdea918f83e180813179 --- M src/mainboard/up/squared/devicetree.cb M src/mainboard/up/squared/ramstage.c 2 files changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/40403/2
Patrik Tesarik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40403 )
Change subject: src/mainboard: Make eMMC for UP2 with EDK2 ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/40403/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40403/1//COMMIT_MSG@7 PS1, Line 7: src/mainboard: Fix eMMC for UP2 with EDK2
mb/up/squared: Make eMMC work with EDK2
Ack
https://review.coreboot.org/c/coreboot/+/40403/1//COMMIT_MSG@9 PS1, Line 9: maximal
maximum
Ack
https://review.coreboot.org/c/coreboot/+/40403/1//COMMIT_MSG@10 PS1, Line 10: found out that with
Should be on the next line
Ack
https://review.coreboot.org/c/coreboot/+/40403/1//COMMIT_MSG@10 PS1, Line 10: i
spurious `i`
Ack
https://review.coreboot.org/c/coreboot/+/40403/1/src/mainboard/up/squared/de... File src/mainboard/up/squared/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40403/1/src/mainboard/up/squared/de... PS1, Line 20: (
Add a space before the parenthesis?
This was copied from another devicetree.cb and is basically the way of writing in the FSP header documentation for Apollolake, if I recall correctly.
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40403
to look at the new patch set (#3).
Change subject: src/mainboard: Make eMMC for UP2 with EDK2 ......................................................................
src/mainboard: Make eMMC for UP2 with EDK2
Historically the UP2's eMMC maximum host speed was reduced to DDR50, because HS200 showed I/O errors in the host kernel. Lately we found out that with EDK2 the correct Host Speed could not be set properly during EDK2 platform initialization. Therefore eMMC would not show up for boot device selection.
This commit sets the eMMC MaxHostSpeed to the designed max value of the used eMMC on the UP2 board and furthermore moves the setting from the ramstage.c to the devicetree.cb file for better maintainability.
Though CRC errors are still visible in EDK II debug logs, no other negative effects have been observed.
Signed-off-by: Patrik Tesarik mail@patrik-tesarik.de Change-Id: I8d53204d8a776efd560fbdea918f83e180813179 --- M src/mainboard/up/squared/devicetree.cb M src/mainboard/up/squared/ramstage.c 2 files changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/40403/3
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40403
to look at the new patch set (#4).
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
mb/up/squared: Fix eMMC speed for UP2 with EDK2
Historically the UP2's eMMC maximum host speed was reduced to DDR50, because HS200 showed I/O errors in the host kernel. Lately we found out that with EDK2 the correct Host Speed could not be set properly during EDK2 platform initialization. Therefore eMMC would not show up for boot device selection.
This commit sets the eMMC MaxHostSpeed to the designed max value of the used eMMC on the UP2 board and furthermore moves the setting from the ramstage.c to the devicetree.cb file for better maintainability.
Though CRC errors are still visible in EDK II debug logs, no other negative effects have been observed.
Signed-off-by: Patrik Tesarik mail@patrik-tesarik.de Change-Id: I8d53204d8a776efd560fbdea918f83e180813179 --- M payloads/external/tianocore/Makefile M src/mainboard/up/squared/devicetree.cb M src/mainboard/up/squared/ramstage.c 3 files changed, 7 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/40403/4
Patrik Tesarik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40403 )
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40403/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40403/3//COMMIT_MSG@7 PS3, Line 7: src/mainboard: Make eMMC for UP2 with EDK2
looks like the verb "work" from my suggestion was omitted
@Angel, yes I forgot to add the additional word. Sorry.
@dMichał, I like your wording better. I think it's more precise to what the commit will affect if applied.
Thanks for both of your suggestions.
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40403
to look at the new patch set (#5).
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
mb/up/squared: Fix eMMC speed for UP2 with EDK2
Historically (402fe20e3e10f0f2aa1329eb60970e56bf92986e) the UP2's eMMC maximum host speed was reduced to DDR50, because HS200 showed I/O errors in the host kernel. Lately we found out that with EDK2 the correct Host Speed could not be set properly during EDK2 platform initialization. Therefore eMMC would not show up for boot device selection.
This commit sets the eMMC MaxHostSpeed to the designed max value of the used eMMC on the UP2 board and furthermore moves the setting from the ramstage.c to the devicetree.cb file for better maintainability.
Though CRC errors are still visible in EDK II debug logs, no other negative effects have been observed.
Signed-off-by: Patrik Tesarik mail@patrik-tesarik.de Change-Id: I8d53204d8a776efd560fbdea918f83e180813179 --- M src/mainboard/up/squared/devicetree.cb M src/mainboard/up/squared/ramstage.c 2 files changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/40403/5
Patrik Tesarik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40403 )
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40403/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40403/4//COMMIT_MSG@9 PS4, Line 9: Historically the UP2's eMMC maximum host speed was reduced to : DDR50, because HS200 showed I/O errors in the host kernel.
Please add a reference.
Ack
https://review.coreboot.org/c/coreboot/+/40403/4//COMMIT_MSG@13 PS4, Line 13: Therefore eMMC would not show up for boot device selection.
Please re-flow for 72/75 characters.
Ack
https://review.coreboot.org/c/coreboot/+/40403/4//COMMIT_MSG@17 PS4, Line 17: ramstage.c to the devicetree.cb file for better maintainability.
The setting seems to be already present in the devicetree.
I don't see that. Could you please tell me why you think that?
I did git diff against master and couldn't find a eMMC host speed setting there.
https://review.coreboot.org/c/coreboot/+/40403/4/payloads/external/tianocore... File payloads/external/tianocore/Makefile:
PS4:
This seems unrelated. Please separate it into a separate commit.
Ack
Patrik Tesarik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40403 )
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40403/1/src/mainboard/up/squared/de... File src/mainboard/up/squared/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40403/1/src/mainboard/up/squared/de... PS1, Line 20: (
It looks awkward, though.
Done
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40403
to look at the new patch set (#6).
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
mb/up/squared: Fix eMMC speed for UP2 with EDK2
Historically (402fe20e3e10f0f2aa1329eb60970e56bf92986e) the UP2's eMMC maximum host speed was reduced to DDR50, because HS200 showed I/O errors in the host kernel. Lately we found out that with EDK2 the correct Host Speed could not be set properly during EDK2 platform initialization. Therefore eMMC would not show up for boot device selection.
This commit sets the eMMC MaxHostSpeed to the designed max value of the used eMMC on the UP2 board and furthermore drops the override from the ramstage.c. It's already set in the devicetree.cb.
Though CRC errors are still visible in EDK II debug logs, no other negative effects have been observed.
Signed-off-by: Patrik Tesarik mail@patrik-tesarik.de Change-Id: I8d53204d8a776efd560fbdea918f83e180813179 --- M src/mainboard/up/squared/devicetree.cb M src/mainboard/up/squared/ramstage.c 2 files changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/40403/6
Patrik Tesarik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40403 )
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40403/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40403/4//COMMIT_MSG@17 PS4, Line 17: ramstage.c to the devicetree.cb file for better maintainability.
What Paul actually meant: […]
Done
Hello Felix Singer, build bot (Jenkins), Patrick Rudolph, Paul Menzel, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40403
to look at the new patch set (#7).
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
mb/up/squared: Fix eMMC speed for UP2 with EDK2
Since commit 402fe20e (mb/up/squared: Add mainboard) the UP2's eMMC maximum host speed was reduced to DDR50, because HS200 showed I/O errors in the host kernel. Lately we found out that with EDK2 master the correct Host Speed could not be set properly during EDK2 platform initialization. Therefore eMMC would not show up for boot device selection.
This commit sets the eMMC MaxHostSpeed to the designed max value of the used eMMC on the UP2 board and furthermore drops the override from the ramstage.c. It's already set in the devicetree.cb.
Though CRC errors are still visible in EDK II debug logs, no other negative effects have been observed.
Signed-off-by: Patrik Tesarik mail@patrik-tesarik.de Change-Id: I8d53204d8a776efd560fbdea918f83e180813179 --- M src/mainboard/up/squared/devicetree.cb M src/mainboard/up/squared/ramstage.c 2 files changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/40403/7
Patrik Tesarik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40403 )
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40403/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40403/6//COMMIT_MSG@9 PS6, Line 9: Historically (402fe20e3e10f0f2aa1329eb60970e56bf92986e)
Since commit 402fe20e (mb/up/squared: Add mainboard)
Ack
https://review.coreboot.org/c/coreboot/+/40403/6//COMMIT_MSG@11 PS6, Line 11: Lately we found out that with EDK2 the correct Host : Speed could not be set properly during EDK2 platform initialization. : Therefore eMMC would not show up for boot device selection.
it never worked with MrChromebox tianocore branch. […]
Ack
Hello Felix Singer, build bot (Jenkins), Patrick Rudolph, Paul Menzel, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40403
to look at the new patch set (#8).
Change subject: mb/up/squared: Fix eMMC speed for UP2 with EDK2 ......................................................................
mb/up/squared: Fix eMMC speed for UP2 with EDK2
Since commit 402fe20e (mb/up/squared: Add mainboard) the UP2's eMMC maximum host speed was reduced to DDR50, because HS200 showed I/O errors in the host kernel. Lately we found out that with EDK2 master the correct Host Speed could not be set properly during EDK2 platform initialization. Therefore eMMC would not show up for boot device selection.
This commit sets the eMMC MaxHostSpeed to the designed max value of the used eMMC on the UP2 board and furthermore drops the override from the ramstage.c. It's already set in the devicetree.cb.
Though CRC errors are still visible in EDK II debug logs, no other negative effects have been observed.
Signed-off-by: Patrik Tesarik mail@patrik-tesarik.de Change-Id: I8d53204d8a776efd560fbdea918f83e180813179 --- M src/mainboard/up/squared/devicetree.cb M src/mainboard/up/squared/ramstage.c 2 files changed, 4 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/40403/8
-
Patrik Tesarik (Code Review)