On Wed, 2016-01-27 at 00:01 +0000, Trent Piepho wrote:
> On Fri, 2015-11-27 at 15:22 +0800, Chin Liang See wrote:
> > Enable SDMMC calibration to determine the best setting for
> > drvsel and smplsel. Calibration will be triggered if the
> > drvsel and smplsel node are not available in DTS.
>
> We have a Cyclone V based board and on the latest revision the SD
> card
> interface has become unreliable at 50 MHz. The SD card moved a few
> inches further away from the SoC in this revision. I though maybe
> smplsel could be an issue and found this patch.
>
> I ported this to our board, but found that this algorithm doesn't
> appear
> to work that well for us.
>
> In my testing I found the fastest way to produce SD errors on Linux
> was
> to repeatedly issue two sector read multiple commands. This produces
> about one error per 10,000 commands. Usually a CRC error on the
> response but also data CRC errors and data stop bit errors.
>
> This calibration code tests by using a single set block length
> command
> to check if the combination of drvsel and smplsel is good. Obviously
> if
> 0.01% of commands fail then checking via the result of one command is
> nearly useless. And indeed this is the case, every cal test passes
> and
> one gets a table of all trues.
Do note that these cal value are not passed to Linux. Wonder your Linux
is using the cal value as obtained from U-Boot calibration?
>
> It is however worse than that. Some cal settings are clearly bad.
> For
> instance, drvsel 3 smplsel 7 does not work at all on our board.
> Every
> read command tried with those settings will fail. But the set block
> length command succeeds. Perhaps because it neither sends nor
> receives
> any data?
Per SD specifications, set block length (CMD16) will expect response R1
which is normal response from the card.
> So the calibration seems unlikely to improve anything, since
> it will always choose 3, 3 for the settings. The basic premise, that
> it
> can tell if a pair of settings work or not, appears to not be true.
Wonder any difference if you change the value in DTS (without the
calibration code)? I am referring U-Boot accessing the SD card.
>
> > +static int socfpga_dwmci_find_row_col_fit_rectangle(unsigned
> > rect_width,
> > + unsigned rect_height,
> > + unsigned char
> > cal_results[SOCFPGA_SD_DRVSEL][SOCFPGA_SD_SMPLSEL],
> > + unsigned int *cal_row, unsigned int *cal_col)
>
> There is no documentation of what exactly parameters are here. But
> one
> can determine that cal_row will contain the zero based index of the
> "best" row.
>
Oh there is a link at the function's comment to explain the algo.
>
> > + for (row = 0; row < SOCFPGA_SD_DRVSEL; row++) {
> > + for (col = 0; col < SOCFPGA_SD_SMPLSEL; col++) {
> > + priv->drvsel = row + 1;
> > + priv->smplsel = col;
>
> Here drvsel is set to row + 1.
This is also part of comment. In quick, we are seeing meta stability
issue when drvsel = 0 and that why we skip it
>
> > +
> > + err = socfpga_dwmci_find_calibration_point(cal_results,
> > sum,
> > + &priv->drvsel,
> > + &priv
> > ->smplsel);
>
> But this function returns the zero based row number (I think it would
> be
> easier to notice if it were documented) in priv->drvsel. Not row +
> 1.
> It seems that you will always program drvsel to be 1 smaller than it
> should be.
Same as previous comment
Thanks
Chin Liang
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
