Hi Zhangfei, On 12/17/13 2:11 AM, zhangfei wrote: > > > On 12/17/2013 01:01 AM, dinguyen@xxxxxxxxxx wrote: >> From: Dinh Nguyen <dinguyen@xxxxxxxxxx> >> >> This patch will enable the SDMMC_CMD_USE_HOLD_REG bit when the slot is >> operating all timing modes, except for SDR50, DDR50, SDR104, and >> MMC_HS200. >> >> According to the Synopsys databook :"To meet the relatively high >> Input Hold >> Time requirement for SDR12, SDR25, and other MMC speed modes, you should >> program bit[29]use_hold_Reg of the CMD register to 1'b1;"..."However, >> for >> the higher speed modes of SDR104, SDR50 and DDR50, you can meet the much >> smaller Input Hold Time requirement of 0.8ns by bypassing the Hold >> Register >> (Path A in Figure 10-8, programming CMD.use_hold_reg = 1'b0) and then >> adding >> delay elements on the output path as indicated. >> >> Also, "Never set CMD.use_hold_reg = 1 and cclk_in_drv phase shift to >> 0 at the >> same time. This would add an extra one-cycle delay on the output >> path, resulting >> in incorrect behavior." >> >> This patch also checks the IHR(Implement Hold Register) in the HCON >> register. >> >> This information is taking from the v2.50a of the Synopsys Designware >> Cores >> Mobile Storage Host Databook. >> >> Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxx> >> Acked-by: Arnd Bergmann <arnd@xxxxxxxx> >> Acked-by: Heiko Stuebner <heiko@xxxxxxxxx> >> Tested-by: Heiko Stuebner <heiko@xxxxxxxxx> >> --- >> v4: use_hold_reg should be set for all modes when cclk_in_drv is >> non-zero. >> v3: Read the IHR(Implement Hold Register) in the HCON >> v2: Add check for cclk_in_drv phase shift in conjunction with >> use_hold_reg. >> --- >> drivers/mmc/host/dw_mmc.c | 52 >> ++++++++++++++++++++++++++++++++++++++++++++ >> drivers/mmc/host/dw_mmc.h | 4 ++++ >> include/linux/mmc/dw_mmc.h | 3 +++ >> 3 files changed, 59 insertions(+) >> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> /* >> * Use mirror of ios->clock to prevent race with mmc >> * core ios update when finding the minimum. >> @@ -2339,6 +2361,8 @@ static struct dw_mci_board >> *dw_mci_parse_dt(struct dw_mci *host) >> const struct dw_mci_drv_data *drv_data = host->drv_data; >> int idx, ret; >> u32 clock_frequency; >> + int sdr_timing[2]; >> + int ddr_timing[2]; >> >> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >> if (!pdata) { >> @@ -2389,6 +2413,25 @@ static struct dw_mci_board >> *dw_mci_parse_dt(struct dw_mci *host) >> if (of_find_property(np, "caps2-mmc-hs200-1_2v", NULL)) >> pdata->caps2 |= MMC_CAP2_HS200_1_2V_SDR; >> >> + /* Check for the "samsung,dw-mshc-sdr-timing" and the >> + * "samsung,dw-mshc-ddr-timing" bindings as this will tell us if we >> + * can safely set the SDMMC_CMD_USE_HOLD_REG bit. The second >> paramater >> + * in these 2 bindings is the value of the cclk_in_drv. If >> cclk_in_drv >> + * is 0, we cannot set the SDMMC_CMD_USE_HOLD_REG bit. The default >> + * behavior will be to set cclk_in_drv, as some platforms do not >> have >> + * to set the sdr or ddr timing parameters. >> + */ >> + sdr_timing[1] = ddr_timing[1] = 1; >> + of_property_read_u32_array(np, >> + "samsung,dw-mshc-sdr-timing", sdr_timing, 2); >> + >> + of_property_read_u32_array(np, >> + "samsung,dw-mshc-ddr-timing", ddr_timing, 2); >> + >> + pdata->cclk_in_drv = 1; >> + if ((sdr_timing[1] == 0) || (ddr_timing[1] == 0)) >> + pdata->cclk_in_drv = 0; >> + > > Have some concern about whether it is suitable putting "samsung,~" > property in dw_mmc.c, is it supposed to be platform related? > Any conflict with drivers/mmc/host/dw_mmc-exynos.c? > If they are really commonly used, how about change name and define in > Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt? I had submitted a patch to make this a common binding before: http://www.spinics.net/lists/devicetree/msg00638.html I think the ultimate conclusion to that thread was that its perfectly acceptable to re-use bindings from other platforms. Dinh > > Thanks -- 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