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

Jaehoon Chung
2015-08-26T05:29:05Z

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@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.

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?

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
>