> -----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 :) #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 ��.n��������+%������w��{.n�����{��i��)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥