On Tue, 2015-12-01 at 16:38 +0100, Marek Vasut wrote:
On Tuesday, December 01, 2015 at 04:32:44 PM, Chin Liang See wrote:
Hi Marek,
Hi!
On Sun, 2015-11-29 at 14:39 +0100, Marek Vasut wrote:
On Friday, November 27, 2015 at 08:22:03 AM, Chin Liang See wrote:
Enable SDMMC calibration to determine the best setting for drvsel and smplsel. Calibration will be triggered if the drvsel and smplsel node are not available in DTS.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: Pavel Machek pavel@denx.de Cc: Marek Vasut marex@denx.de Cc: Stefan Roese sr@denx.de Cc: Pantelis Antoniou pantelis.antoniou@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Jaehoon Chung jh80.chung@samsung.com
Changes for v4
- Calibration only run if node not in DTS
Changes for v3
- Remove the && ok as its redundant
Changes for v2
- Using standard error return macro
- Split to small function to avoid deep identation
- Fix coding standard
drivers/mmc/socfpga_dw_mmc.c | 208
++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 205 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c index 2bd0ebd..a8e2660 100644 --- a/drivers/mmc/socfpga_dw_mmc.c +++ b/drivers/mmc/socfpga_dw_mmc.c @@ -13,6 +13,7 @@
#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;@@ -25,7 +26,144 @@ struct dwmci_socfpga_priv_data {
unsigned int smplsel;
};
-static void socfpga_dwmci_clksel(struct dwmci_host *host) +/*
- rows and columns of calibration rectange. The values are
based on the value + * range of drvsel and smplsel register in system manager. Note drvsel 0 is + * unusable as it has meta-stability issue.
- */
+#define SOCFPGA_SD_DRVSEL 7 +#define SOCFPGA_SD_SMPLSEL 8
+static int socfpga_dwmci_find_row_col_fit_rectangle(unsigned rect_width,
- unsigned rect_height,
- unsigned char
cal_results[SOCFPGA_SD_DRVSEL][SOCFPGA_SD_SMPLSEL],
- unsigned int *cal_row, unsigned int *cal_col)
+{
- unsigned char start_row, start_col;
- /* Find the row and column where the candidate fits */
- for (start_col = 0; start_col < (SOCFPGA_SD_SMPLSEL -
rect_width + 1);
start_col++) {for (start_row = 0;start_row < (SOCFPGA_SD_DRVSEL -rect_height
- 1);
start_row++) {unsigned ok = 1;unsigned add_col, add_row;/* Determine if the rectangle fitshere */
for (add_col = 0; (add_col <rect_width); 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 incase of
* success*/I think you can refactor this indentation horror some more, right ?
Ok, let me use shorter variable to avoid going next line.
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;For example this condition can be inverted and it'd shave off one level of indent.
Ok let me take a look when doing the house keeping
That's really help, thanks/
}}- }
- return -EINVAL;
+}
[...]
@@ -102,8 +304,8 @@ static int socfpga_dwmci_of_probe(const void *blob, int node, const int idx) host->bus_hz = clk;
host->fifoth_val = MSIZE(0x2) |
RX_WMARK(fifo_depth / 2 - 1) |TX_WMARK(fifo_depth
/ 2);
- priv->drvsel = fdtdec_get_uint(blob, node, "drvsel",
3);
- priv->smplsel = fdtdec_get_uint(blob, node, "smplsel",
0);
- priv->drvsel = fdtdec_get_uint(blob, node, "drvsel",
0xf);
- priv->smplsel = fdtdec_get_uint(blob, node, "smplsel",
0xf);
This breaks multiple boards, since it misconfigures the default values.
The 0xf is non valid value. When the node is missing, we will get 0xf during the probe. When the init start, the non valid value will trigger the calibration to get the correct value.
OK, this is bad. Originally, if we didn't specify these in the DT, we would use the default values of 0x3 and 0x0 , but now we do the calibration. I wonder, do we care about DT ABI compatibility on the U-Boot level or not ?
If the compatibility failed, it will still invoke the calibration to get the correct value. Just that its in the cost of boot time.
At same time, I am rethinking another email thread on suggesting to put this into common dw_mmc code. Here are my proposal
1. Put back the correct default value for DT ABI compatiblity 2. Adding a node to enable calibration. If node not exist, calibration will disabled as default. 3. Some common calibration algo will go into dw_mmc.c 4. platform specific such as on configuring the clksel and smplsel will be part of platform specific dw_mmc file such as socfpga_dw_mmc.c
This hope will work for both everyone especially for socfpga and exynos platform. Any thoughts?
Thanks Chin Liang