From mboxrd@z Thu Jan 1 00:00:00 1970 From: zhangfei Subject: Re: [PATCHv4] mmc: dw_mmc: Enable the hold reg for certain speed modes Date: Tue, 17 Dec 2013 16:11:35 +0800 Message-ID: <52B00737.6050701@linaro.org> References: <1387213279-22020-1-git-send-email-dinguyen@altera.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail-pa0-f54.google.com ([209.85.220.54]:59423 "EHLO mail-pa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752660Ab3LQILm (ORCPT ); Tue, 17 Dec 2013 03:11:42 -0500 Received: by mail-pa0-f54.google.com with SMTP id rd3so4175373pab.27 for ; Tue, 17 Dec 2013 00:11:42 -0800 (PST) In-Reply-To: <1387213279-22020-1-git-send-email-dinguyen@altera.com> Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: dinguyen@altera.com, dinh.linux@gmail.com, arnd@arndb.de, cjb@laptop.org, jh80.chung@samsung.com, tgih.jun@samsung.com, heiko@sntech.de, dianders@chromium.org, alim.akhtar@samsung.com, bzhao@marvell.com Cc: linux-mmc@vger.kernel.org On 12/17/2013 01:01 AM, dinguyen@altera.com wrote: > From: Dinh Nguyen > > 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 > Acked-by: Arnd Bergmann > Acked-by: Heiko Stuebner > Tested-by: Heiko Stuebner > --- > 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? Thanks