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 fits here */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 in case 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 ?