Hi Marek,

On Tuesday, September 08, 2015 at 03:32:33 AM, Chin Liang See wrote:

...

Hi,

Hi,

...

On Mon, 2015-09-07 at 03:33 +0000, Jaehoon Chung wrote:

...

Hi,

On 09/04/2015 07:41 PM, Pavel Machek wrote:

...

Hi!

...

...

>>>> How is this SMPLSEL and DRVSEL implemented on Exynos ? >> >> Exynos is using CLKSEL register in dw-mmc controller. >> It's exynos specific register in dwmmc controller. It's also >> represented 45 degree increment. SELCK_DRV is bit[18:16] or more. >> SELCLK_SAMPLE is bit[2:0] or more. There are other bits >> relevant to tuning clock. '_more_' means that it can be changed bandwidth. >> >> Anyway, I think there is no right method about finding the >> best smplclk and drvsel. If this is generic method, i will pick this. >> But i don't think so, and there is no benefit for exynos. >> >> smplclk and drvsel value need to process the tuning sequence. >> There is no tuning case at bootloader, since it's not >> implemented about HS200 or upper mode. >> >> Clksel an drvsel value are passed by device tree. > > In that case, maybe SoCFPGA should also pick those values from DT ? > It would keep the code simple and in case there is a > problematic board, it could use u-boot application to perform the tuning.

I prefer not to do that as it narrows the supported use case for the driver.

How so? It keeps the driver code clean and this code you're adding seems like a special-purpose stuff which needs to be done once for particular board, no ?

Well... stuff that can be automatically detected is not supposed to be in the device tree.

clksel and drvsel can be calibrated, so I see some arguments why we should calibrate them, and not hardcode them in the device tree.

My opinions are

1. This code is not generic dwmmc code. So i don't want to locate

into dwmmc core. If need to apply, i agree that it applies this in socfpga-dw_mmc.c.

Yup, let move that to socfpga-dw_mmc.c as majority believe that is correct way to move forward. I shall send in v3 which integrate Pavel's comment in v2

...

2. In exynos, value of devcie-tree is the tested value.

After has tested with every values, it defined the best value into device-tree. (Working fine with values.) At every time, it doesn't need to detect the best value with same SoC. (Especially, at bootloader)

3. In my experiment, there should be side-effect during finding best

sample/drv value.

4. If HS200 or upper mode is supported at bootloader, it needs the

tuning sequence. Then it needs to find the best sampl/drv values. but it doesn't support HS200 or other at bootloader.

Yah, normally it will not need that. As we are catering for various customer board design, this calibration will help even they routed the SDMMC slot few inches away.

...

5. Affect at booting time??

We measured this before and it took around ~5.2ms for worst case.

Hm, what about supporting both the DT variant (where you specify drvsel and smplsel in DT, the way we do it in Linux) and this calibration variant. The calibration would only be executed if the smplsel and drvsel args were missing from the DT.

What do you think ?

Sorry as missing out this email. Yah, this sound good to me where user can use DT variant if concern about the boot time. Let me create that and sending in new revision.

Thanks Chin Liang