Messages in this thread | | | Date | Fri, 21 Jul 2023 01:45:01 +0800 | Subject | Re: [PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue | From | Ziyang Huang <> |
| |
在 2023/6/20 3:51, Martin Blumenstingl 写道: > Hello, > > first of all: thank you for this patch! > > On Mon, Jun 19, 2023 at 7:36 PM Ziyang Huang <hzyitc@outlook.com> 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. > I think this is the exact same problem I reported some days ago: [0] > Ulf is questioning whether we properly support 52MHz clocks correctly, > so I think you're onto something! > > So this is an excellent finding! I can confirm that using rx_clk_phase > of 1 makes my Odroid-C1 eMMC work again :-) > >> So let's set a common value - 1 just for initialization. Then let >> meson_mx_sdhc_execute_tuning() to find the accurate value for data >> transfer. > As far as I know unconditionally using value 1 can negatively affect > other devices. > I'm assuming that you're testing on an Odroid-C1 or similar board with > HS200 eMMC: > On those SoC + eMMC combinations we do support. But on other boards > (for example Meson8b EC-100 / Endless Mini) there's no HS200 support > because the eMMC is connected with 3.3V IO lines. So tuning is not > executed there (if I recall correctly).
Sorry for the later reply. I'm so busy these day.
After checking the code, I found the following flow, so I think tuningshould work for all cards.
mmc_start_request() -> __mmc_start_request() -> mmc_retune() -> mmc_execute_tuning() -> host->ops->execute_tuning()
And yes, 1 may be not a good choice. I consider 3 values: - 1 - div_val / 2 - div_val - 1
Maybe, (div_val / 2) is a good choice. How do you think?
> What do you think about adding a special case for the 51MHz "actual > clock rate" and adding a comment that it was found by manual testing? > For some reason (that I don't understand) Amlogic's vendor driver > maxes out at 47.22MHz (presumably because they limit themselves to > using FCLK_DIV3 as input only - but I don't get why...).
With some modifications, I found that the mmc controller of S805 can work with all frequency, not only those mentioned in commit e4bf1b0970ef("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host"). It seem like that every things work fine. I will use a oscilloscope to confirm the hardware clock is correct when I have time.
For this future commit, I don't want to add a special case.
> Best regards, > Martin > > > [0] https://lore.kernel.org/linux-amlogic/CAFBinCD0RT0p-jk86W0JuMT3ufohRh1RqWCcM35DKZJpuc10HQ@mail.gmail.com/ > [1] https://lore.kernel.org/linux-amlogic/CAPDyKFpS-UwiaRPMqSpX0mNPrS5p=yJzu3g0=pGyCkWHSYyqWg@mail.gmail.com/
| |