[U-Boot] [PATCH 1/2] arm: socfpga: mmc: Enable calibration for drvsel and smpsel

[Marek Vasut <marex@denx.de>]

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 ?