[U-Boot] [PATCH 1/2] arm: socfpga: mmc: Enable calibration for drvsel and smpsel
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 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/184410529 + * 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; +}
+void socfpga_dwmmc_set_clksel(unsigned int drvsel, unsigned int smplsel) +{ /* Disable SDMMC clock. */ clrbits_le32(&clock_manager_base->per_pll.en, CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK);
- /* Configures drv_sel and smpl_sel */ - drvsel = CONFIG_SOCFPGA_DWMMC_DRVSEL; - smplsel = CONFIG_SOCFPGA_DWMMC_SMPSEL; - debug("%s: drvsel %d smplsel %d\n", __func__, drvsel, smplsel); writel(SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel), &system_manager_base->sdmmcgrp_ctrl); @@ -42,6 +157,64 @@ static void socfpga_dwmci_clksel(struct dwmci_host *host) CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK); }
+/* + * This functions calirates the drvsel and smplsel by trying all possible + * values and selecting the best combnation + */ +void socfpga_dwmmc_calibrate(struct dwmci_host *host) +{ + int err = 0; + unsigned char cal_results[CAL_ROWS][CAL_COLS]; + unsigned int sum = 0; + unsigned int row, col, drvsel, smplsel; + + struct mmc *mmc = (struct mmc *)host->mmc; + if (mmc == NULL) { + printf("struct mmc is NULL\n"); + hang(); + } + + printf("%s: Calibration started.\n", __func__); + + for (row = 0; row < CAL_ROWS; row++) { + for (col = 0; col < CAL_COLS; col++) { + drvsel = row + 1; + smplsel = col; + socfpga_dwmmc_set_clksel(drvsel, smplsel); + cal_results[row][col] = !(mmc_set_blocklen(mmc, + mmc->read_bl_len)); + sum += cal_results[row][col]; + } + } + + debug("%s: Calibration raw data:\n", __func__); + for(row = 0; row < CAL_ROWS; row++) { + debug("\t"); + for(col = 0; col < CAL_COLS; col++) + debug("%d ", cal_results[row][col]); + debug("\n"); + } + + err = find_calibration_point(cal_results, sum, &drvsel, &smplsel); + + if(err) { + printf("%s: Calibration failed.\n", __func__); + hang(); + } else + printf("%s: Calibration passed: drvsel = %d " + "smplsel = %d.\n", __func__, drvsel, smplsel); + + socfpga_dwmmc_set_clksel(drvsel, smplsel); + + /* re-write in case accidentally overwrite with junk */ + mmc_set_blocklen(mmc, mmc->read_bl_len); +} + +void socfpga_dwmci_clksel(struct dwmci_host *host) +{ + socfpga_dwmmc_calibrate(host); +} + int socfpga_dwmmc_init(u32 regbase, int bus_width, int index) { struct dwmci_host *host;
On Wed 2015-08-19 00:54:50, 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 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/184410529
Actually, I'd like to see optimized version. It has potential to be easier to read...
- 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;
Codingstyle: "if ("
- /*
* 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--){
No space before ;.
- /* 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;
Create separate functions to make it easier to read and less indented.
On Wednesday, August 19, 2015 at 09:26:55 AM, Pavel Machek wrote:
On Wed 2015-08-19 00:54:50, 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 Cc: Pavel Machek pavel@denx.de Cc: Marek Vasut marex@denx.de Cc: Wolfgang Denk wd@denx.de
I guess there's no need to CC Wolfgang and Tom on SoCFPGA-specific stuff.
Cc: Stefan Roese sr@denx.de Cc: Tom Rini trini@konsulko.com
Hi!
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
What is this ugliness needed for ?
btw. Please rebase on top of u-boot-socfpga/master .
+/*
- 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/184410529
Actually, I'd like to see optimized version. It has potential to be easier to read...
- 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;
Also get rid of this typedef monster.
- /* 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;Codingstyle: "if ("
- /*
* 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--){No space before ;.
- /* 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;Create separate functions to make it easier to read and less indented.
Right.
Him
On Wed, 2015-08-19 at 09:34 +0200, marex@denx.de wrote:
On Wednesday, August 19, 2015 at 09:26:55 AM, Pavel Machek wrote:
On Wed 2015-08-19 00:54:50, 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 Cc: Pavel Machek pavel@denx.de Cc: Marek Vasut marex@denx.de Cc: Wolfgang Denk wd@denx.de
I guess there's no need to CC Wolfgang and Tom on SoCFPGA-specific stuff.
Sure, I was referring to the previous patch for the mailing list :)
Cc: Stefan Roese sr@denx.de Cc: Tom Rini trini@konsulko.com
Hi!
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
What is this ugliness needed for ?
Actually this is used to create the rectangle based on the result. We are doing the shmoo test for the parameter smpsel and drvsel.
btw. Please rebase on top of u-boot-socfpga/master .
Sure, just saw new changes has been added for last few hours.
+/*
- 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/184410529
Actually, I'd like to see optimized version. It has potential to be easier to read...
- 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;
Also get rid of this typedef monster.
Make sense as we are using here only.
- /* 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;Codingstyle: "if ("
- /*
* 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--){No space before ;.
- /* 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;Create separate functions to make it easier to read and less indented.
Right.
Let me crank my head on this :) Thanks
Chin Liang
On Wednesday, August 19, 2015 at 10:30:17 AM, Chin Liang See wrote:
Him
Her
On Wed, 2015-08-19 at 09:34 +0200, marex@denx.de wrote:
On Wednesday, August 19, 2015 at 09:26:55 AM, Pavel Machek wrote:
On Wed 2015-08-19 00:54:50, 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 Cc: Pavel Machek pavel@denx.de Cc: Marek Vasut marex@denx.de Cc: Wolfgang Denk wd@denx.de
I guess there's no need to CC Wolfgang and Tom on SoCFPGA-specific stuff.
Sure, I was referring to the previous patch for the mailing list :)
No problem.
Cc: Stefan Roese sr@denx.de Cc: Tom Rini trini@konsulko.com
Hi!
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
What is this ugliness needed for ?
Actually this is used to create the rectangle based on the result. We are doing the shmoo test for the parameter smpsel and drvsel.
btw. Please rebase on top of u-boot-socfpga/master .
Sure, just saw new changes has been added for last few hours.
I just got a few acks from Dinh, so I applied the stuff onto master. I'm trying to keep the code up-to-date there. The repository is certainly a bit overly active now, but that should calm down in the upcoming release. Afterall, we got most of the nice stuff into mainline in this MW.
[...]
Create separate functions to make it easier to read and less indented.
Right.
Let me crank my head on this :) Thanks
Thanks!
Hi,
On Wed, 2015-08-19 at 09:26 +0200, ZY - pavel wrote:
On Wed 2015-08-19 00:54:50, 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 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/184410529
Actually, I'd like to see optimized version. It has potential to be easier to read...
Actually in this calibration, we should see a obvious rectangle. We shouldn't have a fail in between the passes.
- 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;Codingstyle: "if ("
Good catch, noted
- /*
* 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--){No space before ;.
Good catch, noted
- /* 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;Create separate functions to make it easier to read and less indented.
Will do, thanks
Chin Liang
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 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/184410529 +
- 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;} elsebreak;}}- }
- /* 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.
Hi,
On Wed, 2015-08-19 at 02:41 +0000, marex@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 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/184410529 +
- 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;} elsebreak;}}- }
- /* 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
Chin Liang
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@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 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/184410529
- 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;} elsebreak;}}- }
- /* 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.
Marek Vasut
Hi,
On Wed, 2015-08-19 at 14:36 +0000, marex@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@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 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/184410529
- 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;} elsebreak;}}- }
- /* 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 am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.
Chin Liang
Marek Vasut
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@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@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 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;} elsebreak;}}- }
- /* 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 am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.
Excellent, thanks!
+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@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@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 descendingarea
* 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 rightposition */
if(cr_rect_cand == 1)continue;/** Put the candidate in right location tomaintain
* 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;} elsebreak;}}}/* 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 caseof
* 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..
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?
Dinh
On Thursday, August 20, 2015 at 11:55:02 PM, Dinh Nguyen wrote:
+CC: Simon Glass
Hi Dinh,
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@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@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.
You're not getting quite a lot of email I think. Could it be that altera has some issues with their mail system after some 300 patches went through your mailbox ? You sure you're not being filtered ? :b
> 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 > ---
[...]
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..
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?
If we're doing the calibration, we should do it correctly :)
On 8/20/15 4:59 PM, Marek Vasut wrote:
On Thursday, August 20, 2015 at 11:55:02 PM, Dinh Nguyen wrote:
+CC: Simon Glass
Hi Dinh,
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@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@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.
You're not getting quite a lot of email I think. Could it be that altera has some issues with their mail system after some 300 patches went through your mailbox ? You sure you're not being filtered ? :b
I would not be surprised if I was, but I don't think I am. Since I explicitly included my email address in the CC line on my earlier response, I am seeing the mail just fine now. My opensource.altera.com address was not even in the cc address line at all in any of the earlier emails.
Dinh
On Friday, August 21, 2015 at 02:33:56 AM, Dinh Nguyen wrote:
On 8/20/15 4:59 PM, Marek Vasut wrote:
On Thursday, August 20, 2015 at 11:55:02 PM, Dinh Nguyen wrote:
+CC: Simon Glass
Hi Dinh,
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@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@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.
You're not getting quite a lot of email I think. Could it be that altera has some issues with their mail system after some 300 patches went through your mailbox ? You sure you're not being filtered ? :b
I would not be surprised if I was, but I don't think I am. Since I explicitly included my email address in the CC line on my earlier response, I am seeing the mail just fine now. My opensource.altera.com address was not even in the cc address line at all in any of the earlier emails.
Oh, I see, thank you for clarifying :)
Could it be that the ML itself is behaving in a funny way ? I saw this a few times before too.
btw did you get my email about the IOCSR ?
Best regards, Marek Vasut
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@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@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.
Regards, Simon
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@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@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.
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
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@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@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
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@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@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
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@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@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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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@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@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@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?
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).
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.
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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@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@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@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?
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? 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?
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?
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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@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@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@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?
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Wed, 2015-08-26 at 01:54 -0500, Chin Liang See wrote:
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@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@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@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?
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.
For this case, I presume the change would applicable for socfpga only instead common dw_mmc. If everyone agreed, I shall send in new revision of the patch.
Thanks Chin Liang
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Tuesday, September 01, 2015 at 10:54:06 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 01:54 -0500, Chin Liang See wrote:
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@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@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@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-pro >>>>>>>>>> blem/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.
For this case, I presume the change would applicable for socfpga only instead common dw_mmc. If everyone agreed, I shall send in new revision of the patch.
I'm not very convinced that this should be outside of the dw_mmc core.
On Tue, 2015-09-01 at 11:01 +0200, marex@denx.de wrote:
On Tuesday, September 01, 2015 at 10:54:06 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 01:54 -0500, Chin Liang See wrote:
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@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@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@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-pro >>>>>>>>>>> blem/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.
For this case, I presume the change would applicable for socfpga only instead common dw_mmc. If everyone agreed, I shall send in new revision of the patch.
I'm not very convinced that this should be outside of the dw_mmc core.
Would want to hear more from Jaehoon as Exynos and SOCFPGA are the one setting up these values.
Thanks Chin Liang
On 09/01/2015 06:10 PM, Chin Liang See wrote:
On Tue, 2015-09-01 at 11:01 +0200, marex@denx.de wrote:
On Tuesday, September 01, 2015 at 10:54:06 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 01:54 -0500, Chin Liang See wrote:
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@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@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@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-pro >>>>>>>>>>>> blem/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.
For this case, I presume the change would applicable for socfpga only instead common dw_mmc. If everyone agreed, I shall send in new revision of the patch.
I'm not very convinced that this should be outside of the dw_mmc core.
Would want to hear more from Jaehoon as Exynos and SOCFPGA are the one setting up these values.
Since this approach is not based on dwmmc TRM, it's based on SOCFPGA SoC, i think that it doesn't need to include into dwmmc core. If need to located into dwmmc core, this code needs to modify more.
As drvsel and clksmpl are used at Exynos and SoCFPGA, it's not core feature. Other SoC can't be used them. we don't know.
Best Regards, Jaehoon Chung
Thanks Chin Liang
On 09/01/2015 05:12 AM, Jaehoon Chung wrote:
On 09/01/2015 06:10 PM, Chin Liang See wrote:
On Tue, 2015-09-01 at 11:01 +0200, marex@denx.de wrote:
On Tuesday, September 01, 2015 at 10:54:06 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 01:54 -0500, Chin Liang See wrote:
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@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@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@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-pro >>>>>>>>>>>>> blem/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.
For this case, I presume the change would applicable for socfpga only instead common dw_mmc. If everyone agreed, I shall send in new revision of the patch.
I'm not very convinced that this should be outside of the dw_mmc core.
Would want to hear more from Jaehoon as Exynos and SOCFPGA are the one setting up these values.
Since this approach is not based on dwmmc TRM, it's based on SOCFPGA SoC, i think that it doesn't need to include into dwmmc core. If need to located into dwmmc core, this code needs to modify more.
As drvsel and clksmpl are used at Exynos and SoCFPGA, it's not core feature. Other SoC can't be used them. we don't know.
Plus the way that this function is implemented, it is very specific to SoCFGA, as the tables and rows are representing 45 degree increments for the drvsel and smplsel value. Other platforms can have either more or less granularity in the drvsel and smplsel values.
Dinh
On Tuesday, September 01, 2015 at 04:53:44 PM, Dinh Nguyen wrote:
On 09/01/2015 05:12 AM, Jaehoon Chung wrote:
On 09/01/2015 06:10 PM, Chin Liang See wrote:
On Tue, 2015-09-01 at 11:01 +0200, marex@denx.de wrote:
On Tuesday, September 01, 2015 at 10:54:06 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 01:54 -0500, Chin Liang See wrote:
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@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@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@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- >>>>>>>>>>>>>> pro blem/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.
For this case, I presume the change would applicable for socfpga only instead common dw_mmc. If everyone agreed, I shall send in new revision of the patch.
I'm not very convinced that this should be outside of the dw_mmc core.
Would want to hear more from Jaehoon as Exynos and SOCFPGA are the one setting up these values.
Since this approach is not based on dwmmc TRM, it's based on SOCFPGA SoC, i think that it doesn't need to include into dwmmc core. If need to located into dwmmc core, this code needs to modify more.
As drvsel and clksmpl are used at Exynos and SoCFPGA, it's not core feature. Other SoC can't be used them. we don't know.
Plus the way that this function is implemented, it is very specific to SoCFGA, as the tables and rows are representing 45 degree increments for the drvsel and smplsel value. Other platforms can have either more or less granularity in the drvsel and smplsel values.
How is this SMPLSEL and DRVSEL implemented on Exynos ?
On Wed, 2015-09-02 at 12:32 +0200, marex@denx.de wrote:
On Tuesday, September 01, 2015 at 04:53:44 PM, Dinh Nguyen wrote:
On 09/01/2015 05:12 AM, Jaehoon Chung wrote:
On 09/01/2015 06:10 PM, Chin Liang See wrote:
On Tue, 2015-09-01 at 11:01 +0200, marex@denx.de wrote:
On Tuesday, September 01, 2015 at 10:54:06 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 01:54 -0500, Chin Liang See wrote: > 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@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@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@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- >>>>>>>>>>>>>>> pro blem/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.
For this case, I presume the change would applicable for socfpga only instead common dw_mmc. If everyone agreed, I shall send in new revision of the patch.
I'm not very convinced that this should be outside of the dw_mmc core.
Would want to hear more from Jaehoon as Exynos and SOCFPGA are the one setting up these values.
Since this approach is not based on dwmmc TRM, it's based on SOCFPGA SoC, i think that it doesn't need to include into dwmmc core. If need to located into dwmmc core, this code needs to modify more.
As drvsel and clksmpl are used at Exynos and SoCFPGA, it's not core feature. Other SoC can't be used them. we don't know.
Plus the way that this function is implemented, it is very specific to SoCFGA, as the tables and rows are representing 45 degree increments for the drvsel and smplsel value. Other platforms can have either more or less granularity in the drvsel and smplsel values.
How is this SMPLSEL and DRVSEL implemented on Exynos ?
The value comes from device tree.
Thanks Chin Liang
Hi,
On 09/03/2015 09:27 AM, Chin Liang See wrote:
On Wed, 2015-09-02 at 12:32 +0200, marex@denx.de wrote:
[snip]
Would want to hear more from Jaehoon as Exynos and SOCFPGA are the one setting up these values.
Since this approach is not based on dwmmc TRM, it's based on SOCFPGA SoC, i think that it doesn't need to include into dwmmc core. If need to located into dwmmc core, this code needs to modify more.
As drvsel and clksmpl are used at Exynos and SoCFPGA, it's not core feature. Other SoC can't be used them. we don't know.
Plus the way that this function is implemented, it is very specific to SoCFGA, as the tables and rows are representing 45 degree increments for the drvsel and smplsel value. Other platforms can have either more or less granularity in the drvsel and smplsel values.
How is this SMPLSEL and DRVSEL implemented on Exynos ?
Exynos is using CLKSEL register in dw-mmc controller. It's exynos specific register in dwmmc controller. It's also represented 45 degree increment. SELCK_DRV is bit[18:16] or more. SELCLK_SAMPLE is bit[2:0] or more. There are other bits relevant to tuning clock. '_more_' means that it can be changed bandwidth.
Anyway, I think there is no right method about finding the best smplclk and drvsel. If this is generic method, i will pick this. But i don't think so, and there is no benefit for exynos.
smplclk and drvsel value need to process the tuning sequence. There is no tuning case at bootloader, since it's not implemented about HS200 or upper mode.
Clksel an drvsel value are passed by device tree.
Best Regards, Jaehoon Chung
The value comes from device tree.
Thanks Chin Liang
On Thursday, September 03, 2015 at 07:30:08 AM, Jaehoon Chung wrote:
Hi,
Hi,
On 09/03/2015 09:27 AM, Chin Liang See wrote:
On Wed, 2015-09-02 at 12:32 +0200, marex@denx.de wrote:
[snip]
thanks
Would want to hear more from Jaehoon as Exynos and SOCFPGA are the one setting up these values.
Since this approach is not based on dwmmc TRM, it's based on SOCFPGA SoC, i think that it doesn't need to include into dwmmc core. If need to located into dwmmc core, this code needs to modify more.
As drvsel and clksmpl are used at Exynos and SoCFPGA, it's not core feature. Other SoC can't be used them. we don't know.
Plus the way that this function is implemented, it is very specific to SoCFGA, as the tables and rows are representing 45 degree increments for the drvsel and smplsel value. Other platforms can have either more or less granularity in the drvsel and smplsel values.
How is this SMPLSEL and DRVSEL implemented on Exynos ?
Exynos is using CLKSEL register in dw-mmc controller. It's exynos specific register in dwmmc controller. It's also represented 45 degree increment. SELCK_DRV is bit[18:16] or more. SELCLK_SAMPLE is bit[2:0] or more. There are other bits relevant to tuning clock. '_more_' means that it can be changed bandwidth.
Anyway, I think there is no right method about finding the best smplclk and drvsel. If this is generic method, i will pick this. But i don't think so, and there is no benefit for exynos.
smplclk and drvsel value need to process the tuning sequence. There is no tuning case at bootloader, since it's not implemented about HS200 or upper mode.
Clksel an drvsel value are passed by device tree.
In that case, maybe SoCFPGA should also pick those values from DT ? It would keep the code simple and in case there is a problematic board, it could use u-boot application to perform the tuning.
Best regards, Marek Vasut
Hi,
On Thu, 2015-09-03 at 11:37 +0200, marex@denx.de wrote:
On Thursday, September 03, 2015 at 07:30:08 AM, Jaehoon Chung wrote:
Hi,
Hi,
On 09/03/2015 09:27 AM, Chin Liang See wrote:
On Wed, 2015-09-02 at 12:32 +0200, marex@denx.de wrote:
[snip]
thanks
> Would want to hear more from Jaehoon as Exynos and SOCFPGA are the > one setting up these values.
Since this approach is not based on dwmmc TRM, it's based on SOCFPGA SoC, i think that it doesn't need to include into dwmmc core. If need to located into dwmmc core, this code needs to modify more.
As drvsel and clksmpl are used at Exynos and SoCFPGA, it's not core feature. Other SoC can't be used them. we don't know.
Plus the way that this function is implemented, it is very specific to SoCFGA, as the tables and rows are representing 45 degree increments for the drvsel and smplsel value. Other platforms can have either more or less granularity in the drvsel and smplsel values.
How is this SMPLSEL and DRVSEL implemented on Exynos ?
Exynos is using CLKSEL register in dw-mmc controller. It's exynos specific register in dwmmc controller. It's also represented 45 degree increment. SELCK_DRV is bit[18:16] or more. SELCLK_SAMPLE is bit[2:0] or more. There are other bits relevant to tuning clock. '_more_' means that it can be changed bandwidth.
Anyway, I think there is no right method about finding the best smplclk and drvsel. If this is generic method, i will pick this. But i don't think so, and there is no benefit for exynos.
smplclk and drvsel value need to process the tuning sequence. There is no tuning case at bootloader, since it's not implemented about HS200 or upper mode.
Clksel an drvsel value are passed by device tree.
In that case, maybe SoCFPGA should also pick those values from DT ? It would keep the code simple and in case there is a problematic board, it could use u-boot application to perform the tuning.
I prefer not to do that as it narrows the supported use case for the driver.
Thanks Chin Liang
Best regards, Marek Vasut
On Thursday, September 03, 2015 at 04:06:12 PM, Chin Liang See wrote:
Hi,
On Thu, 2015-09-03 at 11:37 +0200, marex@denx.de wrote:
On Thursday, September 03, 2015 at 07:30:08 AM, Jaehoon Chung wrote:
Hi,
Hi,
On 09/03/2015 09:27 AM, Chin Liang See wrote:
On Wed, 2015-09-02 at 12:32 +0200, marex@denx.de wrote:
[snip]
thanks
>> Would want to hear more from Jaehoon as Exynos and SOCFPGA are >> the one setting up these values. > > Since this approach is not based on dwmmc TRM, it's based on > SOCFPGA SoC, i think that it doesn't need to include into dwmmc > core. If need to located into dwmmc core, this code needs to > modify more. > > As drvsel and clksmpl are used at Exynos and SoCFPGA, it's not > core feature. Other SoC can't be used them. we don't know.
Plus the way that this function is implemented, it is very specific to SoCFGA, as the tables and rows are representing 45 degree increments for the drvsel and smplsel value. Other platforms can have either more or less granularity in the drvsel and smplsel values.
How is this SMPLSEL and DRVSEL implemented on Exynos ?
Exynos is using CLKSEL register in dw-mmc controller. It's exynos specific register in dwmmc controller. It's also represented 45 degree increment. SELCK_DRV is bit[18:16] or more. SELCLK_SAMPLE is bit[2:0] or more. There are other bits relevant to tuning clock. '_more_' means that it can be changed bandwidth.
Anyway, I think there is no right method about finding the best smplclk and drvsel. If this is generic method, i will pick this. But i don't think so, and there is no benefit for exynos.
smplclk and drvsel value need to process the tuning sequence. There is no tuning case at bootloader, since it's not implemented about HS200 or upper mode.
Clksel an drvsel value are passed by device tree.
In that case, maybe SoCFPGA should also pick those values from DT ? It would keep the code simple and in case there is a problematic board, it could use u-boot application to perform the tuning.
I prefer not to do that as it narrows the supported use case for the driver.
How so? It keeps the driver code clean and this code you're adding seems like a special-purpose stuff which needs to be done once for particular board, no ?
Best regards, Marek Vasut
Hi!
How is this SMPLSEL and DRVSEL implemented on Exynos ?
Exynos is using CLKSEL register in dw-mmc controller. It's exynos specific register in dwmmc controller. It's also represented 45 degree increment. SELCK_DRV is bit[18:16] or more. SELCLK_SAMPLE is bit[2:0] or more. There are other bits relevant to tuning clock. '_more_' means that it can be changed bandwidth.
Anyway, I think there is no right method about finding the best smplclk and drvsel. If this is generic method, i will pick this. But i don't think so, and there is no benefit for exynos.
smplclk and drvsel value need to process the tuning sequence. There is no tuning case at bootloader, since it's not implemented about HS200 or upper mode.
Clksel an drvsel value are passed by device tree.
In that case, maybe SoCFPGA should also pick those values from DT ? It would keep the code simple and in case there is a problematic board, it could use u-boot application to perform the tuning.
I prefer not to do that as it narrows the supported use case for the driver.
How so? It keeps the driver code clean and this code you're adding seems like a special-purpose stuff which needs to be done once for particular board, no ?
Well... stuff that can be automatically detected is not supposed to be in the device tree.
clksel and drvsel can be calibrated, so I see some arguments why we should calibrate them, and not hardcode them in the device tree. Pavel
Hi,
On 09/04/2015 07:41 PM, Pavel Machek wrote:
Hi!
> How is this SMPLSEL and DRVSEL implemented on Exynos ?
Exynos is using CLKSEL register in dw-mmc controller. It's exynos specific register in dwmmc controller. It's also represented 45 degree increment. SELCK_DRV is bit[18:16] or more. SELCLK_SAMPLE is bit[2:0] or more. There are other bits relevant to tuning clock. '_more_' means that it can be changed bandwidth.
Anyway, I think there is no right method about finding the best smplclk and drvsel. If this is generic method, i will pick this. But i don't think so, and there is no benefit for exynos.
smplclk and drvsel value need to process the tuning sequence. There is no tuning case at bootloader, since it's not implemented about HS200 or upper mode.
Clksel an drvsel value are passed by device tree.
In that case, maybe SoCFPGA should also pick those values from DT ? It would keep the code simple and in case there is a problematic board, it could use u-boot application to perform the tuning.
I prefer not to do that as it narrows the supported use case for the driver.
How so? It keeps the driver code clean and this code you're adding seems like a special-purpose stuff which needs to be done once for particular board, no ?
Well... stuff that can be automatically detected is not supposed to be in the device tree.
clksel and drvsel can be calibrated, so I see some arguments why we should calibrate them, and not hardcode them in the device tree.
My opinions are
1. This code is not generic dwmmc code. So i don't want to locate into dwmmc core. If need to apply, i agree that it applies this in socfpga-dw_mmc.c.
2. In exynos, value of devcie-tree is the tested value. After has tested with every values, it defined the best value into device-tree. (Working fine with values.) At every time, it doesn't need to detect the best value with same SoC. (Especially, at bootloader)
3. In my experiment, there should be side-effect during finding best sample/drv value.
4. If HS200 or upper mode is supported at bootloader, it needs the tuning sequence. Then it needs to find the best sampl/drv values. but it doesn't support HS200 or other at bootloader.
5. Affect at booting time??
Best Regards, Jaehoon Chung
Pavel
Hi,
On Mon, 2015-09-07 at 03:33 +0000, Jaehoon Chung wrote:
Hi,
On 09/04/2015 07:41 PM, Pavel Machek wrote:
Hi!
>> How is this SMPLSEL and DRVSEL implemented on Exynos ?
Exynos is using CLKSEL register in dw-mmc controller. It's exynos specific register in dwmmc controller. It's also represented 45 degree increment. SELCK_DRV is bit[18:16] or more. SELCLK_SAMPLE is bit[2:0] or more. There are other bits relevant to tuning clock. '_more_' means that it can be changed bandwidth.
Anyway, I think there is no right method about finding the best smplclk and drvsel. If this is generic method, i will pick this. But i don't think so, and there is no benefit for exynos.
smplclk and drvsel value need to process the tuning sequence. There is no tuning case at bootloader, since it's not implemented about HS200 or upper mode.
Clksel an drvsel value are passed by device tree.
In that case, maybe SoCFPGA should also pick those values from DT ? It would keep the code simple and in case there is a problematic board, it could use u-boot application to perform the tuning.
I prefer not to do that as it narrows the supported use case for the driver.
How so? It keeps the driver code clean and this code you're adding seems like a special-purpose stuff which needs to be done once for particular board, no ?
Well... stuff that can be automatically detected is not supposed to be in the device tree.
clksel and drvsel can be calibrated, so I see some arguments why we should calibrate them, and not hardcode them in the device tree.
My opinions are
- This code is not generic dwmmc code. So i don't want to locate into dwmmc core.
If need to apply, i agree that it applies this in socfpga-dw_mmc.c.
Yup, let move that to socfpga-dw_mmc.c as majority believe that is correct way to move forward. I shall send in v3 which integrate Pavel's comment in v2
- In exynos, value of devcie-tree is the tested value.
After has tested with every values, it defined the best value into device-tree. (Working fine with values.) At every time, it doesn't need to detect the best value with same SoC. (Especially, at bootloader)
In my experiment, there should be side-effect during finding best sample/drv value.
If HS200 or upper mode is supported at bootloader, it needs the tuning sequence.
Then it needs to find the best sampl/drv values. but it doesn't support HS200 or other at bootloader.
Yah, normally it will not need that. As we are catering for various customer board design, this calibration will help even they routed the SDMMC slot few inches away.
- Affect at booting time??
We measured this before and it took around ~5.2ms for worst case.
Thanks Chin Liang
Best Regards, Jaehoon Chung
Pavel
On Tuesday, September 08, 2015 at 03:32:33 AM, Chin Liang See wrote:
Hi,
Hi,
On Mon, 2015-09-07 at 03:33 +0000, Jaehoon Chung wrote:
Hi,
On 09/04/2015 07:41 PM, Pavel Machek wrote:
Hi!
>>> How is this SMPLSEL and DRVSEL implemented on Exynos ? > > Exynos is using CLKSEL register in dw-mmc controller. > It's exynos specific register in dwmmc controller. It's also > represented 45 degree increment. SELCK_DRV is bit[18:16] or more. > SELCLK_SAMPLE is bit[2:0] or more. There are other bits relevant to > tuning clock. '_more_' means that it can be changed bandwidth. > > Anyway, I think there is no right method about finding the best > smplclk and drvsel. If this is generic method, i will pick this. > But i don't think so, and there is no benefit for exynos. > > smplclk and drvsel value need to process the tuning sequence. > There is no tuning case at bootloader, since it's not implemented > about HS200 or upper mode. > > Clksel an drvsel value are passed by device tree.
In that case, maybe SoCFPGA should also pick those values from DT ? It would keep the code simple and in case there is a problematic board, it could use u-boot application to perform the tuning.
I prefer not to do that as it narrows the supported use case for the driver.
How so? It keeps the driver code clean and this code you're adding seems like a special-purpose stuff which needs to be done once for particular board, no ?
Well... stuff that can be automatically detected is not supposed to be in the device tree.
clksel and drvsel can be calibrated, so I see some arguments why we should calibrate them, and not hardcode them in the device tree.
My opinions are
- This code is not generic dwmmc code. So i don't want to locate into
dwmmc core. If need to apply, i agree that it applies this in socfpga-dw_mmc.c.
Yup, let move that to socfpga-dw_mmc.c as majority believe that is correct way to move forward. I shall send in v3 which integrate Pavel's comment in v2
- In exynos, value of devcie-tree is the tested value.
After has tested with every values, it defined the best value into device-tree. (Working fine with values.) At every time, it doesn't need to detect the best value with same SoC. (Especially, at bootloader)
- In my experiment, there should be side-effect during finding best
sample/drv value.
- If HS200 or upper mode is supported at bootloader, it needs the tuning
sequence. Then it needs to find the best sampl/drv values. but it doesn't support HS200 or other at bootloader.
Yah, normally it will not need that. As we are catering for various customer board design, this calibration will help even they routed the SDMMC slot few inches away.
- Affect at booting time??
We measured this before and it took around ~5.2ms for worst case.
Hm, what about supporting both the DT variant (where you specify drvsel and smplsel in DT, the way we do it in Linux) and this calibration variant. The calibration would only be executed if the smplsel and drvsel args were missing from the DT.
What do you think ?
participants (7)
-
Chin Liang See -
Dinh Nguyen -
Dinh Nguyen -
Jaehoon Chung -
Marek Vasut -
Pavel Machek -
Simon Glass