[U-Boot] [PATCH] mmc/dwmmc: remove recursive FIFO threshold setup

Removed code works properly only once after power-up because on every "first" invocation of "dwmci_init" existing value of "fifo_size" is used for calculation of itself. More over other bits in the same register (namely TX watermark and burst size) get corrupted (lost forever till the next power-toggle).
I understand that usually (in production systems) U-Boot starts only once and then passes control to Linux kernel etc. But when you do U-Boot debugging it might be loaded in memory via JTAG multiple times.
Also I wasn't able to find any instance of "host->fifoth_val" usage so I believe it's safe to just remove this code completely.
Another important thing - in the latest Linux kernel driver for DW MMC comtroller I don't see any utilization (configuration/modificatoin) of the register in question (SDMMC_FIFOTH). That's why I think we may want to keep it untouched as well so we don't force a situation when kernel driver gets DW MMC unexpectedly configured by U-Boot.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com
Cc: Mischa Jonker mjonker@synopsys.com Cc: Alim Akhtar alim.akhtar@samsung.com Cc: Rajeshwari Shinde rajeshwari.s@samsung.com Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Amar amarendra.xt@samsung.com Cc: Kyungmin Park kyungmin.park@samsung.com Cc: Hatim Ali hatim.rv@samsung.com Cc: Minkyu Kang mk7.kang@samsung.com Cc: Simon Glass sjg@chromium.org Cc: Pantelis Antoniou panto@antoniou-consulting.com Cc: Andy Fleming afleming@freescale.com --- drivers/mmc/dw_mmc.c | 9 --------- include/dwmmc.h | 1 - 2 files changed, 10 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 1e0f72b..166124d 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -300,7 +300,6 @@ static void dwmci_set_ios(struct mmc *mmc) static int dwmci_init(struct mmc *mmc) { struct dwmci_host *host = (struct dwmci_host *)mmc->priv; - u32 fifo_size;
if (host->quirks & DWMCI_QUIRK_DISABLE_SMU) { dwmci_writel(host, EMMCP_MPSBEGIN0, 0); @@ -330,14 +329,6 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_IDINTEN, 0); dwmci_writel(host, DWMCI_BMOD, 1);
- if (!host->fifoth_val) { - fifo_size = dwmci_readl(host, DWMCI_FIFOTH); - fifo_size = ((fifo_size & RX_WMARK_MASK) >> RX_WMARK_SHIFT) + 1; - host->fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size / 2 - 1) | - TX_WMARK(fifo_size / 2); - } - dwmci_writel(host, DWMCI_FIFOTH, host->fifoth_val); - dwmci_writel(host, DWMCI_CLKENA, 0); dwmci_writel(host, DWMCI_CLKSRC, 0);
diff --git a/include/dwmmc.h b/include/dwmmc.h index 6c91143..20cd1eb 100644 --- a/include/dwmmc.h +++ b/include/dwmmc.h @@ -137,7 +137,6 @@ struct dwmci_host { int dev_index; int buswidth; u32 clksel_val; - u32 fifoth_val; struct mmc *mmc;
void (*clksel)(struct dwmci_host *host);

Hi, Alexey,
I think good that use the initial fifoth value at register. Then we need not to change the value. But according to my experiment, some SoC needs to change the fifoth value.
On 11/27/2013 12:08 AM, Alexey Brodkin wrote:
Removed code works properly only once after power-up because on every "first" invocation of "dwmci_init" existing value of "fifo_size" is used for calculation of itself. More over other bits in the same register (namely TX watermark and burst size) get corrupted (lost forever till the next power-toggle).
I understand that usually (in production systems) U-Boot starts only once and then passes control to Linux kernel etc. But when you do U-Boot debugging it might be loaded in memory via JTAG multiple times.
Also I wasn't able to find any instance of "host->fifoth_val" usage so I believe it's safe to just remove this code completely.
If developer is need to change the fifoth value, the use the host->fifoth_val. So it's existed.
Another important thing - in the latest Linux kernel driver for DW MMC comtroller I don't see any utilization (configuration/modificatoin) of the register in question (SDMMC_FIFOTH). That's why I think we may want to keep it untouched as well so we don't force a situation when kernel driver gets DW MMC unexpectedly configured by U-Boot.
Right, if fifoth register is changed into U-boot, then kernel driver use the value that fifoth register was set to value at U-Boot. I known that kernel driver can set to the value with dt file.
Best Regards, Jaehoon Chung
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com
Cc: Mischa Jonker mjonker@synopsys.com Cc: Alim Akhtar alim.akhtar@samsung.com Cc: Rajeshwari Shinde rajeshwari.s@samsung.com Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Amar amarendra.xt@samsung.com Cc: Kyungmin Park kyungmin.park@samsung.com Cc: Hatim Ali hatim.rv@samsung.com Cc: Minkyu Kang mk7.kang@samsung.com Cc: Simon Glass sjg@chromium.org Cc: Pantelis Antoniou panto@antoniou-consulting.com Cc: Andy Fleming afleming@freescale.com
drivers/mmc/dw_mmc.c | 9 --------- include/dwmmc.h | 1 - 2 files changed, 10 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 1e0f72b..166124d 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -300,7 +300,6 @@ static void dwmci_set_ios(struct mmc *mmc) static int dwmci_init(struct mmc *mmc) { struct dwmci_host *host = (struct dwmci_host *)mmc->priv;
u32 fifo_size;
if (host->quirks & DWMCI_QUIRK_DISABLE_SMU) { dwmci_writel(host, EMMCP_MPSBEGIN0, 0);
@@ -330,14 +329,6 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_IDINTEN, 0); dwmci_writel(host, DWMCI_BMOD, 1);
- if (!host->fifoth_val) {
fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
fifo_size = ((fifo_size & RX_WMARK_MASK) >> RX_WMARK_SHIFT) + 1;
host->fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size / 2 - 1) |
TX_WMARK(fifo_size / 2);
- }
- dwmci_writel(host, DWMCI_FIFOTH, host->fifoth_val);
- dwmci_writel(host, DWMCI_CLKENA, 0); dwmci_writel(host, DWMCI_CLKSRC, 0);
diff --git a/include/dwmmc.h b/include/dwmmc.h index 6c91143..20cd1eb 100644 --- a/include/dwmmc.h +++ b/include/dwmmc.h @@ -137,7 +137,6 @@ struct dwmci_host { int dev_index; int buswidth; u32 clksel_val;
u32 fifoth_val; struct mmc *mmc;
void (*clksel)(struct dwmci_host *host);
participants (2)
-
Alexey Brodkin
-
Jaehoon Chung