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

[Jaehoon Chung <jh80.chung@samsung.com>]

Hi, Dinh.

Sorry for replying too late.

..[snip]..
>>>>>>> +    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?
> 
> Ping Jaehoon?
> 
> Do you think I need to add a generic "snps,dw-mshc-sdr-timing" and
> "snps,dw-mshc-ddr-timing" bindings then?

Well, i think it's also something wrong. ddr/sdr-timing is exynos specific value, not synopsys value.
If synopsys use the sdr/ddr timing, then it's right that "snps, dw-mshc-sdr/ddr-timing" is used.
But i didn't see sdr/ddr timing in synopsys DoC.
I know you want to control the hold-reg bit. 
But this approach is not good.
Rather, how about using the callback function for exynos specific value.
Then other SoC can also use it.

Best Regards,
Jaehoon Chung

> 
> Dinh
>>
>> Dinh
>>>
>>> If i missed something, then let me know, plz.
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>> Thanks
>>>>
>>
>>
> 
> 
> 
>