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

Jaehoon Chung
2015-08-25T02:36:19Z

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.

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