Bora Guvendik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37775 )
Change subject: storage/mmc: Fix wrong frequency setting for HS speed mode. ......................................................................
storage/mmc: Fix wrong frequency setting for HS speed mode.
Emmc spec, JEDEC Standard No. 84-B51, section 6.6.2.3, selection flow of HS400 using Enhanced Strobe states that host should change frequency to ≤ 52MHz when switching to HS speed mode first. In current code, mmc_select_hs400() calls mmc_select_hs() to do this, however caps are not cleared, so when switching from HS200 to HS400, caps will still have DRVR_CAP_HS200, and mmc_recalculate_clock() will set 200Mhz instead of ≤ 52MHz.
BUG=b:140124451 TEST=Switch speed from HS200 to HS400 on WHL RVP.
Change-Id: Ie639c7616105cca638417d7bc1db95b561afb7af Signed-off-by: Bora Guvendik bora.guvendik@intel.com --- M src/commonlib/storage/mmc.c 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/37775/1
diff --git a/src/commonlib/storage/mmc.c b/src/commonlib/storage/mmc.c index 0b682ad..1e0f7d2 100644 --- a/src/commonlib/storage/mmc.c +++ b/src/commonlib/storage/mmc.c @@ -186,6 +186,7 @@
/* Increase the controller clock speed */ SET_TIMING(media->ctrlr, BUS_TIMING_MMC_HS); + media->caps &= ~(DRVR_CAP_HS200 | DRVR_CAP_HS400); media->caps |= DRVR_CAP_HS52 | DRVR_CAP_HS; mmc_recalculate_clock(media); ret = sd_mmc_send_status(media, SD_MMC_IO_RETRIES);
Hello Thejaswani Putta, Subrata Banik, Selma Bensaid, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37775
to look at the new patch set (#2).
Change subject: storage/mmc: Fix wrong frequency setting for HS speed mode ......................................................................
storage/mmc: Fix wrong frequency setting for HS speed mode
Emmc spec, JEDEC Standard No. 84-B51, section 6.6.2.3, selection flow of HS400 using Enhanced Strobe states that host should change frequency to ≤ 52MHz when switching to HS speed mode first. In current code, mmc_select_hs400() calls mmc_select_hs() to do this, however caps are not cleared, so when switching from HS200 to HS400, caps will still have DRVR_CAP_HS200, and mmc_recalculate_clock() will set 200Mhz instead of ≤ 52MHz.
BUG=b:140124451 TEST=Switch speed from HS200 to HS400 on WHL RVP.
Change-Id: Ie639c7616105cca638417d7bc1db95b561afb7af Signed-off-by: Bora Guvendik bora.guvendik@intel.com --- M src/commonlib/storage/mmc.c 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/37775/2
Selma Bensaid has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37775 )
Change subject: storage/mmc: Fix wrong frequency setting for HS speed mode ......................................................................
Patch Set 2: Code-Review+1
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37775 )
Change subject: storage/mmc: Fix wrong frequency setting for HS speed mode ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37775 )
Change subject: storage/mmc: Fix wrong frequency setting for HS speed mode ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37775/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37775/2//COMMIT_MSG@16 PS2, Line 16: What practical problems does this cause?
Hello Thejaswani Putta, Subrata Banik, Selma Bensaid, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37775
to look at the new patch set (#3).
Change subject: storage/mmc: Fix wrong frequency setting for HS speed mode ......................................................................
storage/mmc: Fix wrong frequency setting for HS speed mode
Emmc spec, JEDEC Standard No. 84-B51, section 6.6.2.3, selection flow of HS400 using Enhanced Strobe states that host should change frequency to ≤ 52MHz when switching to HS speed mode first. In current code, mmc_select_hs400() calls mmc_select_hs() to do this, however caps are not cleared, so when switching from HS200 to HS400, caps will still have DRVR_CAP_HS200, and mmc_recalculate_clock() will set 200Mhz instead of ≤ 52MHz. As a result, switching to HS400 will intermittently fail.
BUG=b:140124451 TEST=Switch speed from HS200 to HS400 on WHL RVP.
Change-Id: Ie639c7616105cca638417d7bc1db95b561afb7af Signed-off-by: Bora Guvendik bora.guvendik@intel.com --- M src/commonlib/storage/mmc.c 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/37775/3
Bora Guvendik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37775 )
Change subject: storage/mmc: Fix wrong frequency setting for HS speed mode ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37775/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37775/2//COMMIT_MSG@16 PS2, Line 16:
What practical problems does this cause?
Updated the comment.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37775 )
Change subject: storage/mmc: Fix wrong frequency setting for HS speed mode ......................................................................
storage/mmc: Fix wrong frequency setting for HS speed mode
Emmc spec, JEDEC Standard No. 84-B51, section 6.6.2.3, selection flow of HS400 using Enhanced Strobe states that host should change frequency to ≤ 52MHz when switching to HS speed mode first. In current code, mmc_select_hs400() calls mmc_select_hs() to do this, however caps are not cleared, so when switching from HS200 to HS400, caps will still have DRVR_CAP_HS200, and mmc_recalculate_clock() will set 200Mhz instead of ≤ 52MHz. As a result, switching to HS400 will intermittently fail.
BUG=b:140124451 TEST=Switch speed from HS200 to HS400 on WHL RVP.
Change-Id: Ie639c7616105cca638417d7bc1db95b561afb7af Signed-off-by: Bora Guvendik bora.guvendik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37775 Reviewed-by: Selma Bensaid selma.bensaid@intel.com Reviewed-by: Subrata Banik subrata.banik@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/commonlib/storage/mmc.c 1 file changed, 1 insertion(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Selma Bensaid: Looks good to me, but someone else must approve
diff --git a/src/commonlib/storage/mmc.c b/src/commonlib/storage/mmc.c index 0b682ad..1e0f7d2 100644 --- a/src/commonlib/storage/mmc.c +++ b/src/commonlib/storage/mmc.c @@ -186,6 +186,7 @@
/* Increase the controller clock speed */ SET_TIMING(media->ctrlr, BUS_TIMING_MMC_HS); + media->caps &= ~(DRVR_CAP_HS200 | DRVR_CAP_HS400); media->caps |= DRVR_CAP_HS52 | DRVR_CAP_HS; mmc_recalculate_clock(media); ret = sd_mmc_send_status(media, SD_MMC_IO_RETRIES);
-
Bora Guvendik (Code Review) -
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Selma Bensaid (Code Review)
-
Subrata Banik (Code Review)