Change in ...coreboot[master]: Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz
Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31772
Change subject: Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz ......................................................................
Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz
Currently bayhub BH7720 eMMC is set to internal clock of 50MHz. This is because some eMMC device don't work at 200 MHz without clock tuning. This was never intended as permanent code, as it also slowed eMMC after OS loaded because internal clock remained at 50MHz. In order to solve this issue, code must be added to execute clock tuning, and BH720_PCR_CSR_EMMC_MODE_SEL bit must be set, not cleared (to revert clock back to 200 MHz).
BUG=b:122244718 TEST=Built grunt without code, boot/reboot, measured eMMC time, saved results. Added code, rebuilt grunt, boot/reboot, measured eMMC time. Time saved: 123.5 milliseconds.
Change-Id: I8aefb524a76ab469e776f2f22aed80b29b67ae8e Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- M src/commonlib/storage/bouncebuf.c M src/commonlib/storage/pci_sdhci.c M src/commonlib/storage/sdhci.h M src/drivers/generic/bayhub/bh720.h M src/mainboard/google/kahlee/variants/baseboard/mainboard.c 5 files changed, 80 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/31772/1
diff --git a/src/commonlib/storage/bouncebuf.c b/src/commonlib/storage/bouncebuf.c index 6ddfb70..96554e6 100644 --- a/src/commonlib/storage/bouncebuf.c +++ b/src/commonlib/storage/bouncebuf.c @@ -19,6 +19,7 @@ #include "bouncebuf.h" #include <halt.h> #include "storage.h" +#include "sd_mmc.h" #include <string.h> #include <commonlib/stdlib.h>
diff --git a/src/commonlib/storage/pci_sdhci.c b/src/commonlib/storage/pci_sdhci.c index 2bba084..329e069 100644 --- a/src/commonlib/storage/pci_sdhci.c +++ b/src/commonlib/storage/pci_sdhci.c @@ -22,9 +22,12 @@ #include <commonlib/sdhci.h> #include <device/pci.h> #include <device/pci_ops.h> +#include <device/pci_ids.h> #include "sd_mmc.h" #include <stdint.h> #include "storage.h" +#include "sdhci.h" +#include <delay.h>
/* Initialize an SDHCI port */ int sdhci_controller_init(struct sdhci_ctrlr *sdhci_ctrlr, void *ioaddr) @@ -54,9 +57,59 @@ return car_get_var_ptr(&sdhci_ctrlr.sd_mmc_ctrlr); }
+static void bh720_tuning_start(struct sd_mmc_ctrlr *ctrlr, int retune) +{ + /* switch to 4bit, for bh720 didn't support 8bit tuning */ + u16 host_ctrl2, vendor_spec; + struct sdhci_ctrlr *sdhci_ctrlr = (struct sdhci_ctrlr *)ctrlr; + + host_ctrl2 = sdhci_readw(sdhci_ctrlr, SDHCI_HOST_CONTROL2); + host_ctrl2 &= ~(SDHCI_CTRL_8BITBUS); + host_ctrl2 |= (SDHCI_CTRL_4BITBUS); + sdhci_writew(sdhci_ctrlr, SDHCI_HOST_CONTROL2, host_ctrl2); + + /* set tuning mode to hw tuning */ + vendor_spec = sdhci_readw(sdhci_ctrlr, SDHCI_VENDOR_SPEC_CTRL); + vendor_spec &= ~SDHCI_VENDOR_HW_TUNING; + sdhci_writew(sdhci_ctrlr, vendor_spec, SDHCI_VENDOR_SPEC_CTRL); + + /* Start the bus tuning */ + host_ctrl2 = sdhci_readw(sdhci_ctrlr, SDHCI_HOST_CONTROL2); + host_ctrl2 &= ~SDHCI_CTRL_TUNED_CLK; + host_ctrl2 |= (retune ? SDHCI_CTRL_TUNED_CLK : 0) + | SDHCI_CTRL_EXEC_TUNING; + sdhci_writew(sdhci_ctrlr, host_ctrl2, SDHCI_HOST_CONTROL2); +} + +static int bh720_is_tuning_complete(struct sd_mmc_ctrlr *ctrlr, int *successful) +{ + /* if tuning complete, switch to 8bit */ + int index; + u16 regval; + struct sdhci_ctrlr *sdhci_ctrlr = (struct sdhci_ctrlr *)ctrlr; + + for (index = 0; index < 1000; index++) { + regval = sdhci_readw(sdhci_ctrlr, SDHCI_HOST_CONTROL2); + if ((regval & SDHCI_CTRL_EXEC_TUNING) == 0) { + regval = sdhci_readw(sdhci_ctrlr, SDHCI_HOST_CONTROL2); + *successful = ((regval & SDHCI_CTRL_TUNED_CLK) != 0); + + /*switch back to 8bit after tuning complete*/ + regval |= (SDHCI_CTRL_8BITBUS); + regval &= ~(SDHCI_CTRL_4BITBUS); + sdhci_writew(sdhci_ctrlr, SDHCI_HOST_CONTROL2, regval); + return 1; + } + else + udelay(100); + } while (1); +} + struct sd_mmc_ctrlr *new_pci_sdhci_controller(uint32_t dev) { uint32_t addr; + uint32_t vendor_id, device_id; + struct sd_mmc_ctrlr *sd_mmc_ctrlr;
addr = pci_read_config32(dev, PCI_BASE_ADDRESS_0); if (addr == ((uint32_t)~0)) { @@ -65,5 +118,15 @@ }
addr &= ~0xf; - return new_mem_sdhci_controller((void *)addr); + sd_mmc_ctrlr = new_mem_sdhci_controller((void *)addr); + + vendor_id = pci_read_config32(dev, PCI_VENDOR_ID); + device_id = pci_read_config32(dev, PCI_DEVICE_ID); + if (vendor_id == PCI_VENDOR_ID_O2 && + device_id == PCI_DEVICE_ID_O2_BH720) { + sd_mmc_ctrlr->tuning_start = bh720_tuning_start; + sd_mmc_ctrlr->is_tuning_complete = bh720_is_tuning_complete; + } + + return sd_mmc_ctrlr; } diff --git a/src/commonlib/storage/sdhci.h b/src/commonlib/storage/sdhci.h index ed8984a..ea0c501 100644 --- a/src/commonlib/storage/sdhci.h +++ b/src/commonlib/storage/sdhci.h @@ -217,6 +217,9 @@ #define SDHCI_SPEC_200 1 #define SDHCI_SPEC_300 2
+#define SDHCI_VENDOR_SPEC_CTRL 0x110 +#define SDHCI_VENDOR_HW_TUNING 0x10 + /* * End of controller registers. */ diff --git a/src/drivers/generic/bayhub/bh720.h b/src/drivers/generic/bayhub/bh720.h index 3183bf1..03da99e 100644 --- a/src/drivers/generic/bayhub/bh720.h +++ b/src/drivers/generic/bayhub/bh720.h @@ -41,6 +41,8 @@ BH720_PCR_DrvStrength_PLL = 0x304, BH720_PCR_DATA_CMD_DRV_MAX = 7, BH720_PCR_CLK_DRV_MAX = 7, + BH720_PCR_PLL_MASK = 0xFFFF0000, + BH720_PCR_PLL_200M = 0x25100000, BH720_PCR_EMMC_SETTING = 0x308, BH720_PCR_EMMC_SETTING_1_8V = BIT(4),
diff --git a/src/mainboard/google/kahlee/variants/baseboard/mainboard.c b/src/mainboard/google/kahlee/variants/baseboard/mainboard.c index 518d457..ba3bf96 100644 --- a/src/mainboard/google/kahlee/variants/baseboard/mainboard.c +++ b/src/mainboard/google/kahlee/variants/baseboard/mainboard.c @@ -61,12 +61,20 @@ write32((void *)(sdbar + BH720_MEM_RW_ADR), BH720_MEM_RW_WRITE | BH720_PCR_EMMC_SETTING);
- /* Set Bayhub SD base CLK 50MHz: case#1 PCR 0x3E4[22] = 0 */ + /* Set Bayhub SD base CLK 200MHz */ + write32((void *)(sdbar + BH720_MEM_RW_ADR), + BH720_MEM_RW_READ | BH720_PCR_DrvStrength_PLL); + bh720_pcr_data = read32((void *)(sdbar + BH720_MEM_RW_DATA)); + write32((void *)(sdbar + BH720_MEM_RW_DATA), + (bh720_pcr_data & ~BH720_PCR_PLL_MASK) | BH720_PCR_PLL_200M); + write32((void *)(sdbar + BH720_MEM_RW_ADR), + BH720_MEM_RW_WRITE | BH720_PCR_DrvStrength_PLL); + write32((void *)(sdbar + BH720_MEM_RW_ADR), BH720_MEM_RW_READ | BH720_PCR_CSR); bh720_pcr_data = read32((void *)(sdbar + BH720_MEM_RW_DATA)); write32((void *)(sdbar + BH720_MEM_RW_DATA), - bh720_pcr_data & ~BH720_PCR_CSR_EMMC_MODE_SEL); + bh720_pcr_data | BH720_PCR_CSR_EMMC_MODE_SEL); write32((void *)(sdbar + BH720_MEM_RW_ADR), BH720_MEM_RW_WRITE | BH720_PCR_CSR);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31772 )
Change subject: Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31772/1/src/commonlib/storage/pci_sdhci.c File src/commonlib/storage/pci_sdhci.c:
https://review.coreboot.org/#/c/31772/1/src/commonlib/storage/pci_sdhci.c@10... PS1, Line 103: else else should follow close brace '}'
Richard Spiegel has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/31772 )
Change subject: Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz ......................................................................
Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz
Currently bayhub BH7720 eMMC is set to internal clock of 50MHz. This is because some eMMC device don't work at 200 MHz without clock tuning. This was never intended as permanent code, as it also slowed eMMC after OS loaded because internal clock remained at 50MHz. In order to solve this issue, code must be added to execute clock tuning, and BH720_PCR_CSR_EMMC_MODE_SEL bit must be set, not cleared (to revert clock back to 200 MHz).
BUG=b:122244718 TEST=Built grunt without code, boot/reboot, measured eMMC time, saved results. Added code, rebuilt grunt, boot/reboot, measured eMMC time. Time saved: 123.5 milliseconds.
Change-Id: I8aefb524a76ab469e776f2f22aed80b29b67ae8e Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- M src/commonlib/storage/bouncebuf.c M src/commonlib/storage/pci_sdhci.c M src/commonlib/storage/sdhci.h M src/drivers/generic/bayhub/bh720.h M src/mainboard/google/kahlee/variants/baseboard/mainboard.c 5 files changed, 79 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/31772/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31772 )
Change subject: Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c File src/commonlib/storage/pci_sdhci.c:
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c@10... PS2, Line 102: } else else is not generally useful after a break or return
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31772 )
Change subject: Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c File src/commonlib/storage/pci_sdhci.c:
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c@60 PS2, Line 60: bh720_tuning_start the bayhub functions should go into the bayhub directory.
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c@10... PS2, Line 102: else Please put braces around the udelay so that both sides of the else match. Checkpatch might not like it, but sometimes checkpatch is stupid.
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c@12... PS2, Line 121: after you move the functions above, this should get protected by a config option so that it only gets called if the bayhub part is enabled.
https://review.coreboot.org/#/c/31772/2/src/mainboard/google/kahlee/variants... File src/mainboard/google/kahlee/variants/baseboard/mainboard.c:
https://review.coreboot.org/#/c/31772/2/src/mainboard/google/kahlee/variants... PS2, Line 64: /* Set Bayhub SD base CLK 200MHz */ Put this change in a separate commit please.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31772 )
Change subject: Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c File src/commonlib/storage/pci_sdhci.c:
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c@60 PS2, Line 60: bh720_tuning_start
the bayhub functions should go into the bayhub directory.
Will do.
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c@10... PS2, Line 102: else
Please put braces around the udelay so that both sides of the else match. […]
Will do, though I believe it should be 2 commits. Create (move) bayhub functions, use bayhub functions.
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/pci_sdhci.c@12... PS2, Line 121:
after you move the functions above, this should get protected by a config option so that it only get […]
Agree, will do.
https://review.coreboot.org/#/c/31772/2/src/mainboard/google/kahlee/variants... File src/mainboard/google/kahlee/variants/baseboard/mainboard.c:
https://review.coreboot.org/#/c/31772/2/src/mainboard/google/kahlee/variants... PS2, Line 64: /* Set Bayhub SD base CLK 200MHz */
Put this change in a separate commit please.
Will do.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31772 )
Change subject: Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/bouncebuf.c File src/commonlib/storage/bouncebuf.c:
https://review.coreboot.org/#/c/31772/2/src/commonlib/storage/bouncebuf.c@22 PS2, Line 22: #include "sd_mmc.h" Just found out... not needed, will be removed.
Richard Spiegel has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31772 )
Change subject: Grunt platforms: Change bayhub BH720 eMMC speed to 200 MHz ......................................................................
Abandoned
Raul Rangel has solved the tuning problem, and thus also submitted a patch to change eMMC speed.
-
build bot (Jenkins) (Code Review) -
Martin Roth (Code Review)
-
Richard Spiegel (Code Review)