Hi Jaehoon,
On 12/25/13 8:57 PM, Jaehoon Chung wrote:
> On 12/17/2013 11:54 PM, zhangfei wrote:
>>
>> On 12/17/2013 10:03 PM, Dinh Nguyen wrote:
>>> 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.
>>>
>> Hmm, ususally I may look for the properties of dw_mmc.c in synopsys-dw-mshc.txt.
>> If this is the conclusion before, then just ignore this noise.
> If can be removed the samsung property, then i think property of clock timing can be used into dw-mmc.c
> But if samsung property is used, well. I think right that it's used into dw_mmc-exynos.c.
> Dw-mmc.c is general driver..so we don't want to include any SoC specific code.
Then do you suggest I go forward with an attempt to add a new generic
"snps,dw-mshc-sdr-timing"
binding?
Dinh
>
> If i missed something, then let me know, plz.
>
> Best Regards,
> Jaehoon Chung
>> 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
