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

Chin Liang See
2015-08-25T03:08:46Z

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@gmail.com> wrote:
> >>> +CC: Simon Glass
> >>>
> >>> On Thu, Aug 20, 2015 at 12:32 AM, Marek Vasut <marex@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@altera.com>
> >>>>>>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> >>>
> >>> I think there's something wrong with your git scripts, I did not get this email.
> >>>
> >>>>>>>>> Cc: Pavel Machek <pavel@denx.de>
> >>>>>>>>> Cc: Marek Vasut <marex@denx.de>
> >>>>>>>>> Cc: Wolfgang Denk <wd@denx.de>
> >>>>>>>>> Cc: Stefan Roese <sr@denx.de>
> >>>>>>>>> Cc: Tom Rini <trini@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.
>
> 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
> >
> >
> >
>