[U-Boot] [PATCH 1/2] arm: socfpga: mmc: Enable calibration for drvsel and smpsel
Chin Liang See
clsee at altera.com
Wed Aug 26 08:54:00 CEST 2015
On Wed, 2015-08-26 at 15:14 +0900, Jaehoon Chung wrote:
> On 08/26/2015 02:47 PM, Chin Liang See wrote:
> > On Wed, 2015-08-26 at 14:29 +0900, Jaehoon Chung wrote:
> >> Hi,
> >>
> >> On 08/25/2015 12:08 PM, Chin Liang See wrote:
> >>> On Tue, 2015-08-25 at 11:36 +0900, Jaehoon Chung wrote:
> >>>> Hi,
> >>>>
> >>>> On 08/25/2015 12:04 AM, Chin Liang See wrote:
> >>>>> On Fri, 2015-08-21 at 14:52 -0600, Simon Glass wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> On 20 August 2015 at 15:55, Dinh Nguyen <dinh.linux at gmail.com> wrote:
> >>>>>>> +CC: Simon Glass
> >>>>>>>
> >>>>>>> On Thu, Aug 20, 2015 at 12:32 AM, Marek Vasut <marex at denx.de> wrote:
> >>>>>>>> On Thursday, August 20, 2015 at 07:28:02 AM, Chin Liang See wrote:
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> On Wed, 2015-08-19 at 14:36 +0000, marex at denx.de wrote:
> >>>>>>>>>> On Wednesday, August 19, 2015 at 10:21:17 AM, Chin Liang See wrote:
> >>>>>>>>>>> Hi,
> >>>>>>>>>>
> >>>>>>>>>> Hi again,
> >>>>>>>>>>
> >>>>>>>>>>> On Wed, 2015-08-19 at 02:41 +0000, marex at denx.de wrote:
> >>>>>>>>>>>> On Wednesday, August 19, 2015 at 07:54:50 AM, Chin Liang See wrote:
> >>>>>>>>>>>>> Enable SDMMC calibration to determine the best setting for
> >>>>>>>>>>>>> drvsel and smpsel. It will be triggered whenever there is
> >>>>>>>>>>>>> a change of card frequency and bus width. This is to ensure
> >>>>>>>>>>>>> reliable transmission between the controller and the card.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Signed-off-by: Chin Liang See <clsee at altera.com>
> >>>>>>>>>>>>> Cc: Dinh Nguyen <dinguyen at opensource.altera.com>
> >>>>>>>
> >>>>>>> I think there's something wrong with your git scripts, I did not get this email.
> >>>>>>>
> >>>>>>>>>>>>> Cc: Pavel Machek <pavel at denx.de>
> >>>>>>>>>>>>> Cc: Marek Vasut <marex at denx.de>
> >>>>>>>>>>>>> Cc: Wolfgang Denk <wd at denx.de>
> >>>>>>>>>>>>> Cc: Stefan Roese <sr at denx.de>
> >>>>>>>>>>>>> Cc: Tom Rini <trini at konsulko.com>
> >>>>>>>>>>>>> ---
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> drivers/mmc/socfpga_dw_mmc.c | 187
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180
> >>>>>>>>>>>>> insertions(+), 7 deletions(-)
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> diff --git a/drivers/mmc/socfpga_dw_mmc.c
> >>>>>>>>>>>>> b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644
> >>>>>>>>>>>>> --- a/drivers/mmc/socfpga_dw_mmc.c
> >>>>>>>>>>>>> +++ b/drivers/mmc/socfpga_dw_mmc.c
> >>>>>>>>>>>>> @@ -11,25 +11,140 @@
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> #include <asm/arch/dwmmc.h>
> >>>>>>>>>>>>> #include <asm/arch/clock_manager.h>
> >>>>>>>>>>>>> #include <asm/arch/system_manager.h>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> +#include "mmc_private.h"
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> static const struct socfpga_clock_manager *clock_manager_base =
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> (void *)SOCFPGA_CLKMGR_ADDRESS;
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> static const struct socfpga_system_manager *system_manager_base =
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> (void *)SOCFPGA_SYSMGR_ADDRESS;
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> -static void socfpga_dwmci_clksel(struct dwmci_host *host)
> >>>>>>>>>>>>> +#define CAL_ROWS 7
> >>>>>>>>>>>>> +#define CAL_COLS 8
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> +/*
> >>>>>>>>>>>>> + * This function determines the largest rectangle filled with 1's
> >>>>>>>>>>>>> and returns + * the middle. This functon can be optimized, for
> >>>>>>>>>>>>> example using the algorithm + * from
> >>>>>>>>>>>>> http://www.drdobbs.com/database/the-maximal-rectangle-problem/18441
> >>>>>>>>>>>>> 0529 + * It currently takes less than 1ms, while creating the input
> >>>>>>>>>>>>> data takes ~5ms + * so there is not a real need to optimize it. +
> >>>>>>>>>>>>> */
> >>>>>>>>>>>>> +int find_calibration_point(unsigned char
> >>>>>>>>>>>>> cal_results[CAL_ROWS][CAL_COLS], +unsigned int sum, unsigned int *
> >>>>>>>>>>>>> cal_row, unsigned int * cal_col)
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> {
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> - unsigned int drvsel;
> >>>>>>>>>>>>> - unsigned int smplsel;
> >>>>>>>>>>>>> + /* Structure containing a rectangle candidate */
> >>>>>>>>>>>>> + typedef struct
> >>>>>>>>>>>>> + {
> >>>>>>>>>>>>> + unsigned char height;
> >>>>>>>>>>>>> + unsigned char width;
> >>>>>>>>>>>>> + unsigned short area;
> >>>>>>>>>>>>> + } rect_cand_t;
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /* Array with the rectangle candidates */
> >>>>>>>>>>>>> + rect_cand_t rect_cands[CAL_ROWS * CAL_COLS];
> >>>>>>>>>>>>> + unsigned char cr_rect_cand = 0;
> >>>>>>>>>>>>> + unsigned char height, width, k;
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /* No solution if all combinations fail */
> >>>>>>>>>>>>> + if(sum == 0)
> >>>>>>>>>>>>> + return 1;
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /* Simple solution if all combinations pass */
> >>>>>>>>>>>>> + if(sum == (CAL_ROWS * CAL_COLS)) {
> >>>>>>>>>>>>> + *cal_row = (CAL_ROWS - 1) / 2;
> >>>>>>>>>>>>> + *cal_col = (CAL_COLS - 1) / 2;
> >>>>>>>>>>>>> + return 0;
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /*
> >>>>>>>>>>>>> + * Create list of all possible sub-rectangles, in descending
> >>>>>>>> area
> >>>>>>>>>>>>> + * order
> >>>>>>>>>>>>> + */
> >>>>>>>>>>>>> + for(height = CAL_ROWS; height >= 1; height--) {
> >>>>>>>>>>>>> + for(width = CAL_COLS; width >= 1 ; width--){
> >>>>>>>>>>>>> + /* Add a new rectangle candidate */
> >>>>>>>>>>>>> + rect_cands[cr_rect_cand].height = height;
> >>>>>>>>>>>>> + rect_cands[cr_rect_cand].width = width;
> >>>>>>>>>>>>> + rect_cands[cr_rect_cand].area = height * width;
> >>>>>>>>>>>>> + cr_rect_cand++;
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /* First candidate it always in the right
> >>>>>>>> position */
> >>>>>>>>>>>>> + if(cr_rect_cand == 1)
> >>>>>>>>>>>>> + continue;
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /*
> >>>>>>>>>>>>> + * Put the candidate in right location to
> >>>>>>>> maintain
> >>>>>>>>>>>>> + * descending order
> >>>>>>>>>>>>> + */
> >>>>>>>>>>>>> + for(k = cr_rect_cand - 1; k > 1; k--){
> >>>>>>>>>>>>> + if(rect_cands[k-1].area <
> >>>>>>>> rect_cands[k].area){
> >>>>>>>>>>>>> + rect_cand_t tmp =
> >>>>>>>> rect_cands[k-1];
> >>>>>>>>>>>>> + rect_cands[k-1] = rect_cands[k];
> >>>>>>>>>>>>> + rect_cands[k] = tmp;
> >>>>>>>>>>>>> + } else
> >>>>>>>>>>>>> + break;
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /* Try to fit the rectangle candidates, in descending area order
> >>>>>>>>>>>>> */ + for(k = 0; k < CAL_ROWS * CAL_COLS; k++) {
> >>>>>>>>>>>>> + unsigned char start_row, start_col;
> >>>>>>>>>>>>> + unsigned rect_width = rect_cands[k].width;
> >>>>>>>>>>>>> + unsigned rect_height = rect_cands[k].height;
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /* Find the row and column where the candidate fits */
> >>>>>>>>>>>>> + for (start_col = 0; start_col < (CAL_COLS - rect_width +
> >>>>>>>> 1);
> >>>>>>>>>>>>> + start_col++) {
> >>>>>>>>>>>>> + for (start_row = 0; start_row < (CAL_ROWS -
> >>>>>>>>>>>>> + rect_height + 1); start_row++) {
> >>>>>>>>>>>>> + unsigned ok = 1;
> >>>>>>>>>>>>> + unsigned add_col, add_row;
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /* Determine if the rectangle fits here
> >>>>>>>> */
> >>>>>>>>>>>>> + for (add_col = 0; (add_col < rect_width)
> >>>>>>>> && ok;
> >>>>>>>>>>>>> + add_col++) {
> >>>>>>>>>>>>> + for (add_row = 0; add_row <
> >>>>>>>> rect_height;
> >>>>>>>>>>>>> + add_row++) {
> >>>>>>>>>>>>> + if (!cal_results
> >>>>>>>>>>>>> + [start_row+add_row]
> >>>>>>>> [start_col
> >>>>>>>>>>>>> + + add_col]) {
> >>>>>>>>>>>>> + ok = 0;
> >>>>>>>>>>>>> + break;
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /*
> >>>>>>>>>>>>> + * Return 'middle' of rectangle in case
> >>>>>>>> of
> >>>>>>>>>>>>> + * success
> >>>>>>>>>>>>> + */
> >>>>>>>>>>>>> + if (ok) {
> >>>>>>>>>>>>> + if(rect_width > 1)
> >>>>>>>>>>>>> + rect_width--;
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + if(rect_height > 1)
> >>>>>>>>>>>>> + rect_height--;
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + *cal_row = start_row +
> >>>>>>>>>>>>> + (rect_height / 2);
> >>>>>>>>>>>>> + *cal_col = start_col +
> >>>>>>>> (rect_width / 2);
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + return 0;
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> + }
> >>>>>>>>>>>>> +
> >>>>>>>>>>>>> + /* We could not fit any rectangle - return failure */
> >>>>>>>>>>>>> + return 1;
> >>>>>>>>>>>>
> >>>>>>>>>>>> Oh btw. please make sure to use errno.h and standard return codes ;
> >>>>>>>>>>>> -EINVAL in this case I believe.
> >>>>>>>>>>>
> >>>>>>>>>>> Good suggestion, will fix in v2.
> >>>>>>>>>>> Thanks
> >>>>>>>>>>
> >>>>>>>>>> Thanks. I was curious, is this really socfpga specific or can this
> >>>>>>>>>> calibration be used on dwmmc in general -- thus on exynos and rockchip
> >>>>>>>>>> systems -- as well? If it's generic to dwmmc, this should be moved into
> >>>>>>>>>> the dwmmc core code. Also, I am CCing Simon, he's been plumbing in the
> >>>>>>>>>> DWMMC recently.
> >>>>>>>
> >>>>>>> I didn't see you explicitly add Simon to the thread. Doing it here..
> >>>>>>
> >>>>>> One option is to put the function in the common dw_mmc.c file but have
> >>>>>> it be called explicitly by drivers that want it (e.g. for now, just
> >>>>>> socfpga). The code you have written does look generic and perhaps
> >>>>>> should be used on other platforms,
> >>>>>>
> >>>>>>>
> >>>>>>>>>
> >>>>>>>>> I am not sure. But definitely we can make the calibration common enough
> >>>>>>>>> if they required the calibration too. Let wait feedback from Simon.
> >>>>>>>>
> >>>>>>>
> >>>>>>> I know that the linux driver for the DW MMC for SoCFGPA, Exynos and Rockchip
> >>>>>>> are using the drvsel and smpsel parameters. But I also think these
> >>>>>>> parameters can
> >>>>>>> be different for different types of SD card. Are you sure this
> >>>>>>> function will cover those
> >>>>>>> cases?
> >>>>>>
> >>>>>> Not at all, but perhaps that is something we will find out in the future.
> >>>>>>
> >>>>>
> >>>>> Thanks Simon.
> >>>>>
> >>>>> I further checked today and noticed Exynos is setting up the clksel
> >>>>> (sample clock and drive clock). But from exynos_dw_mmc.c, the value is
> >>>>> coming from device tree. So I am notifying Jaehoon wonder the
> >>>>> calibration might applicable for him.
> >>
> >> I think this patch should not be common code at all.
> >> Of course, it seems that find_calibration_point function can be used at exynos.
> >> But it's too complex and not readable.
> >>
> >> And smpclk and drvsel are also not common.
> >> Depending on SoC, finding approach should be differed.
> >> I don't know how smpclk and drvsel controlled, since i don't read TRM of other SoC.
> >>
> >> Does it need to find the best clksmpl and drvsel value in bootloader?
> >>
> >
> > Yah, its little complex as we try to find the best value for 2
> > parameters together. Its also due to the nature that we rely on full
> > complete transaction cycle to determine pass or fail (drvsel for out
> > while clksmp for in).
>
> In my experiment, spmlclk value is more important than drvsel.
> Anyway, CAL_ROWS and CAL_COLS are candiates, right?
Yup, they are representing the value permutation for smplclk and drvsel.
> If it can be passed by parameter from each SoC dwmmc controller, that function should be located into dwmmc.c.
> Because drvsel and smplclk bits should be difference at each SoC specific register or Generic dwmmc(UHS_REG_EXT regster).
>
> Do you have the plan that change the more generic code than now?
Actually I look back and different SoC have different register location
for drvsel and smplclk. They are not located within dwmmc controller
registers. As of now, they are configure within the callback function of
clksel, I believe that should good.
>
> >
> > While finding the value, one of our customer have this issue. The SDMMC
> > communication doesn't work when they route the SDMMC slot few inches
> > away from the chip and running at high frequency.
>
> High frequency? DDR50 mode? I know that it's not implemented for the upper mode at bootloader.
> Is it related with controlling power, not controlling cmu?
Actually I am comparing the enumeration and normal run (SDR25). The
failure is spotted during data transfer such as fatload. Wonder what is
cmu? :)
Thanks
Chin Liang
>
> Best Regards,
> Jaehoon Chung
>
> >
> > With that, I shall introduce the code into socfpga_dw_mmc instead common
> > dw_mmc then. Wonder you would agree with that?
> >
> > Thanks
> > Chin Liang
> >
> >
> >> Best Regards,
> >> Jaehoon Chung
> >>
> >>>>
> >>>> Thanks for notifying this. But I can't find this patch at patchwork.
> >>>> I'm not reading fully mail thread, so after reading mails, i will resend at mailing list.
> >>>
> >>> Sure, here is the link :)
> >>> http://lists.denx.de/pipermail/u-boot/2015-August/224566.html
> >>>
> >>> Thanks
> >>> Chin Liang
> >>>
> >>>
> >>>>
> >>>> Best Regards,
> >>>> Jaehoon Chung
> >>>>
> >>>>>
> >>>>> Jaehoon, this patch will do a calibration to determine the clksel value.
> >>>>> Wonder is this something would help or applicable for Exynos? We are
> >>>>> probing whether this code should go dw_mmc.c instead socfpga_dw_mmc.c.
> >>>>>
> >>>>> Thanks
> >>>>> Chin Liang
> >>>>>
> >>>>>> Regards,
> >>>>>> Simon
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >>> _______________________________________________
> >>> U-Boot mailing list
> >>> U-Boot at lists.denx.de
> >>> http://lists.denx.de/mailman/listinfo/u-boot
> >>>
> >>
> >
> >
> >
>
More information about the U-Boot
mailing list