On 23.10.23 13:14, Ulf Hansson wrote: > On Tue, 10 Oct 2023 at 18:44, Ziyang Huang <hzyitc@xxxxxxxxxxx> wrote: >> >> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending >> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this >> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9). >> Then we set rx_clk_phase to 11 or 15 which is out of range and make >> hardware frozen. After we send command request, no irq will be >> interrupted and the mmc driver will keep to wait for request finished, >> even durning rebooting. >> >> So let's set it to Phase 90 which should work in most cases. Then let >> meson_mx_sdhc_execute_tuning() to find the accurate value for data >> transfer. >> >> If this doesn't work, maybe need to define a factor in dts. >> >> Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host") >> Signed-off-by: Ziyang Huang <hzyitc@xxxxxxxxxxx> > > I have re-added Thorsten to see if he has some time to test this on his end. FWIW, I was only involved wrt to regression tracking. It iirc was was Martin that had reported the problem here: https://lore.kernel.org/all/CAFBinCD0RT0p-jk86W0JuMT3ufohRh1RqWCcM35DKZJpuc10HQ@xxxxxxxxxxxxxx/ Side note: a proper Reported-by: tag together with a Link or Closes tag would be good to have in the patch descriptions, as explained in the Docs. Ciao, Thorsten >> --- >> Changes since v1: >> Use Phase 90 instand of value 1 >> >> drivers/mmc/host/meson-mx-sdhc-mmc.c | 26 +++++--------------------- >> 1 file changed, 5 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c >> index 97168cdfa8e9..29698fceb89c 100644 >> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c >> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c >> @@ -269,7 +269,7 @@ static int meson_mx_sdhc_enable_clks(struct mmc_host *mmc) >> static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios) >> { >> struct meson_mx_sdhc_host *host = mmc_priv(mmc); >> - u32 rx_clk_phase; >> + u32 val, rx_clk_phase; >> int ret; >> >> meson_mx_sdhc_disable_clks(mmc); >> @@ -290,27 +290,11 @@ static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios) >> mmc->actual_clock = clk_get_rate(host->sd_clk); >> >> /* >> - * according to Amlogic the following latching points are >> - * selected with empirical values, there is no (known) formula >> - * to calculate these. >> + * Phase 90 should work in most cases. For data transmission, >> + * meson_mx_sdhc_execute_tuning() will find a accurate value >> */ >> - if (mmc->actual_clock > 100000000) { >> - rx_clk_phase = 1; >> - } else if (mmc->actual_clock > 45000000) { >> - if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) >> - rx_clk_phase = 15; >> - else >> - rx_clk_phase = 11; >> - } else if (mmc->actual_clock >= 25000000) { >> - rx_clk_phase = 15; >> - } else if (mmc->actual_clock > 5000000) { >> - rx_clk_phase = 23; >> - } else if (mmc->actual_clock > 1000000) { >> - rx_clk_phase = 55; >> - } else { >> - rx_clk_phase = 1061; >> - } >> - >> + regmap_read(host->regmap, MESON_SDHC_CLKC, &val); >> + rx_clk_phase = FIELD_GET(MESON_SDHC_CLKC_CLK_DIV, val) / 4; >> regmap_update_bits(host->regmap, MESON_SDHC_CLK2, >> MESON_SDHC_CLK2_RX_CLK_PHASE, >> FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE, >> -- >> 2.34.1 >> >