Skip to contentSkip to conversation

arm: socfpga: mmc: Enable calibration for drvsel and smpsel

PATCH Superseded Chin Liang See 7 participants 40 messages Aug 19, 2015 – Oct 12, 2015 1 message not in the archive MMC, SD and SDIO

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>
---
 drivers/mmc/socfpga_dw_mmc.c |  187 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 180 insertions(+), 7 deletions(-)
drivers/mmc/socfpga_dw_mmc.c +180 -7
index eb69aed..15e537c 100644
@@ -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;
Signature · 1 line
1.7.7.4
#1 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 29 lines · Chin Liang See

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>
---
 drivers/mmc/socfpga_dw_mmc.c |  187 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 180 insertions(+), 7 deletions(-)
drivers/mmc/socfpga_dw_mmc.c +1 -0
index eb69aed..15e537c 100644
@@ -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 =

drivers/mmc/socfpga_dw_mmc.c line 27

Quoted 10 lines
drivers/mmc/socfpga_dw_mmc.c +7 -1
 		(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...

Quoted 14 lines
drivers/mmc/socfpga_dw_mmc.c +11 -2
+ * 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;

drivers/mmc/socfpga_dw_mmc.c line 49

Quoted 10 lines
drivers/mmc/socfpga_dw_mmc.c +10 -0
+	} 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 ("

drivers/mmc/socfpga_dw_mmc.c line 63

Quoted 6 lines
drivers/mmc/socfpga_dw_mmc.c +6 -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--){

No space before ;.

Quoted 14 lines
drivers/mmc/socfpga_dw_mmc.c +14 -0
+	/* 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;
+

drivers/mmc/socfpga_dw_mmc.c line 112

Quoted 10 lines
drivers/mmc/socfpga_dw_mmc.c +10 -0
+				/* 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.

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#2 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 11 lines · Pavel Machek
Quoted 10 lines · Chin Liang See

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>

I guess there's no need to CC Wolfgang and Tom on SoCFPGA-specific stuff.

Quoted 3 lines
Quoted 3 lines
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

Hi!

Quoted 20 lines
Quoted 20 lines
 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

drivers/mmc/socfpga_dw_mmc.c +1 -0
index eb69aed..15e537c 100644
@@ -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;
 
Quoted 7 lines

drivers/mmc/socfpga_dw_mmc.c line 22

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +2 -1
 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 .

Quoted 19 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+/*
+ * 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...

Quoted 9 lines

+ * 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)

{

Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c line 40

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +7 -2
-	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.

Quoted 38 lines

drivers/mmc/socfpga_dw_mmc.c line 49

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+	/* 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 ("

drivers/mmc/socfpga_dw_mmc.c line 63

Quoted 6 lines
drivers/mmc/socfpga_dw_mmc.c +6 -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--){

No space before ;.

Quoted 18 lines
drivers/mmc/socfpga_dw_mmc.c +18 -0
+	/* 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;
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c line 112

Quoted 6 lines
drivers/mmc/socfpga_dw_mmc.c +6 -0
+					     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.

#3 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Him

Quoted 7 lines · marex at denx.de
Quoted 6 lines · Pavel Machek
Quoted 5 lines · Chin Liang See

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.

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>

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

Quoted 29 lines
Quoted 4 lines
Quoted 4 lines
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

Hi!

Quoted 22 lines
Quoted 22 lines
 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

drivers/mmc/socfpga_dw_mmc.c +1 -0
index eb69aed..15e537c 100644
@@ -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 =
 
Quoted 7 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c line 22

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +2 -1
 		(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.

Quoted 2 lines

btw. Please rebase on top of u-boot-socfpga/master .

Sure, just saw new changes has been added for last few hours.

Quoted 20 lines
Quoted 20 lines
Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+/*
+ * 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...

Quoted 9 lines

+ * 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)

{

Quoted 11 lines
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c line 40

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +7 -2
-	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.

Quoted 48 lines
Quoted 39 lines

drivers/mmc/socfpga_dw_mmc.c line 49

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+	/* 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 ("

drivers/mmc/socfpga_dw_mmc.c line 63

Quoted 6 lines
drivers/mmc/socfpga_dw_mmc.c +6 -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--){

No space before ;.

Quoted 18 lines
drivers/mmc/socfpga_dw_mmc.c +18 -0
+	/* 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;
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c line 112

Quoted 6 lines
drivers/mmc/socfpga_dw_mmc.c +6 -0
+					     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.

Quoted 1 line

Right.

Let me crank my head on this :)
Thanks

Chin Liang

#4 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 1 line · Chin Liang See

Him

Her

Quoted 14 lines · marex at denx.de
Quoted 13 lines · Pavel Machek
Quoted 11 lines · Chin Liang See
Quoted 10 lines

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Quoted 3 lines
Quoted 1 line

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.

Quoted 36 lines
Quoted 28 lines
Quoted 3 lines
Quoted 3 lines
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

Hi!

Quoted 22 lines
Quoted 22 lines
 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +1 -0
@@ -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 =
 
Quoted 7 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c line 22

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +2 -1
 		(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 ?

Quoted 6 lines

Actually this is used to create the rectangle based on the result. We are doing the shmoo test for the parameter smpsel and drvsel.

Quoted 1 line

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.

[...]

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Create separate functions to make it easier to read and less indented.

Right.

Let me crank my head on this :)
Thanks

Thanks!

#5 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

In reply to Pavel Machek

Hi,

Quoted 42 lines · ZY - pavel
Quoted 29 lines · Chin Liang See

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>
---
 drivers/mmc/socfpga_dw_mmc.c |  187 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 180 insertions(+), 7 deletions(-)
drivers/mmc/socfpga_dw_mmc.c +1 -0
index eb69aed..15e537c 100644
@@ -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 =

drivers/mmc/socfpga_dw_mmc.c line 27

Quoted 10 lines
drivers/mmc/socfpga_dw_mmc.c +7 -1
 		(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

Quoted 3 lines

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.

Quoted 18 lines
Quoted 18 lines
drivers/mmc/socfpga_dw_mmc.c +15 -2
+ * 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];
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c line 49

Quoted 6 lines
drivers/mmc/socfpga_dw_mmc.c +6 -0
+	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

Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c line 63

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +6 -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--){

No space before ;.

Good catch, noted

Quoted 20 lines
Quoted 20 lines
drivers/mmc/socfpga_dw_mmc.c +18 -0
+	/* 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;
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c line 112

Quoted 6 lines
drivers/mmc/socfpga_dw_mmc.c +6 -0
+					     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

#6 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

In reply to Chin Liang See
Quoted 145 lines · Chin Liang See

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>
---
 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

drivers/mmc/socfpga_dw_mmc.c +6 -1
index eb69aed..15e537c 100644
@@ -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.
+ */

drivers/mmc/socfpga_dw_mmc.c +98 -2
+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);

drivers/mmc/socfpga_dw_mmc.c line 139

Quoted 10 lines
drivers/mmc/socfpga_dw_mmc.c +10 -0
+					*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.

#7 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi,

Quoted 151 lines · marex at denx.de
Quoted 150 lines · Chin Liang See

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>
---
 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

drivers/mmc/socfpga_dw_mmc.c +6 -1
index eb69aed..15e537c 100644
@@ -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.
+ */

drivers/mmc/socfpga_dw_mmc.c +103 -2
+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;
+				}
+			}
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c line 139

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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

#8 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 171 lines · marex at denx.de
Quoted 161 lines · Chin Liang See
Quoted 160 lines

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +101 -2
-	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;
+				}
+			}
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c line 139

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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.

Quoted 2 lines

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

#9 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi,

Quoted 176 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 171 lines · marex at denx.de
Quoted 169 lines · Chin Liang See
Quoted 155 lines

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +96 -2
-	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);

drivers/mmc/socfpga_dw_mmc.c line 139

Quoted 10 lines
drivers/mmc/socfpga_dw_mmc.c +10 -0
+					*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.

Quoted 8 lines
Quoted 2 lines

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

Quoted 1 line

Marek Vasut

#10 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 87 lines · Chin Liang See

Hi,

Quoted 84 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 79 lines · marex at denx.de
Quoted 78 lines · Chin Liang See
Quoted 77 lines

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.

Cc Dinh Nguyen <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +18 -2
-	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 */
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+	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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 11 lines
drivers/mmc/socfpga_dw_mmc.c +11 -0
+	 * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -0
+			if(cr_rect_cand == 1)
+				continue;
+
+			/*
+			 * Put the candidate in right location to 

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+			 * descending order
+			 */
+			for(k = cr_rect_cand - 1; k > 1; k--){
+				if(rect_cands[k-1].area < 

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/mmc/socfpga_dw_mmc.c

Quoted 1 line
drivers/mmc/socfpga_dw_mmc.c +1 -0
+					rect_cand_t tmp = 

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+					rect_cands[k-1] = rect_cands[k];
+					rect_cands[k] = tmp;
+				} else
+					break;
+			}
+		}
+	}
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +1 -0
+	/* Try to fit the rectangle candidates, in descending area order

*/ + for(k = 0; k < CAL_ROWS * CAL_COLS; k++) {

drivers/mmc/socfpga_dw_mmc.c +6 -0
+		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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +7 -0
+		     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 

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/mmc/socfpga_dw_mmc.c

Quoted 1 line
drivers/mmc/socfpga_dw_mmc.c +1 -0
+				for (add_col = 0; (add_col < rect_width) 

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 2 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+				     add_col++) {
+					for (add_row = 0; add_row < 

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 3 lines
drivers/mmc/socfpga_dw_mmc.c +3 -0
+					     add_row++) {
+						if (!cal_results
+						   [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +9 -0
+						   + add_col]) {
+							ok = 0;
+							break;
+						}
+					}
+				}
+
+				/*
+				 * Return 'middle' of rectangle in case 

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+				 * success
+				 */
+				if (ok) {
+					if(rect_width > 1)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+						rect_width--;
+
+					if(rect_height > 1)
+						rect_height--;
+
+					*cal_row = start_row +
+						   (rect_height / 2);
+					*cal_col = start_col + 

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+
+					return 0;
+				}
+			}
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c line 139

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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

Quoted 8 lines
Quoted 5 lines

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!

#11 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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.

I think there's something wrong with your git scripts, I did not get this email.

Quoted 148 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +18 -2
-     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 */
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+     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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 11 lines
drivers/mmc/socfpga_dw_mmc.c +11 -0
+      * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -0
+                     if(cr_rect_cand == 1)
+                             continue;
+
+                     /*
+                      * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                      * descending order
+                      */
+                     for(k = cr_rect_cand - 1; k > 1; k--){
+                             if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                     rect_cands[k-1] = rect_cands[k];
+                                     rect_cands[k] = tmp;
+                             } else
+                                     break;
+                     }
+             }
+     }
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines

+ /* Try to fit the rectangle candidates, in descending area order
*/ + for(k = 0; k < CAL_ROWS * CAL_COLS; k++) {

drivers/mmc/socfpga_dw_mmc.c +6 -0
+             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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +7 -0
+                  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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 2 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+                                  add_col++) {
+                                     for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 3 lines
drivers/mmc/socfpga_dw_mmc.c +3 -0
+                                          add_row++) {
+                                             if (!cal_results
+                                                [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +9 -0
+                                                + add_col]) {
+                                                     ok = 0;
+                                                     break;
+                                             }
+                                     }
+                             }
+
+                             /*
+                              * Return 'middle' of rectangle in case

of

Quoted 34 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                              * success
+                              */
+                             if (ok) {
+                                     if(rect_width > 1)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                             rect_width--;
+
+                                     if(rect_height > 1)
+                                             rect_height--;
+
+                                     *cal_row = start_row +
+                                                (rect_height / 2);
+                                     *cal_col = start_col +

(rect_width / 2);

Quoted 21 lines
Quoted 21 lines
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+
+                                     return 0;
+                             }
+                     }
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c line 139

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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.

Quoted 2 lines

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

Quoted 4 lines
Quoted 3 lines

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

#12 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 1 line · Dinh Nguyen

+CC: Simon Glass

Hi Dinh,

Quoted 17 lines · Marek Vasut
Quoted 16 lines · Chin Liang See
Quoted 15 lines

Hi,

Quoted 12 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 7 lines · marex at denx.de
Quoted 6 lines · Chin Liang See
Quoted 5 lines

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.

Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

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

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

[...]

Quoted 11 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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

Quoted 2 lines
Quoted 2 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 6 lines

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

#13 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 25 lines · Marek Vasut
Quoted 1 line · Dinh Nguyen

+CC: Simon Glass

Hi Dinh,

Quoted 20 lines · Marek Vasut
Quoted 18 lines · Chin Liang See
Quoted 17 lines

Hi,

Quoted 14 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 9 lines · marex at denx.de
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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.

Quoted 7 lines
Quoted 2 lines

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

#14 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 33 lines · Dinh Nguyen
Quoted 25 lines · Marek Vasut
Quoted 1 line · Dinh Nguyen

+CC: Simon Glass

Hi Dinh,

Quoted 20 lines · Marek Vasut
Quoted 18 lines · Chin Liang See
Quoted 17 lines

Hi,

Quoted 14 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 9 lines · marex at denx.de
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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.

Quoted 6 lines
Quoted 2 lines

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

Quoted 5 lines

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

#15 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

In reply to Dinh Nguyen

Hi,

Quoted 201 lines · Dinh Nguyen

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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.

I think there's something wrong with your git scripts, I did not get this email.

Quoted 177 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +18 -2
-     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 */
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+     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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 11 lines
drivers/mmc/socfpga_dw_mmc.c +11 -0
+      * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -0
+                     if(cr_rect_cand == 1)
+                             continue;
+
+                     /*
+                      * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                      * descending order
+                      */
+                     for(k = cr_rect_cand - 1; k > 1; k--){
+                             if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                     rect_cands[k-1] = rect_cands[k];
+                                     rect_cands[k] = tmp;
+                             } else
+                                     break;
+                     }
+             }
+     }
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines

+ /* Try to fit the rectangle candidates, in descending area order
*/ + for(k = 0; k < CAL_ROWS * CAL_COLS; k++) {

drivers/mmc/socfpga_dw_mmc.c +6 -0
+             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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +7 -0
+                  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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 2 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+                                  add_col++) {
+                                     for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 3 lines
drivers/mmc/socfpga_dw_mmc.c +3 -0
+                                          add_row++) {
+                                             if (!cal_results
+                                                [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +9 -0
+                                                + add_col]) {
+                                                     ok = 0;
+                                                     break;
+                                             }
+                                     }
+                             }
+
+                             /*
+                              * Return 'middle' of rectangle in case

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                              * success
+                              */
+                             if (ok) {
+                                     if(rect_width > 1)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                             rect_width--;
+
+                                     if(rect_height > 1)
+                                             rect_height--;
+
+                                     *cal_row = start_row +
+                                                (rect_height / 2);
+                                     *cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+
+                                     return 0;
+                             }
+                     }
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 6 lines
Quoted 5 lines
Quoted 4 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 6 lines

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

#16 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 223 lines · Simon Glass

Hi,

Quoted 201 lines · Dinh Nguyen

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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.

I think there's something wrong with your git scripts, I did not get this email.

Quoted 177 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +18 -2
-     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 */
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+     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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 11 lines
drivers/mmc/socfpga_dw_mmc.c +11 -0
+      * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -0
+                     if(cr_rect_cand == 1)
+                             continue;
+
+                     /*
+                      * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                      * descending order
+                      */
+                     for(k = cr_rect_cand - 1; k > 1; k--){
+                             if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                     rect_cands[k-1] = rect_cands[k];
+                                     rect_cands[k] = tmp;
+                             } else
+                                     break;
+                     }
+             }
+     }
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines

+ /* Try to fit the rectangle candidates, in descending area order
*/ + for(k = 0; k < CAL_ROWS * CAL_COLS; k++) {

drivers/mmc/socfpga_dw_mmc.c +6 -0
+             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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +7 -0
+                  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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 2 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+                                  add_col++) {
+                                     for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 3 lines
drivers/mmc/socfpga_dw_mmc.c +3 -0
+                                          add_row++) {
+                                             if (!cal_results
+                                                [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +9 -0
+                                                + add_col]) {
+                                                     ok = 0;
+                                                     break;
+                                             }
+                                     }
+                             }
+
+                             /*
+                              * Return 'middle' of rectangle in case

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                              * success
+                              */
+                             if (ok) {
+                                     if(rect_width > 1)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                             rect_width--;
+
+                                     if(rect_height > 1)
+                                             rect_height--;
+
+                                     *cal_row = start_row +
+                                                (rect_height / 2);
+                                     *cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+
+                                     return 0;
+                             }
+                     }
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 6 lines
Quoted 5 lines
Quoted 4 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 9 lines
Quoted 6 lines

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

Quoted 2 lines

Regards,
Simon

#17 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi,

Quoted 234 lines · Chin Liang See
Quoted 223 lines · Simon Glass

Hi,

Quoted 201 lines · Dinh Nguyen

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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.

I think there's something wrong with your git scripts, I did not get this email.

Quoted 177 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +18 -2
-     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 */
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+     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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 11 lines
drivers/mmc/socfpga_dw_mmc.c +11 -0
+      * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -0
+                     if(cr_rect_cand == 1)
+                             continue;
+
+                     /*
+                      * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                      * descending order
+                      */
+                     for(k = cr_rect_cand - 1; k > 1; k--){
+                             if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                     rect_cands[k-1] = rect_cands[k];
+                                     rect_cands[k] = tmp;
+                             } else
+                                     break;
+                     }
+             }
+     }
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines

+ /* Try to fit the rectangle candidates, in descending area order
*/ + for(k = 0; k < CAL_ROWS * CAL_COLS; k++) {

drivers/mmc/socfpga_dw_mmc.c +6 -0
+             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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +7 -0
+                  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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 2 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+                                  add_col++) {
+                                     for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 3 lines
drivers/mmc/socfpga_dw_mmc.c +3 -0
+                                          add_row++) {
+                                             if (!cal_results
+                                                [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +9 -0
+                                                + add_col]) {
+                                                     ok = 0;
+                                                     break;
+                                             }
+                                     }
+                             }
+
+                             /*
+                              * Return 'middle' of rectangle in case

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                              * success
+                              */
+                             if (ok) {
+                                     if(rect_width > 1)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                             rect_width--;
+
+                                     if(rect_height > 1)
+                                             rect_height--;
+
+                                     *cal_row = start_row +
+                                                (rect_height / 2);
+                                     *cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+
+                                     return 0;
+                             }
+                     }
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 6 lines
Quoted 5 lines
Quoted 4 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 9 lines
Quoted 6 lines

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.

Quoted 6 lines

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

Quoted 13 lines

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

Quoted 2 lines

Regards,
Simon

#18 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 239 lines · Jaehoon Chung

Hi,

Quoted 236 lines · Chin Liang See
Quoted 223 lines · Simon Glass

Hi,

Quoted 201 lines · Dinh Nguyen

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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.

I think there's something wrong with your git scripts, I did not get this email.

Quoted 177 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +18 -2
-     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 */
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+     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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 11 lines
drivers/mmc/socfpga_dw_mmc.c +11 -0
+      * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -0
+                     if(cr_rect_cand == 1)
+                             continue;
+
+                     /*
+                      * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                      * descending order
+                      */
+                     for(k = cr_rect_cand - 1; k > 1; k--){
+                             if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                     rect_cands[k-1] = rect_cands[k];
+                                     rect_cands[k] = tmp;
+                             } else
+                                     break;
+                     }
+             }
+     }
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines

+ /* Try to fit the rectangle candidates, in descending area order
*/ + for(k = 0; k < CAL_ROWS * CAL_COLS; k++) {

drivers/mmc/socfpga_dw_mmc.c +6 -0
+             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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +7 -0
+                  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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 2 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+                                  add_col++) {
+                                     for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 3 lines
drivers/mmc/socfpga_dw_mmc.c +3 -0
+                                          add_row++) {
+                                             if (!cal_results
+                                                [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +9 -0
+                                                + add_col]) {
+                                                     ok = 0;
+                                                     break;
+                                             }
+                                     }
+                             }
+
+                             /*
+                              * Return 'middle' of rectangle in case

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                              * success
+                              */
+                             if (ok) {
+                                     if(rect_width > 1)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                             rect_width--;
+
+                                     if(rect_height > 1)
+                                             rect_height--;
+
+                                     *cal_row = start_row +
+                                                (rect_height / 2);
+                                     *cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+
+                                     return 0;
+                             }
+                     }
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 6 lines
Quoted 5 lines
Quoted 4 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 9 lines
Quoted 6 lines

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.

Quoted 7 lines
Quoted 4 lines

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

Quoted 18 lines

Best Regards,
Jaehoon Chung

Quoted 13 lines

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

Quoted 2 lines

Regards,
Simon

#19 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi,

Quoted 238 lines · Chin Liang See
Quoted 237 lines · Jaehoon Chung

Hi,

Quoted 234 lines · Chin Liang See
Quoted 232 lines · Simon Glass

Hi,

Quoted 201 lines · Dinh Nguyen

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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.

I think there's something wrong with your git scripts, I did not get this email.

Quoted 177 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +18 -2
-     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 */
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+     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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 11 lines
drivers/mmc/socfpga_dw_mmc.c +11 -0
+      * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -0
+                     if(cr_rect_cand == 1)
+                             continue;
+
+                     /*
+                      * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                      * descending order
+                      */
+                     for(k = cr_rect_cand - 1; k > 1; k--){
+                             if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                     rect_cands[k-1] = rect_cands[k];
+                                     rect_cands[k] = tmp;
+                             } else
+                                     break;
+                     }
+             }
+     }
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines

+ /* Try to fit the rectangle candidates, in descending area order
*/ + for(k = 0; k < CAL_ROWS * CAL_COLS; k++) {

drivers/mmc/socfpga_dw_mmc.c +6 -0
+             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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +7 -0
+                  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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 2 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+                                  add_col++) {
+                                     for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 3 lines
drivers/mmc/socfpga_dw_mmc.c +3 -0
+                                          add_row++) {
+                                             if (!cal_results
+                                                [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +9 -0
+                                                + add_col]) {
+                                                     ok = 0;
+                                                     break;
+                                             }
+                                     }
+                             }
+
+                             /*
+                              * Return 'middle' of rectangle in case

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                              * success
+                              */
+                             if (ok) {
+                                     if(rect_width > 1)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                             rect_width--;
+
+                                     if(rect_height > 1)
+                                             rect_height--;
+
+                                     *cal_row = start_row +
+                                                (rect_height / 2);
+                                     *cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+
+                                     return 0;
+                             }
+                     }
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 6 lines
Quoted 5 lines
Quoted 4 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 6 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

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

Quoted 36 lines
Quoted 3 lines

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

Quoted 18 lines

Best Regards,
Jaehoon Chung

Quoted 13 lines

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

Quoted 2 lines

Regards,
Simon

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#20 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 252 lines · Jaehoon Chung

Hi,

Quoted 238 lines · Chin Liang See
Quoted 237 lines · Jaehoon Chung

Hi,

Quoted 234 lines · Chin Liang See
Quoted 232 lines · Simon Glass

Hi,

Quoted 201 lines · Dinh Nguyen

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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.

I think there's something wrong with your git scripts, I did not get this email.

Quoted 177 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Cc Pavel Machek <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Rini <[email protected]>

---

 drivers/mmc/socfpga_dw_mmc.c |  187

++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 180 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/socfpga_dw_mmc.c index eb69aed..15e537c 100644

drivers/mmc/socfpga_dw_mmc.c +6 -1
@@ -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)

{

drivers/mmc/socfpga_dw_mmc.c +18 -2
-     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 */
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+     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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 11 lines
drivers/mmc/socfpga_dw_mmc.c +11 -0
+      * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -0
+                     if(cr_rect_cand == 1)
+                             continue;
+
+                     /*
+                      * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                      * descending order
+                      */
+                     for(k = cr_rect_cand - 1; k > 1; k--){
+                             if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                     rect_cands[k-1] = rect_cands[k];
+                                     rect_cands[k] = tmp;
+                             } else
+                                     break;
+                     }
+             }
+     }
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines

+ /* Try to fit the rectangle candidates, in descending area order
*/ + for(k = 0; k < CAL_ROWS * CAL_COLS; k++) {

drivers/mmc/socfpga_dw_mmc.c +6 -0
+             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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 7 lines
drivers/mmc/socfpga_dw_mmc.c +7 -0
+                  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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 2 lines
drivers/mmc/socfpga_dw_mmc.c +2 -0
+                                  add_col++) {
+                                     for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 3 lines
drivers/mmc/socfpga_dw_mmc.c +3 -0
+                                          add_row++) {
+                                             if (!cal_results
+                                                [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 9 lines
drivers/mmc/socfpga_dw_mmc.c +9 -0
+                                                + add_col]) {
+                                                     ok = 0;
+                                                     break;
+                                             }
+                                     }
+                             }
+
+                             /*
+                              * Return 'middle' of rectangle in case

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+                              * success
+                              */
+                             if (ok) {
+                                     if(rect_width > 1)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 8 lines
drivers/mmc/socfpga_dw_mmc.c +8 -0
+                                             rect_width--;
+
+                                     if(rect_height > 1)
+                                             rect_height--;
+
+                                     *cal_row = start_row +
+                                                (rect_height / 2);
+                                     *cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Quoted 4 lines
drivers/mmc/socfpga_dw_mmc.c +4 -0
+
+                                     return 0;
+                             }
+                     }
Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c

Quoted 5 lines
drivers/mmc/socfpga_dw_mmc.c +5 -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

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 6 lines
Quoted 5 lines
Quoted 4 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 6 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

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.

Quoted 6 lines

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

Quoted 40 lines

Best Regards,
Jaehoon Chung

Quoted 36 lines
Quoted 3 lines

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

Quoted 18 lines

Best Regards,
Jaehoon Chung

Quoted 13 lines

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

Quoted 2 lines

Regards,
Simon

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#21 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 257 lines · Chin Liang See
Quoted 256 lines · Jaehoon Chung

Hi,

Quoted 238 lines · Chin Liang See
Quoted 237 lines · Jaehoon Chung

Hi,

Quoted 234 lines · Chin Liang See
Quoted 232 lines · Simon Glass

Hi,

Quoted 201 lines · Dinh Nguyen

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Chin Liang See
Quoted 7 lines

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 <[email protected]>
Cc: Dinh Nguyen <[email protected]>

I think there's something wrong with your git scripts, I did not get this email.

Quoted 177 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines

Cc: Pavel Machek <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Wolfgang Denk <[email protected]>
Cc: Stefan Roese <[email protected]>
Cc: Tom Rini <[email protected]>
---

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

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ 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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

+ * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

+ if(cr_rect_cand == 1)
+ continue;
+
+ /*
+ * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

+ * descending order
+ */
+ for(k = cr_rect_cand - 1; k > 1; k--){
+ if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ rect_cands[k-1] = rect_cands[k];
+ rect_cands[k] = tmp;
+ } else
+ break;
+ }
+ }
+ }
+

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ /* 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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

+ 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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

+ add_col++) {
+ for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

+ add_row++) {
+ if (!cal_results
+ [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

+ + add_col]) {
+ ok = 0;
+ break;
+ }
+ }
+ }
+
+ /*
+ * Return 'middle' of rectangle in case

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

+ * success
+ */
+ if (ok) {
+ if(rect_width > 1)

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ rect_width--;
+
+ if(rect_height > 1)
+ rect_height--;
+
+ *cal_row = start_row +
+ (rect_height / 2);
+ *cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Quoted 4 lines

+
+ return 0;
+ }
+ }

Quoted 8 lines
Quoted 5 lines

+ }
+ }
+
+ /* 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

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 6 lines
Quoted 5 lines
Quoted 4 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 6 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

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.

Quoted 7 lines
Quoted 2 lines

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?

Quoted 4 lines

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

Quoted 51 lines

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

Quoted 40 lines

Best Regards,
Jaehoon Chung

Quoted 36 lines
Quoted 3 lines

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

Quoted 18 lines

Best Regards,
Jaehoon Chung

Quoted 13 lines

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

Quoted 2 lines

Regards,
Simon

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#22 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 261 lines · Jaehoon Chung
Quoted 260 lines · Chin Liang See
Quoted 258 lines · Jaehoon Chung

Hi,

Quoted 238 lines · Chin Liang See
Quoted 237 lines · Jaehoon Chung

Hi,

Quoted 234 lines · Chin Liang See
Quoted 232 lines · Simon Glass

Hi,

Quoted 201 lines · Dinh Nguyen

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Chin Liang See

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 <[email protected]>
Cc: Dinh Nguyen <[email protected]>

I think there's something wrong with your git scripts, I did not get this email.

Quoted 177 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines

Cc: Pavel Machek <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Wolfgang Denk <[email protected]>
Cc: Stefan Roese <[email protected]>
Cc: Tom Rini <[email protected]>
---

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

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ 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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

+ * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

+ if(cr_rect_cand == 1)
+ continue;
+
+ /*
+ * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

+ * descending order
+ */
+ for(k = cr_rect_cand - 1; k > 1; k--){
+ if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ rect_cands[k-1] = rect_cands[k];
+ rect_cands[k] = tmp;
+ } else
+ break;
+ }
+ }
+ }
+

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ /* 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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

+ 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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

+ add_col++) {
+ for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

+ add_row++) {
+ if (!cal_results
+ [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

+ + add_col]) {
+ ok = 0;
+ break;
+ }
+ }
+ }
+
+ /*
+ * Return 'middle' of rectangle in case

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

+ * success
+ */
+ if (ok) {
+ if(rect_width > 1)

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ rect_width--;
+
+ if(rect_height > 1)
+ rect_height--;
+
+ *cal_row = start_row +
+ (rect_height / 2);
+ *cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 12 lines

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

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 6 lines
Quoted 5 lines
Quoted 4 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 6 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 8 lines
Quoted 5 lines

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

Quoted 56 lines

Best Regards,
Jaehoon Chung

Quoted 51 lines

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

Quoted 40 lines

Best Regards,
Jaehoon Chung

Quoted 36 lines
Quoted 3 lines

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

Quoted 18 lines

Best Regards,
Jaehoon Chung

Quoted 13 lines

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

Quoted 2 lines

Regards,
Simon

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#23 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 275 lines · Chin Liang See
Quoted 261 lines · Jaehoon Chung
Quoted 260 lines · Chin Liang See
Quoted 258 lines · Jaehoon Chung

Hi,

Quoted 238 lines · Chin Liang See
Quoted 237 lines · Jaehoon Chung

Hi,

Quoted 234 lines · Chin Liang See
Quoted 232 lines · Simon Glass

Hi,

Quoted 201 lines · Dinh Nguyen

+CC: Simon Glass

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Chin Liang See

Hi,

Quoted 6 lines · marex at denx.de
Quoted 1 line · Chin Liang See

Hi,

Hi again,

Quoted 1 line · marex at denx.de
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

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 <[email protected]>
Cc: Dinh Nguyen <[email protected]>

I think there's something wrong with your git scripts, I did not get this email.

Quoted 177 lines
Quoted 70 lines
Quoted 70 lines
Quoted 70 lines

Cc: Pavel Machek <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Wolfgang Denk <[email protected]>
Cc: Stefan Roese <[email protected]>
Cc: Tom Rini <[email protected]>
---

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

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ 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

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

+ * 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 */

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

+ if(cr_rect_cand == 1)
+ continue;
+
+ /*
+ * Put the candidate in right location to

maintain

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

+ * descending order
+ */
+ for(k = cr_rect_cand - 1; k > 1; k--){
+ if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ rect_cands[k-1] = rect_cands[k];
+ rect_cands[k] = tmp;
+ } else
+ break;
+ }
+ }
+ }
+

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ /* 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);

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

+ 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

*/

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ for (add_col = 0; (add_col < rect_width)

&& ok;

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

+ add_col++) {
+ for (add_row = 0; add_row <

rect_height;

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

+ add_row++) {
+ if (!cal_results
+ [start_row+add_row]

[start_col

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

+ + add_col]) {
+ ok = 0;
+ break;
+ }
+ }
+ }
+
+ /*
+ * Return 'middle' of rectangle in case

of

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

+ * success
+ */
+ if (ok) {
+ if(rect_width > 1)

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ rect_width--;
+
+ if(rect_height > 1)
+ rect_height--;
+
+ *cal_row = start_row +
+ (rect_height / 2);
+ *cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines

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

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 6 lines
Quoted 5 lines
Quoted 4 lines

I am not sure. But definitely we can make the calibration common enough if they required the calibration too. Let wait feedback from Simon.

Quoted 6 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 3 lines

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

Quoted 6 lines
Quoted 1 line

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

Quoted 75 lines
Quoted 9 lines
Quoted 6 lines

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

Quoted 56 lines

Best Regards,
Jaehoon Chung

Quoted 51 lines

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

Quoted 40 lines

Best Regards,
Jaehoon Chung

Quoted 36 lines
Quoted 3 lines

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

Quoted 18 lines

Best Regards,
Jaehoon Chung

Quoted 13 lines

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

Quoted 2 lines

Regards,
Simon

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#24 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 15 lines · Chin Liang See
Quoted 14 lines · Chin Liang See
Quoted 13 lines · Jaehoon Chung
Quoted 12 lines · Chin Liang See
Quoted 11 lines · Jaehoon Chung

Hi,

Quoted 8 lines · Chin Liang See
Quoted 7 lines · Jaehoon Chung

Hi,

Quoted 4 lines · Chin Liang See
Quoted 3 lines · Simon Glass

Hi,

On 20 August 2015 at 15:55, Dinh Nguyen <[email protected]>

Quoted 3 lines · wrote:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

+CC: Simon Glass

On Thu, Aug 20, 2015 at 12:32 AM, Marek Vasut <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

On Thursday, August 20, 2015 at 07:28:02 AM, Chin Liang See

Quoted 4 lines · wrote:
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Hi,

Quoted 1 line · marex at denx.de

On Wednesday, August 19, 2015 at 10:21:17 AM, Chin Liang See

Quoted 6 lines · wrote:
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Hi,

Hi again,

On Wed, 2015-08-19 at 02:41 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 07:54:50 AM, Chin Liang See

Quoted 294 lines · wrote:
Quoted 294 lines
Quoted 277 lines
Quoted 277 lines
Quoted 276 lines
Quoted 258 lines
Quoted 258 lines
Quoted 258 lines
Quoted 257 lines
Quoted 230 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

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 <[email protected]>
Cc: Dinh Nguyen <[email protected]>

I think there's something wrong with your git scripts, I did not get this email.

Quoted 218 lines
Quoted 73 lines
Quoted 73 lines

Cc: Pavel Machek <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Wolfgang Denk <[email protected]>
Cc: Stefan Roese <[email protected]>
Cc: Tom Rini <[email protected]>
---

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

Quoted 8 lines
Quoted 8 lines

+ *cal_row = (CAL_ROWS - 1) / 2;
+ *cal_col = (CAL_COLS - 1) / 2;
+ return 0;
+ }
+
+ /*
+ * Create list of all possible sub-rectangles, in
descending

area

Quoted 5 lines
Quoted 5 lines

+ * order
+ */
+ for(height = CAL_ROWS; height >= 1; height--) {
+ for(width = CAL_COLS; width >= 1 ;
width--){ + /* Add a new rectangle

Quoted 8 lines
Quoted 8 lines

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

Quoted 6 lines
Quoted 6 lines

+ if(cr_rect_cand == 1)
+ continue;
+
+ /*
+ * Put the candidate in right
location to

maintain

Quoted 5 lines
Quoted 5 lines

+ * descending order
+ */
+ for(k = cr_rect_cand - 1; k > 1;
k--){ +
if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line
Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 10 lines
Quoted 10 lines

+ rect_cands[k-1] =
rect_cands[k]; +
rect_cands[k] = tmp; + }
else
+ break;
+ }
+ }
+ }
+
+ /* Try to fit the rectangle candidates, in

Quoted 8 lines
Quoted 8 lines

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

Quoted 9 lines
Quoted 9 lines

+ 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

*/

Quoted 2 lines
Quoted 2 lines

+ for (add_col = 0; (add_col
< rect_width)

&& ok;

Quoted 3 lines
Quoted 3 lines

+ add_col++) {
+ for (add_row = 0;
add_row <

rect_height;

Quoted 4 lines
Quoted 4 lines

+ add_row++) {
+ if
(!cal_results +
[start_row+add_row]

[start_col

Quoted 11 lines
Quoted 11 lines

+ +
add_col]) { +
ok = 0; +
break; +
}
+ }
+ }
+
+ /*
+ * Return 'middle' of
rectangle in case

of

Quoted 5 lines
Quoted 5 lines

+ * success
+ */
+ if (ok) {
+ if(rect_width > 1)
+

Quoted 8 lines
Quoted 8 lines

rect_width--; +
+ if(rect_height > 1)
+
rect_height--; +
+ *cal_row =
start_row + +
(rect_height / 2); +
*cal_col = start_col +

(rect_width / 2);

Quoted 16 lines
Quoted 16 lines

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

Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 10 lines
Quoted 3 lines
Quoted 3 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 6 lines

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?

Quoted 8 lines
Quoted 4 lines

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.

#25 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 334 lines · marex at denx.de
Quoted 15 lines · Chin Liang See
Quoted 14 lines · Chin Liang See
Quoted 13 lines · Jaehoon Chung
Quoted 12 lines · Chin Liang See
Quoted 11 lines · Jaehoon Chung

Hi,

Quoted 8 lines · Chin Liang See
Quoted 7 lines · Jaehoon Chung

Hi,

Quoted 4 lines · Chin Liang See
Quoted 3 lines · Simon Glass

Hi,

On 20 August 2015 at 15:55, Dinh Nguyen <[email protected]>

Quoted 3 lines · wrote:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

+CC: Simon Glass

On Thu, Aug 20, 2015 at 12:32 AM, Marek Vasut <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

On Thursday, August 20, 2015 at 07:28:02 AM, Chin Liang See

Quoted 4 lines · wrote:
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Hi,

On Wed, 2015-08-19 at 14:36 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 10:21:17 AM, Chin Liang See

Quoted 6 lines · wrote:
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Hi,

Hi again,

On Wed, 2015-08-19 at 02:41 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 07:54:50 AM, Chin Liang See

Quoted 299 lines · wrote:
Quoted 298 lines
Quoted 277 lines
Quoted 277 lines
Quoted 276 lines
Quoted 258 lines
Quoted 258 lines
Quoted 258 lines
Quoted 257 lines
Quoted 230 lines
Quoted 8 lines
Quoted 8 lines

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 <[email protected]>
Cc: Dinh Nguyen <[email protected]>

I think there's something wrong with your git scripts, I did not get this email.

Quoted 218 lines
Quoted 81 lines

Cc: Pavel Machek <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Wolfgang Denk <[email protected]>
Cc: Stefan Roese <[email protected]>
Cc: Tom Rini <[email protected]>
---

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

Quoted 13 lines

+ * 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 */

Quoted 6 lines

+ if(cr_rect_cand == 1)
+ continue;
+
+ /*
+ * Put the candidate in right
location to

maintain

Quoted 5 lines

+ * descending order
+ */
+ for(k = cr_rect_cand - 1; k > 1;
k--){ +
if(rect_cands[k-1].area <

rect_cands[k].area){

Quoted 1 line

+ rect_cand_t tmp =

rect_cands[k-1];

Quoted 18 lines

+ 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);

Quoted 9 lines

+ 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

*/

Quoted 2 lines

+ for (add_col = 0; (add_col
< rect_width)

&& ok;

Quoted 3 lines

+ add_col++) {
+ for (add_row = 0;
add_row <

rect_height;

Quoted 4 lines

+ add_row++) {
+ if
(!cal_results +
[start_row+add_row]

[start_col

Quoted 11 lines

+ +
add_col]) { +
ok = 0; +
break; +
}
+ }
+ }
+
+ /*
+ * Return 'middle' of
rectangle in case

of

Quoted 13 lines

+ * 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);

Quoted 16 lines

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

Quoted 8 lines
Quoted 5 lines
Quoted 5 lines

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,

Quoted 10 lines
Quoted 3 lines
Quoted 3 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 5 lines
Quoted 3 lines

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

#26 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 335 lines · Chin Liang See
Quoted 334 lines · marex at denx.de
Quoted 15 lines · Chin Liang See
Quoted 14 lines · Chin Liang See
Quoted 13 lines · Jaehoon Chung
Quoted 12 lines · Chin Liang See
Quoted 11 lines · Jaehoon Chung

Hi,

Quoted 8 lines · Chin Liang See
Quoted 7 lines · Jaehoon Chung

Hi,

Quoted 4 lines · Chin Liang See
Quoted 3 lines · Simon Glass

Hi,

On 20 August 2015 at 15:55, Dinh Nguyen <[email protected]>

Quoted 3 lines · wrote:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

+CC: Simon Glass

On Thu, Aug 20, 2015 at 12:32 AM, Marek Vasut <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

On Thursday, August 20, 2015 at 07:28:02 AM, Chin Liang See

Quoted 4 lines · wrote:
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Hi,

On Wed, 2015-08-19 at 14:36 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 10:21:17 AM, Chin Liang See

Quoted 6 lines · wrote:
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Hi,

Hi again,

On Wed, 2015-08-19 at 02:41 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 07:54:50 AM, Chin Liang See

Quoted 299 lines · wrote:
Quoted 298 lines
Quoted 277 lines
Quoted 277 lines
Quoted 276 lines
Quoted 258 lines
Quoted 258 lines
Quoted 258 lines
Quoted 257 lines
Quoted 230 lines
Quoted 8 lines

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 <[email protected]>
Cc: Dinh Nguyen <[email protected]>

I think there's something wrong with your git scripts, I did not get this email.

Quoted 218 lines

Cc: Pavel Machek <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Wolfgang Denk <[email protected]>
Cc: Stefan Roese <[email protected]>
Cc: Tom Rini <[email protected]>
---

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

Quoted 8 lines
Quoted 5 lines

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,

Quoted 10 lines
Quoted 3 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

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

Quoted 6 lines

Thanks
Chin Liang

#27 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 345 lines · Jaehoon Chung
Quoted 335 lines · Chin Liang See
Quoted 334 lines · marex at denx.de
Quoted 15 lines · Chin Liang See
Quoted 14 lines · Chin Liang See
Quoted 13 lines · Jaehoon Chung
Quoted 12 lines · Chin Liang See
Quoted 11 lines · Jaehoon Chung

Hi,

Quoted 8 lines · Chin Liang See
Quoted 7 lines · Jaehoon Chung

Hi,

Quoted 4 lines · Chin Liang See
Quoted 3 lines · Simon Glass

Hi,

On 20 August 2015 at 15:55, Dinh Nguyen <[email protected]>

Quoted 3 lines · wrote:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

+CC: Simon Glass

On Thu, Aug 20, 2015 at 12:32 AM, Marek Vasut <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

On Thursday, August 20, 2015 at 07:28:02 AM, Chin Liang See

Quoted 4 lines · wrote:
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Hi,

On Wed, 2015-08-19 at 14:36 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 10:21:17 AM, Chin Liang See

Quoted 6 lines · wrote:
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Hi,

Hi again,

On Wed, 2015-08-19 at 02:41 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 07:54:50 AM, Chin Liang See

Quoted 299 lines · wrote:
Quoted 298 lines
Quoted 277 lines
Quoted 277 lines
Quoted 276 lines
Quoted 258 lines
Quoted 258 lines
Quoted 258 lines
Quoted 257 lines
Quoted 238 lines

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 <[email protected]>
Cc: Dinh Nguyen <[email protected]>

I think there's something wrong with your git scripts, I did
not get this email.

Cc: Pavel Machek <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Wolfgang Denk <[email protected]>
Cc: Stefan Roese <[email protected]>
Cc: Tom Rini <[email protected]>
---

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,

Quoted 10 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

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.

Quoted 8 lines

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

#28 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 370 lines · Dinh Nguyen
Quoted 369 lines · Jaehoon Chung
Quoted 355 lines · Chin Liang See
Quoted 354 lines · marex at denx.de
Quoted 15 lines · Chin Liang See
Quoted 14 lines · Chin Liang See
Quoted 13 lines · Jaehoon Chung
Quoted 12 lines · Chin Liang See
Quoted 11 lines · Jaehoon Chung

Hi,

Quoted 8 lines · Chin Liang See
Quoted 7 lines · Jaehoon Chung

Hi,

Quoted 4 lines · Chin Liang See
Quoted 3 lines · Simon Glass

Hi,

On 20 August 2015 at 15:55, Dinh Nguyen <[email protected]>

Quoted 3 lines · wrote:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

+CC: Simon Glass

On Thu, Aug 20, 2015 at 12:32 AM, Marek Vasut <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

On Thursday, August 20, 2015 at 07:28:02 AM, Chin Liang See

Quoted 5 lines · wrote:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

Hi,

On Wed, 2015-08-19 at 14:36 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 10:21:17 AM, Chin Liang
See

Quoted 7 lines · wrote:
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Hi,

Hi again,

On Wed, 2015-08-19 at 02:41 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 07:54:50 AM, Chin Liang
See

Quoted 312 lines · wrote:
Quoted 311 lines
Quoted 289 lines
Quoted 289 lines
Quoted 288 lines
Quoted 269 lines
Quoted 269 lines
Quoted 269 lines
Quoted 268 lines

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 <[email protected]>
Cc: Dinh Nguyen <[email protected]>

I think there's something wrong with your git scripts, I did
not get this email.

Cc: Pavel Machek <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Wolfgang Denk <[email protected]>
Cc: Stefan Roese <[email protected]>
Cc: Tom Rini <[email protected]>
---

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.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

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.

Quoted 7 lines
Quoted 2 lines

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 ?

#29 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 374 lines · marex at denx.de
Quoted 373 lines · Dinh Nguyen
Quoted 371 lines · Jaehoon Chung
Quoted 355 lines · Chin Liang See
Quoted 354 lines · marex at denx.de
Quoted 15 lines · Chin Liang See
Quoted 14 lines · Chin Liang See
Quoted 13 lines · Jaehoon Chung
Quoted 12 lines · Chin Liang See
Quoted 11 lines · Jaehoon Chung

Hi,

Quoted 8 lines · Chin Liang See
Quoted 7 lines · Jaehoon Chung

Hi,

Quoted 4 lines · Chin Liang See

On Fri, 2015-08-21 at 14:52 -0600, Simon Glass wrote:
Hi,

On 20 August 2015 at 15:55, Dinh Nguyen <[email protected]>

Quoted 3 lines · wrote:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

+CC: Simon Glass

On Thu, Aug 20, 2015 at 12:32 AM, Marek Vasut <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

On Thursday, August 20, 2015 at 07:28:02 AM, Chin Liang See

Quoted 5 lines · wrote:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

Hi,

On Wed, 2015-08-19 at 14:36 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 10:21:17 AM, Chin Liang
See

Quoted 7 lines · wrote:
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Hi,

Hi again,

On Wed, 2015-08-19 at 02:41 +0000, marex at denx.de wrote:
On Wednesday, August 19, 2015 at 07:54:50 AM, Chin Liang
See

Quoted 312 lines · wrote:
Quoted 311 lines
Quoted 289 lines
Quoted 289 lines
Quoted 288 lines
Quoted 269 lines
Quoted 269 lines
Quoted 269 lines

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 <[email protected]>
Cc: Dinh Nguyen <[email protected]>

I think there's something wrong with your git scripts, I did
not get this email.

Cc: Pavel Machek <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Wolfgang Denk <[email protected]>
Cc: Stefan Roese <[email protected]>
Cc: Tom Rini <[email protected]>
---

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.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

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.

Quoted 6 lines
Quoted 4 lines

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

#30 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi,

Quoted 1 line · Chin Liang See

On Wed, 2015-09-02 at 12:32 +0200, marex at denx.de wrote:

[snip]

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
Quoted 3 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

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

Quoted 8 lines

The value comes from device tree.

Thanks
Chin Liang

#31 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 1 line · Jaehoon Chung

Hi,

Hi,

Quoted 3 lines · Chin Liang See
Quoted 1 line · marex at denx.de

[snip]

thanks

Quoted 27 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Quoted 2 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

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.

Quoted 5 lines

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

#32 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi,

Quoted 42 lines · marex at denx.de
Quoted 1 line · Jaehoon Chung

Hi,

Hi,

Quoted 3 lines · Chin Liang See
Quoted 1 line · marex at denx.de

[snip]

thanks

Quoted 31 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Quoted 2 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

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.

Quoted 5 lines
Quoted 1 line

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

Quoted 3 lines

Best regards,
Marek Vasut

#33 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 47 lines · Chin Liang See

Hi,

Quoted 44 lines · marex at denx.de
Quoted 1 line · Jaehoon Chung

Hi,

Hi,

Quoted 3 lines · Chin Liang See
Quoted 1 line · marex at denx.de

[snip]

thanks

Quoted 33 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
Quoted 2 lines

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.

Quoted 7 lines
Quoted 7 lines
Quoted 5 lines

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.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

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

#34 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi!

Quoted 22 lines
Quoted 22 lines
Quoted 21 lines
Quoted 12 lines
Quoted 1 line
Quoted 1 line

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.

Quoted 5 lines

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.

Quoted 6 lines
Quoted 2 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#35 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi,

Quoted 31 lines · Pavel Machek

Hi!

Quoted 22 lines
Quoted 22 lines
Quoted 21 lines
Quoted 12 lines
Quoted 1 line
Quoted 1 line

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.

Quoted 5 lines

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.

Quoted 6 lines
Quoted 2 lines

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 ?

Quoted 5 lines

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

Quoted 2 lines

Pavel

#36 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi,

Quoted 37 lines · Jaehoon Chung

Hi,

Quoted 34 lines · Pavel Machek

Hi!

Quoted 22 lines
Quoted 22 lines
Quoted 21 lines
Quoted 12 lines
Quoted 1 line
Quoted 1 line

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.

Quoted 5 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 2 lines

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.

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

Quoted 11 lines

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.

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.

Quoted 1 line

5. Affect at booting time??

We measured this before and it took around ~5.2ms for worst case.

Thanks
Chin Liang

Quoted 8 lines

Best Regards,
Jaehoon Chung

Quoted 2 lines

Pavel

#37 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 1 line · Chin Liang See

Hi,

Hi,

Quoted 64 lines · Jaehoon Chung
Quoted 37 lines

Hi,

Quoted 34 lines · Pavel Machek

Hi!

Quoted 22 lines
Quoted 22 lines
Quoted 21 lines
Quoted 12 lines
Quoted 1 line
Quoted 1 line

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.

Quoted 5 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 2 lines

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.

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

Quoted 6 lines

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)

Quoted 6 lines

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.

Quoted 7 lines

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.

Quoted 1 line

5. 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 ?

A message that is not in the archive (sent off-list or not yet received): 0016CF5815A1B142902817051AF62EB305FDF5BA@PG-ITEXCH01.altera.priv.altera.com

#39 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Hi Marek,

Quoted 73 lines · Chin Liang See
Quoted 1 line

Hi,

Hi,

Quoted 68 lines · Jaehoon Chung
Quoted 37 lines

Hi,

Quoted 34 lines · Pavel Machek

Hi!

Quoted 22 lines
Quoted 22 lines
Quoted 21 lines
Quoted 12 lines
Quoted 1 line
Quoted 1 line

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.

Quoted 5 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 2 lines

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.

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

Quoted 6 lines

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)

Quoted 6 lines

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.

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.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

5. 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 ?

Sorry as missing out this email.
Yah, this sound good to me where user can use DT variant if concern about the boot time. Let me create that and sending in new revision.

Thanks
Chin Liang

#40 lore raw

PATCH 1/2 arm: socfpga: mmc: Enable calibration for drvsel and smpsel

Quoted 1 line · Chin Liang See

Hi Marek,

Hi!

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

5. Affect at booting time??

We measured this before and it took around ~5.2ms for worst case.

Quoted 8 lines
Quoted 6 lines

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 ?

Sorry as missing out this email.

No problem, I was also pretty busy myself.

Quoted 2 lines

Yah, this sound good to me where user can use DT variant if concern about the boot time. Let me create that and sending in new revision.

Thanks!

Best regards,
Marek Vasut