Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63311 )
Change subject: herobrine: fix emmc and sd card clocks ......................................................................
herobrine: fix emmc and sd card clocks
Found an issue where emmc and sd clocks were being misconfigured due to using incorrect integer values when called instead of the defined enums. Fixing by splitting the clock_configure_sdcc() function into two (sdcc1 and sdcc2) as there was no commonality between the two cases anyway. As a result, we can also get rid of the clk_sdcc enum.
BUG=b:198627043 BRANCH=None TEST=build herobrine image and test in conjunction with CB:63289 make sure assert is not thrown.
Change-Id: I68f9167499ede057922135623a4b04202f4da9b5 Signed-off-by: Shelley Chen shchen@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/63311 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/mainboard/google/herobrine/mainboard.c M src/soc/qualcomm/sc7280/clock.c M src/soc/qualcomm/sc7280/include/soc/clock.h 3 files changed, 42 insertions(+), 45 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/mainboard/google/herobrine/mainboard.c b/src/mainboard/google/herobrine/mainboard.c index 7bdede0..c8af686 100644 --- a/src/mainboard/google/herobrine/mainboard.c +++ b/src/mainboard/google/herobrine/mainboard.c @@ -50,9 +50,9 @@ static void mainboard_init(struct device *dev) { /* Configure clock for eMMC */ - clock_configure_sdcc(1, 384 * MHz); + clock_configure_sdcc1(384 * MHz); /* Configure clock for SD card */ - clock_configure_sdcc(2, 50 * MHz); + clock_configure_sdcc2(50 * MHz); configure_sdhci();
gpi_firmware_load(QUP_0_GSI_BASE); diff --git a/src/soc/qualcomm/sc7280/clock.c b/src/soc/qualcomm/sc7280/clock.c index 746035c..e321e8d 100644 --- a/src/soc/qualcomm/sc7280/clock.c +++ b/src/soc/qualcomm/sc7280/clock.c @@ -291,45 +291,46 @@ } }
-void clock_configure_sdcc(enum clk_sdcc sdcc, uint32_t hz) +void clock_configure_sdcc1(uint32_t hz) { - if (sdcc == SDCC1_CLK) { - if (hz > CLK_100MHZ) { - struct alpha_pll_reg_val_config gpll10_cfg = {0}; - gpll10_cfg.reg_mode = &gcc->gpll10.mode; - gpll10_cfg.reg_opmode = &gcc->gpll10.opmode; - gpll10_cfg.reg_l = &gcc->gpll10.l; - gpll10_cfg.l_val = 0x14; - gpll10_cfg.reg_cal_l = &gcc->gpll10.cal_l; - gpll10_cfg.cal_l_val = 0x44; - gpll10_cfg.fsm_enable = true; - gpll10_cfg.reg_apcs_pll_br_en = &gcc->apcs_pll_br_en; - clock_configure_enable_gpll(&gpll10_cfg, true, 9); - } - clock_configure((struct clock_rcg *)&gcc->sdcc1, sdcc1_core_cfg, - hz, ARRAY_SIZE(sdcc1_core_cfg)); - clock_enable(&gcc->sdcc1_ahb_cbcr); - clock_enable(&gcc->sdcc1_apps_cbcr); - } else if (sdcc == SDCC2_CLK) { - if (hz > CLK_100MHZ) { - struct alpha_pll_reg_val_config gpll9_cfg = {0}; - gpll9_cfg.reg_mode = &gcc->gpll9.mode; - gpll9_cfg.reg_opmode = &gcc->gpll9.opmode; - gpll9_cfg.reg_alpha = &gcc->gpll9.alpha; - gpll9_cfg.alpha_val = 0x1555; - gpll9_cfg.reg_l = &gcc->gpll9.l; - gpll9_cfg.l_val = 0x2A; - gpll9_cfg.reg_cal_l = &gcc->gpll9.cal_l; - gpll9_cfg.cal_l_val = 0x44; - gpll9_cfg.fsm_enable = true; - gpll9_cfg.reg_apcs_pll_br_en = &gcc->apcs_pll_br_en; - clock_configure_enable_gpll(&gpll9_cfg, true, 8); - } - clock_configure((struct clock_rcg *)&gcc->sdcc2, sdcc2_core_cfg, - hz, ARRAY_SIZE(sdcc2_core_cfg)); - clock_enable(&gcc->sdcc2_ahb_cbcr); - clock_enable(&gcc->sdcc2_apps_cbcr); + if (hz > CLK_100MHZ) { + struct alpha_pll_reg_val_config gpll10_cfg = {0}; + gpll10_cfg.reg_mode = &gcc->gpll10.mode; + gpll10_cfg.reg_opmode = &gcc->gpll10.opmode; + gpll10_cfg.reg_l = &gcc->gpll10.l; + gpll10_cfg.l_val = 0x14; + gpll10_cfg.reg_cal_l = &gcc->gpll10.cal_l; + gpll10_cfg.cal_l_val = 0x44; + gpll10_cfg.fsm_enable = true; + gpll10_cfg.reg_apcs_pll_br_en = &gcc->apcs_pll_br_en; + clock_configure_enable_gpll(&gpll10_cfg, true, 9); } + clock_configure((struct clock_rcg *)&gcc->sdcc1, sdcc1_core_cfg, + hz, ARRAY_SIZE(sdcc1_core_cfg)); + clock_enable(&gcc->sdcc1_ahb_cbcr); + clock_enable(&gcc->sdcc1_apps_cbcr); +} + +void clock_configure_sdcc2(uint32_t hz) +{ + if (hz > CLK_100MHZ) { + struct alpha_pll_reg_val_config gpll9_cfg = {0}; + gpll9_cfg.reg_mode = &gcc->gpll9.mode; + gpll9_cfg.reg_opmode = &gcc->gpll9.opmode; + gpll9_cfg.reg_alpha = &gcc->gpll9.alpha; + gpll9_cfg.alpha_val = 0x1555; + gpll9_cfg.reg_l = &gcc->gpll9.l; + gpll9_cfg.l_val = 0x2A; + gpll9_cfg.reg_cal_l = &gcc->gpll9.cal_l; + gpll9_cfg.cal_l_val = 0x44; + gpll9_cfg.fsm_enable = true; + gpll9_cfg.reg_apcs_pll_br_en = &gcc->apcs_pll_br_en; + clock_configure_enable_gpll(&gpll9_cfg, true, 8); + } + clock_configure((struct clock_rcg *)&gcc->sdcc2, sdcc2_core_cfg, + hz, ARRAY_SIZE(sdcc2_core_cfg)); + clock_enable(&gcc->sdcc2_ahb_cbcr); + clock_enable(&gcc->sdcc2_apps_cbcr); }
void clock_configure_dfsr(int qup) diff --git a/src/soc/qualcomm/sc7280/include/soc/clock.h b/src/soc/qualcomm/sc7280/include/soc/clock.h index 744734e..d22ba37 100644 --- a/src/soc/qualcomm/sc7280/include/soc/clock.h +++ b/src/soc/qualcomm/sc7280/include/soc/clock.h @@ -272,11 +272,6 @@ int vote_bit; };
-enum clk_sdcc { - SDCC1_CLK, - SDCC2_CLK, -}; - enum clk_qup { QUP_WRAP0_S0, QUP_WRAP0_S1, @@ -385,7 +380,8 @@ void clock_init(void); void clock_configure_qspi(uint32_t hz); void clock_enable_qup(int qup); -void clock_configure_sdcc(enum clk_sdcc, uint32_t hz); +void clock_configure_sdcc1(uint32_t hz); +void clock_configure_sdcc2(uint32_t hz); void clock_configure_dfsr(int qup); int clock_enable_gdsc(enum clk_gdsc gdsc_type);