Re: [PATCHv4] mmc: dw_mmc: Enable the hold reg for certain speed modes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux