Re: [PATCH 3/3] ARM: dts: exynos5422-odroidxu3: Added UHS-I bus speed support

From: Krzysztof Kozlowski
Date: Tue Oct 13 2015 - 20:00:00 EST

On 14.10.2015 01:27, Anand Moon wrote:
> Hi Krzysztof,
>
> On 13 October 2015 at 09:13, Krzysztof Kozlowski
> <k.kozlowski@xxxxxxxxxxx> wrote:
>>
>> On 13.10.2015 12:08, Anand Moon wrote:
>>> Hi Krzysztof,
>>>
>>> On 13 October 2015 at 05:44, Krzysztof Kozlowski
>>> <k.kozlowski@xxxxxxxxxxx> wrote:
>>>> On 13.10.2015 00:32, Anand Moon wrote:
>>>>> Hi Krzysztof,
>>>>>
>>>>> On 12 October 2015 at 11:14, Krzysztof Kozlowski
>>>>> <k.kozlowski@xxxxxxxxxxx> wrote:
>>>>>> On 12.10.2015 00:46, Anand Moon wrote:
>>>>>>> Added support for UHS-I bus speed 50MB/s (SDR50, DDR50) 104MB/s (SDR104)
>>>>>>
>>>>>> This description is not entirely correct. The MMC driver already
>>>>>> supports these UHS speeds (you did not any code) so you rather enabled
>>>>>> it (description of bindings says "is supported").
>>>>>>
>>>>>> You mentioned DDR50 but I don't see respective property below.
>>>>>>
>>>>>> How do you know that these modes are really supported? I don't know. Can
>>>>>> you convince me?
>>>>>
>>>>> Setting this DDR50 capability give me this error. That's the reason to
>>>>> drop this capability.
>>>>
>>>> But you mentioned it in commit message! "Added support for UHS-I ...
>>>> (DDR50)"
>>>>
>>>> In the same time dropping DDR50 is not an sufficient proof that "SDR50
>>>> and SDR104 are really supported".
>>>>
>>>
>>> These changes are related to the microSD card capabilities.
>>> So SDR50 have better frequency over DDR50. On the same Sandisk card.
>>>
>>> When the card select the capability for DDR50
>>> ---------------------------------------------------
>>> [ 4.001477] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot
>>> req 50000000Hz, actual 50000000HZ div = 0)
>>> [ 4.001604] mmc1: new ultra high speed DDR50 SDHC card at address aaaa
>>> [ 4.004505] mmcblk0: mmc1:aaaa SL32G 29.7 GiB
>>> [ 4.009179] mmcblk0: error -110 sending status command, retrying
>>> [ 4.009271] mmcblk0: error -115 sending stop command, original cmd
>>> response 0x900, card status 0x900
>>> [ 4.009275] mmcblk0: error -84 transferring data, sector 0, nr 8,
>>> cmd response 0x900, card status 0x0
>>> [ 4.025563] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot
>>> req 400000Hz, actual 396825HZ div = 63)
>>> [ 4.067770] Console: switching to colour frame buffer device 274x77
>>> [ 4.098782] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot
>>> req 50000000Hz, actual 50000000HZ div = 0)
>>> [ 4.099692] mmc1: tried to reset card
>>> [ 4.101332] mmcblk0: p1 p2
>>>
>>>
>>> When the card select the capability for SDR50
>>> ---------------------------------------------------------------------------------
>>> [ 2.439806] mmc_host mmc1: Bus speed (slot 0) = 100000000Hz (slot req
>>> 100000000Hz, actual 100000000HZ div = 0)
>>> [ 2.449729] mmc1: new ultra high speed SDR50 SDHC card at address aaaa
>>> [ 2.455984] mmcblk0: mmc1:aaaa SL32G 29.7 GiB
>>> [ 2.461743] mmcblk0: p1 p2
>>>
>>> Which will relate to better read/write speed.
>>
>> Which is not an answer to my question. To none of my previous questions.
>>
>
> Basically UHS-I capability (sd-uhs-sdr12, sd-uhs-sdr25, sd-uhs-sdr50,
> sd-uhs-sdr104) help tune speed supported for mmc
>
> I have tired to compare the speed on high speed UHS-I vs ultra high
> speed UHS-I using izone utility.
>
> [ 2.572469] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot
> req 50000000Hz, actual 50000000HZ div = 0)
> [ 2.572609] mmc1: new high speed SDHC card at address aaaa
>
> Command line used: ./iozone -L64 -S32 -azecwI -+n -r4k -r64k
> -r128k -s10M -i0 -i1 -i2 -f datafile -Rb out.xls
> Output is in kBytes/sec
> Time Resolution = 0.000001 seconds.
> Processor cache size set to 32 kBytes.
> Processor cache line size set to 64 bytes.
> File stride size set to 17 * record size.
> random
> random bkwd record stride
> kB reclen write rewrite read reread read
> write read rewrite read fwrite frewrite fread
> freread
> 10240 4 1631 0 6556 0 5538 982
> 10240 64 8828 0 18897 0 17994 303
> 10240 128 6269 0 20670 0 20128 1096
> ---------------------------------------------------------------------------------------------------------
> [ 2.613761] mmc_host mmc1: Bus speed (slot 0) = 100000000Hz (slot
> req 100000000Hz, actual 100000000HZ div = 0)
> [ 2.623573] mmc1: new ultra high speed SDR50 SDHC card at address aaaa
>
> Command line used: ./iozone -L64 -S32 -azecwI -+n -r4k -r64k
> -r128k -s10M -i0 -i1 -i2 -f datafile -Rb out.xls
> Output is in kBytes/sec
> Time Resolution = 0.000001 seconds.
> Processor cache size set to 32 kBytes.
> Processor cache line size set to 64 bytes.
> File stride size set to 17 * record size.
> random
> random bkwd record stride
> kB reclen write rewrite read reread read
> write read rewrite read fwrite frewrite fread
> freread
> 10240 4 1809 0 7507 0 5233 859
> 10240 64 11622 0 31250 0 28072 516
> 10240 128 4320 0 34417 0 32509 1148
>
> My observation is that their slight increase in read/write operation.
>
> Hope I have tried to answer you query. If I am wrong please let me know.

Nope, that did not answer my query. You gave some performance benchmarks
but my question was not about the speed of anything. The question is
(once again):
How do you know that these modes are really supported?

You are marking the *host* as supporting these modes. Please provide
information that host supports them *really*, not by experimenting "oh,
it seems to work now, maybe it will work always".

Usually vendors, if their products implement some kind of
specification/protocol, they mark the products as "compatible with XYZ" etc.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/