On 1 June 2018 at 08:31, Y.b. Lu <yangbo.lu@xxxxxxx> wrote: >> -----Original Message----- >> From: Yinbo Zhu >> Sent: Monday, May 28, 2018 5:58 PM >> To: Adrian Hunter <adrian.hunter@xxxxxxxxx>; Y.b. Lu <yangbo.lu@xxxxxxx>; >> linux-mmc@xxxxxxxxxxxxxxx >> Cc: Xiaobo Xie <xiaobo.xie@xxxxxxx> >> Subject: RE: [PATCH v2] mmc: sdhci-of-esdhc: modify the sd clock in >> soc_device_match way >> >> >> >> -----Original Message----- >> From: Adrian Hunter [mailto:adrian.hunter@xxxxxxxxx] >> Sent: 2018年5月24日 16:58 >> To: Yinbo Zhu <yinbo.zhu@xxxxxxx>; Y.b. Lu <yangbo.lu@xxxxxxx>; >> linux-mmc@xxxxxxxxxxxxxxx >> Cc: Xiaobo Xie <xiaobo.xie@xxxxxxx> >> Subject: Re: [PATCH v2] mmc: sdhci-of-esdhc: modify the sd clock in >> soc_device_match way >> >> On 22/05/18 15:06, Yinbo Zhu wrote: > [...] >> > + switch (host->mmc->ios.timing) { >> > + case MMC_TIMING_LEGACY: >> > + fixup = esdhc->clk_fixup->ds; >> > + clock = esdhc_clock_fixup(clock, FULL_SPEED_MAX_DTR, fixup); >> > + fixup = esdhc->clk_fixup->mmc_ds; >> > + clock = esdhc_clock_fixup(clock, MMC_HIGH_26_MAX_DTR, fixup); >> > + break; >> > + case MMC_TIMING_SD_HS: >> > + fixup = esdhc->clk_fixup->hs; >> > + clock = esdhc_clock_fixup(clock, HIGH_SPEED_MAX_DTR, fixup); >> > + break; >> > + case MMC_TIMING_MMC_HS: >> > + fixup = esdhc->clk_fixup->mmc_hs; >> > + clock = esdhc_clock_fixup(clock, MMC_HIGH_52_MAX_DTR, fixup); >> > + break; >> > + case MMC_TIMING_UHS_SDR104: >> > + fixup = esdhc->clk_fixup->sdr104; >> > + clock = esdhc_clock_fixup(clock, UHS_SDR104_MAX_DTR, fixup); >> > + break; >> > + case MMC_TIMING_MMC_HS200: >> > + fixup = esdhc->clk_fixup->hs200; >> > + clock = esdhc_clock_fixup(clock, MMC_HS200_MAX_DTR, fixup); >> > + break; >> > + default: >> > + break; >> >> >This logic looks fragile. I would expect to see something that looks more >> like: >> >> > max_clk = esdhc->clk_fixup[host->mmc->ios.timing]; >> > if (max_clk && clock > max_clk) >> > clock = max_CLK; >> Hi Adrian Hunter, >> >> Thanks your advice, your way will make code logic more clearly, but MMC >> default max clock is 26MHz, SD default max clock is 25MHz, they all use the >> same ios.timing in public code, If use array, There will be a conflict, Do you >> have any good ways to resolve the conflict. >> Thanks. >> >> Regards, >> Yinbo >> > > [Y.b. Lu] The MMC core driver used the timing MMC_TIMING_LEGACY for both MMC and SD/SDIO at card initialization. > However, the timing was different between MMC and SD/SDIO at initialization. MMC was at legacy speed mode (up to 26MHz) while SD/SDIO was at default speed mode (up to 25MHz). > Actually I think we should define MMC_TIMING_MMC_LEGACY and MMC_TIMING_SD_DEFAULT for MMC and SD/SDIO initializing timing. > And use the right timing at the beginning in mmc_attach_sd/sdio/mmc(). > Any suggestion? Adrian and Uffe :) I am fine with that. However, you need to make sure all host drivers can cope with changing this. Perhaps, it's also sufficient to introduce MMC_TIMING_DEFAULT (used for SD) and keep MMC_TIMING_LEGACY (for MMC). > > #define MMC_TIMING_LEGACY 0 > #define MMC_TIMING_MMC_LEGACY 1 > #define MMC_TIMING_SD_DEFAULT 2 > #define MMC_TIMING_MMC_HS 3 > #define MMC_TIMING_SD_HS 4 > #define MMC_TIMING_UHS_SDR12 5 > #define MMC_TIMING_UHS_SDR25 6 > #define MMC_TIMING_UHS_SDR50 7 > #define MMC_TIMING_UHS_SDR104 8 > #define MMC_TIMING_UHS_DDR50 9 > #define MMC_TIMING_MMC_DDR52 10 > #define MMC_TIMING_MMC_HS200 11 > #define MMC_TIMING_MMC_HS400 12 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
