Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On 5 August 2014 03:19, Sonny Rao sonny...@chromium.org wrote:
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
Acked-by: Seungwon Jeon tgih@samsung.com
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
[sonnyrao: fix compile for !CONFIG_MMC_DW_IDMAC case]
Thanks! Applied for next!
Kind regards
Uffe
---
drivers/mmc/host/dw_mmc.c | 87
++-
drivers/mmc/host/dw_mmc.h | 5 +++
2 files changed, 69 insertions(+), 23 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 1ac227c..39cf54f 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
};
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static bool dw_mci_reset(struct dw_mci *host);
#if defined(CONFIG_DEBUG_FS)
static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1235,7 +1234,7 @@ static int dw_mci_data_complete(struct dw_mci *host,
struct mmc_data *data)
* After an error, there may be data lingering
* in the FIFO
*/
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1352,7 +1351,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
/* CMD error in data command */
if (mrq-cmd-error mrq-data)
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
host-cmd = NULL;
host-data = NULL;
@@ -1963,14 +1962,8 @@ static void dw_mci_work_routine_card(struct
work_struct *work)
}
/* Power down slot */
- if (present == 0) {
- /* Clear down the FIFO */
- dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
- dw_mci_idmac_reset(host);
-#endif
-
- }
+ if (present == 0)
+ dw_mci_reset(host);
spin_unlock_bh(host-lock);
@@ -2208,8 +2201,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32
reset)
return false;
}
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static bool dw_mci_reset(struct dw_mci *host)
{
+ u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+ bool ret = false;
+
/*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
@@ -2219,15 +2215,60 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+ if (host-use_dma)
+ flags |= SDMMC_CTRL_DMA_RESET;
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
- return dw_mci_ctrl_reset(host,
-SDMMC_CTRL_FIFO_RESET |
-SDMMC_CTRL_RESET |
-SDMMC_CTRL_DMA_RESET);
+ if (dw_mci_ctrl_reset(host, flags)) {
+ /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+ mci_writel(host, RINTSTS, 0x);
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-use_dma) {
+ unsigned long timeout = jiffies +
msecs_to_jiffies(500);
+ u32 status;
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ) {
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req to
+ clear during reset\n, __func__);
+ goto ciu_out;
+ }
+
+ /* when using DMA next we reset the fifo again */
+ if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET))
+ goto ciu_out;
+
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
Hi,
I remembered that this patch was pushed at Ulf's tree.
Since dw_mci_idmac_reset() is located into #if CONFIG_MMC_DW_IDMAC,
it occurred the compiler error.
And it seems that didn't need to use IS_ENABLED() at there.
Acked-by: Jaehoon Chung jh80.ch...@samsung.com
Best Regards,
Jaehoon Chung
On 08/05/2014 10:19 AM, Sonny Rao wrote:
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
Acked-by: Seungwon Jeon tgih@samsung.com
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
[sonnyrao: fix compile for !CONFIG_MMC_DW_IDMAC case]
---
drivers/mmc/host/dw_mmc.c | 87
++-
drivers/mmc/host/dw_mmc.h | 5 +++
2 files changed, 69 insertions(+), 23 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 1ac227c..39cf54f 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
};
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static bool dw_mci_reset(struct dw_mci *host);
#if defined(CONFIG_DEBUG_FS)
static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1235,7 +1234,7 @@ static int dw_mci_data_complete(struct dw_mci *host,
struct mmc_data *data)
* After an error, there may be data lingering
* in the FIFO
*/
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1352,7 +1351,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
/* CMD error in data command */
if (mrq-cmd-error mrq-data)
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
host-cmd = NULL;
host-data = NULL;
@@ -1963,14 +1962,8 @@ static void dw_mci_work_routine_card(struct
work_struct *work)
}
/* Power down slot */
- if (present == 0) {
- /* Clear down the FIFO */
- dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
- dw_mci_idmac_reset(host);
-#endif
-
- }
+ if (present == 0)
+ dw_mci_reset(host);
spin_unlock_bh(host-lock);
@@ -2208,8 +2201,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32
reset)
return false;
}
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static bool dw_mci_reset(struct dw_mci *host)
{
+ u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+ bool ret = false;
+
/*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
@@ -2219,15 +2215,60 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+ if (host-use_dma)
+ flags |= SDMMC_CTRL_DMA_RESET;
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
- return dw_mci_ctrl_reset(host,
- SDMMC_CTRL_FIFO_RESET |
- SDMMC_CTRL_RESET |
- SDMMC_CTRL_DMA_RESET);
+ if (dw_mci_ctrl_reset(host, flags)) {
+ /*
+ * In all cases we clear the RAWINTS register to clear any
+ * interrupts.
+ */
+ mci_writel(host, RINTSTS, 0x);
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-use_dma) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status;
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ) {
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req to
+ clear during reset\n, __func__);
+ goto ciu_out;
+ }
+
+ /* when using DMA next we reset the fifo again */
+
RE: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On Fri, July 11, 2014, Sonny Rao wrote:
On Thu, Jul 10, 2014 at 5:28 AM, Seungwon Jeon tgih@samsung.com wrote:
Hi Sonny,
I have missed this patch.
You finally choose to take extra interrupt handling.
If it is not harm, it's fine.
Hi, thanks for coming back to it. Based on my tracing, the interrupt
seems to be okay and is just ignored.
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static inline bool dw_mci_reset(struct dw_mci *host)
inline is no needed anymore.
{
+ u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+ bool ret = false;
+
/*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
@@ -2334,15 +2330,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+ if (host-use_dma)
+ flags |= SDMMC_CTRL_DMA_RESET;
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
- return dw_mci_ctrl_reset(host,
- SDMMC_CTRL_FIFO_RESET |
- SDMMC_CTRL_RESET |
- SDMMC_CTRL_DMA_RESET);
+ if (dw_mci_ctrl_reset(host, flags)) {
+ /*
+ * In all cases we clear the RAWINTS register to clear any
+ * interrupts.
+ */
+ mci_writel(host, RINTSTS, 0x);
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-use_dma) {
+ unsigned long timeout = jiffies +
msecs_to_jiffies(500);
+ u32 status;
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ) {
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req to
+ clear during reset, __func__);
+ goto ciu_out;
+ }
+
+ /* when using DMA next we reset the fifo again */
+ if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET))
+ goto ciu_out;
+ }
+ } else {
+ /* if the controller reset bit did clear, then set clock
regs */
+ if (!(mci_readl(host, CTRL) SDMMC_CTRL_RESET)) {
+ dev_err(host-dev, %s: fifo/dma reset bits didn't
+ clear but ciu was reset, doing clock
update.,
+ __func__);
+ goto ciu_out;
+ }
+ }
+
+ if (IS_ENABLED(CONFIG_MMC_DW_IDMAC))
+ /* It is also recommended that we reset and reprogram idmac
*/
+ dw_mci_idmac_reset(host);
+
+ ret = true;
+
+ciu_out:
+ /* After a CTRL reset we need to have CIU set clock registers */
+ mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
+
+ return ret;
}
#ifdef CONFIG_OF
@@ -2555,7 +2595,7 @@ int dw_mci_probe(struct dw_mci *host)
}
/* Reset all blocks */
- if (!dw_mci_ctrl_all_reset(host))
+ if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS))
return -ENODEV;
host-dma_ops = host-pdata-dma_ops;
@@ -2744,7 +2784,7 @@ int dw_mci_resume(struct dw_mci *host)
}
}
- if (!dw_mci_ctrl_all_reset(host)) {
+ if (!dw_mci_reset(host)) {
Do you have any reason to use dw_mci_reset() unlike doing on probing?
I really wanted to use dw_mci_reset() everwhere, including probe,
because that would be simplest, where there is just one reset function
always, but the host structure is not completely set up at probe time,
so the code in dw_mci_reset() where we try to send a command to update
clock fails, so that's why I had to just do a reset.
Yes, if we can keep one interface, it would be good.
But can you check below?
Did you see the kernel panic on probing with host-cur_slot is NULL?
If right, resume seems not different from probe in case of removable type.
And dw_mci_idmac_reset() is redundant when dw_mci_reset() is used at resume.
I think dw_mci_ctrl_reset() can be also used at resume like at probe for simple
way.
For safety's sake, host-cur_slot should be guaranteed it's not NULL.
Thanks,
Seungwon Jeon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On Fri, Jul 11, 2014 at 3:20 AM, Seungwon Jeon tgih@samsung.com wrote:
On Fri, July 11, 2014, Sonny Rao wrote:
On Thu, Jul 10, 2014 at 5:28 AM, Seungwon Jeon tgih@samsung.com wrote:
Hi Sonny,
I have missed this patch.
You finally choose to take extra interrupt handling.
If it is not harm, it's fine.
Hi, thanks for coming back to it. Based on my tracing, the interrupt
seems to be okay and is just ignored.
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static inline bool dw_mci_reset(struct dw_mci *host)
inline is no needed anymore.
{
+ u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+ bool ret = false;
+
/*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
@@ -2334,15 +2330,59 @@ static inline bool dw_mci_fifo_reset(struct
dw_mci *host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+ if (host-use_dma)
+ flags |= SDMMC_CTRL_DMA_RESET;
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
- return dw_mci_ctrl_reset(host,
- SDMMC_CTRL_FIFO_RESET |
- SDMMC_CTRL_RESET |
- SDMMC_CTRL_DMA_RESET);
+ if (dw_mci_ctrl_reset(host, flags)) {
+ /*
+ * In all cases we clear the RAWINTS register to clear any
+ * interrupts.
+ */
+ mci_writel(host, RINTSTS, 0x);
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-use_dma) {
+ unsigned long timeout = jiffies +
msecs_to_jiffies(500);
+ u32 status;
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ) {
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req to
+ clear during reset, __func__);
+ goto ciu_out;
+ }
+
+ /* when using DMA next we reset the fifo again */
+ if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET))
+ goto ciu_out;
+ }
+ } else {
+ /* if the controller reset bit did clear, then set clock
regs */
+ if (!(mci_readl(host, CTRL) SDMMC_CTRL_RESET)) {
+ dev_err(host-dev, %s: fifo/dma reset bits didn't
+ clear but ciu was reset, doing clock
update.,
+ __func__);
+ goto ciu_out;
+ }
+ }
+
+ if (IS_ENABLED(CONFIG_MMC_DW_IDMAC))
+ /* It is also recommended that we reset and reprogram idmac
*/
+ dw_mci_idmac_reset(host);
+
+ ret = true;
+
+ciu_out:
+ /* After a CTRL reset we need to have CIU set clock registers */
+ mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
+
+ return ret;
}
#ifdef CONFIG_OF
@@ -2555,7 +2595,7 @@ int dw_mci_probe(struct dw_mci *host)
}
/* Reset all blocks */
- if (!dw_mci_ctrl_all_reset(host))
+ if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS))
return -ENODEV;
host-dma_ops = host-pdata-dma_ops;
@@ -2744,7 +2784,7 @@ int dw_mci_resume(struct dw_mci *host)
}
}
- if (!dw_mci_ctrl_all_reset(host)) {
+ if (!dw_mci_reset(host)) {
Do you have any reason to use dw_mci_reset() unlike doing on probing?
I really wanted to use dw_mci_reset() everwhere, including probe,
because that would be simplest, where there is just one reset function
always, but the host structure is not completely set up at probe time,
so the code in dw_mci_reset() where we try to send a command to update
clock fails, so that's why I had to just do a reset.
Yes, if we can keep one interface, it would be good.
But can you check below?
Did you see the kernel panic on probing with host-cur_slot is NULL?
Yes, I think that was the one.
If right, resume seems not different from probe in case of removable type.
And dw_mci_idmac_reset() is redundant when dw_mci_reset() is used at resume.
I think dw_mci_ctrl_reset() can be also used at resume like at probe for
simple way.
For safety's sake, host-cur_slot should be guaranteed it's not NULL.
Ok, I didn't try on removable, but I can change it to match probe,
thanks for looking at this.
Thanks,
RE: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
Hi Sonny,
I have missed this patch.
You finally choose to take extra interrupt handling.
If it is not harm, it's fine.
Please check one thing below.
On Tue, June 10, 2014, Sonny Rao wrote:
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
v2: Add Generic DMA support
per the documentation, move interrupt clear before wait
make the test for DMA host-use_dma rather than host-using_dma
add proper return values (although it appears no caller checks)
v3: rename fifo reset function, and change callers
use this combined reset function in dw_mci_resume()
just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
use DMA reset bit for all systems which use DMA
remove extra IDMAC reset in dw_mci_work_routine_card()
do CIU clock update in error path, if CIU reset cleared
v4: remove comment about FIFO reset in dw_mci_work_routine_card()
move down error message when control reset clears but others don't
and clarify the error stating that we will still update clocks
make flags for all reset bits a macro
drivers/mmc/host/dw_mmc.c | 86
++-
drivers/mmc/host/dw_mmc.h | 5 +++
2 files changed, 68 insertions(+), 23 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 55cd110..1d6d984 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
};
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static inline bool dw_mci_reset(struct dw_mci *host);
#if defined(CONFIG_DEBUG_FS)
static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host,
struct mmc_data *data)
* After an error, there may be data lingering
* in the FIFO
*/
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
/* CMD error in data command */
if (mrq-cmd-error mrq-data)
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
host-cmd = NULL;
host-data = NULL;
@@ -1982,14 +1981,8 @@ static void dw_mci_work_routine_card(struct
work_struct *work)
}
/* Power down slot */
- if (present == 0) {
- /* Clear down the FIFO */
- dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
- dw_mci_idmac_reset(host);
-#endif
-
- }
+ if (present == 0)
+ dw_mci_reset(host);
spin_unlock_bh(host-lock);
@@ -2323,8 +2316,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32
reset)
return false;
}
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static inline bool dw_mci_reset(struct dw_mci *host)
{
+ u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+ bool ret = false;
+
/*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
@@ -2334,15 +2330,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+ if (host-use_dma)
+ flags |= SDMMC_CTRL_DMA_RESET;
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
- return dw_mci_ctrl_reset(host,
- SDMMC_CTRL_FIFO_RESET |
- SDMMC_CTRL_RESET |
- SDMMC_CTRL_DMA_RESET);
+ if (dw_mci_ctrl_reset(host, flags)) {
+ /*
+ * In all cases we clear the RAWINTS register to clear any
+ * interrupts.
+ */
+ mci_writel(host, RINTSTS, 0x);
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-use_dma) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status;
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On Thu, Jul 10, 2014 at 5:28 AM, Seungwon Jeon tgih@samsung.com wrote:
Hi Sonny,
I have missed this patch.
You finally choose to take extra interrupt handling.
If it is not harm, it's fine.
Hi, thanks for coming back to it. Based on my tracing, the interrupt
seems to be okay and is just ignored.
Please check one thing below.
On Tue, June 10, 2014, Sonny Rao wrote:
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
v2: Add Generic DMA support
per the documentation, move interrupt clear before wait
make the test for DMA host-use_dma rather than host-using_dma
add proper return values (although it appears no caller checks)
v3: rename fifo reset function, and change callers
use this combined reset function in dw_mci_resume()
just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
use DMA reset bit for all systems which use DMA
remove extra IDMAC reset in dw_mci_work_routine_card()
do CIU clock update in error path, if CIU reset cleared
v4: remove comment about FIFO reset in dw_mci_work_routine_card()
move down error message when control reset clears but others don't
and clarify the error stating that we will still update clocks
make flags for all reset bits a macro
drivers/mmc/host/dw_mmc.c | 86
++-
drivers/mmc/host/dw_mmc.h | 5 +++
2 files changed, 68 insertions(+), 23 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 55cd110..1d6d984 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
};
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static inline bool dw_mci_reset(struct dw_mci *host);
#if defined(CONFIG_DEBUG_FS)
static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host,
struct mmc_data *data)
* After an error, there may be data lingering
* in the FIFO
*/
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
/* CMD error in data command */
if (mrq-cmd-error mrq-data)
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
host-cmd = NULL;
host-data = NULL;
@@ -1982,14 +1981,8 @@ static void dw_mci_work_routine_card(struct
work_struct *work)
}
/* Power down slot */
- if (present == 0) {
- /* Clear down the FIFO */
- dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
- dw_mci_idmac_reset(host);
-#endif
-
- }
+ if (present == 0)
+ dw_mci_reset(host);
spin_unlock_bh(host-lock);
@@ -2323,8 +2316,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host,
u32 reset)
return false;
}
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static inline bool dw_mci_reset(struct dw_mci *host)
{
+ u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+ bool ret = false;
+
/*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
@@ -2334,15 +2330,59 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+ if (host-use_dma)
+ flags |= SDMMC_CTRL_DMA_RESET;
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
- return dw_mci_ctrl_reset(host,
- SDMMC_CTRL_FIFO_RESET |
- SDMMC_CTRL_RESET |
- SDMMC_CTRL_DMA_RESET);
+ if (dw_mci_ctrl_reset(host, flags)) {
+ /*
+ * In all cases we clear the RAWINTS register to clear any
+ * interrupts.
+ */
+ mci_writel(host, RINTSTS, 0x);
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-use_dma) {
+ unsigned long timeout = jiffies +
msecs_to_jiffies(500);
+ u32 status;
+
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On Wed, May 28, 2014 at 10:17 PM, Jaehoon Chung jh80.ch...@samsung.com wrote:
Hi, Sonny.
On 05/29/2014 09:35 AM, Sonny Rao wrote:
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
v2: Add Generic DMA support
per the documentation, move interrupt clear before wait
make the test for DMA host-use_dma rather than host-using_dma
add proper return values (although it appears no caller checks)
v3: rename fifo reset function, and change callers
use this combined reset function in dw_mci_resume()
just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
use DMA reset bit for all systems which use DMA
remove extra IDMAC reset in dw_mci_work_routine_card()
do CIU clock update in error path, if CIU reset cleared
drivers/mmc/host/dw_mmc.c | 85
+++
drivers/mmc/host/dw_mmc.h | 1 +
2 files changed, 64 insertions(+), 22 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 55cd110..988492c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
};
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static inline bool dw_mci_reset(struct dw_mci *host);
#if defined(CONFIG_DEBUG_FS)
static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host,
struct mmc_data *data)
* After an error, there may be data lingering
* in the FIFO
*/
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
/* CMD error in data command */
if (mrq-cmd-error mrq-data)
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
host-cmd = NULL;
host-data = NULL;
@@ -1982,14 +1981,9 @@ static void dw_mci_work_routine_card(struct
work_struct *work)
}
/* Power down slot */
- if (present == 0) {
+ if (present == 0)
/* Clear down the FIFO */
Didn't Need to change the Comment?
Well, it does still clear the fifo, but I can remove the comment since
we are now doing more than that.
- dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
- dw_mci_idmac_reset(host);
-#endif
-
- }
+ dw_mci_reset(host);
spin_unlock_bh(host-lock);
@@ -2323,8 +2317,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host,
u32 reset)
return false;
}
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static inline bool dw_mci_reset(struct dw_mci *host)
{
+ u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+ bool ret = false;
+
/*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
@@ -2334,15 +2331,57 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+ if (host-use_dma)
+ flags |= SDMMC_CTRL_DMA_RESET;
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
- return dw_mci_ctrl_reset(host,
- SDMMC_CTRL_FIFO_RESET |
- SDMMC_CTRL_RESET |
- SDMMC_CTRL_DMA_RESET);
+ if (dw_mci_ctrl_reset(host, flags)) {
+ /*
+ * In all cases we clear the RAWINTS register to clear any
+ * interrupts.
+ */
+ mci_writel(host, RINTSTS, 0x);
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-use_dma) {
+ unsigned long timeout = jiffies +
msecs_to_jiffies(500);
+ u32 status;
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
Hi, Sonny.
On 05/29/2014 09:35 AM, Sonny Rao wrote:
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
---
v2: Add Generic DMA support
per the documentation, move interrupt clear before wait
make the test for DMA host-use_dma rather than host-using_dma
add proper return values (although it appears no caller checks)
v3: rename fifo reset function, and change callers
use this combined reset function in dw_mci_resume()
just one caller left (probe), so get rid of dw_mci_ctrl_all_reset()
use DMA reset bit for all systems which use DMA
remove extra IDMAC reset in dw_mci_work_routine_card()
do CIU clock update in error path, if CIU reset cleared
drivers/mmc/host/dw_mmc.c | 85
+++
drivers/mmc/host/dw_mmc.h | 1 +
2 files changed, 64 insertions(+), 22 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 55cd110..988492c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -111,8 +111,7 @@ static const u8 tuning_blk_pattern_8bit[] = {
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
};
-static inline bool dw_mci_fifo_reset(struct dw_mci *host);
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host);
+static inline bool dw_mci_reset(struct dw_mci *host);
#if defined(CONFIG_DEBUG_FS)
static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1254,7 +1253,7 @@ static int dw_mci_data_complete(struct dw_mci *host,
struct mmc_data *data)
* After an error, there may be data lingering
* in the FIFO
*/
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
} else {
data-bytes_xfered = data-blocks * data-blksz;
data-error = 0;
@@ -1371,7 +1370,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
/* CMD error in data command */
if (mrq-cmd-error mrq-data)
- dw_mci_fifo_reset(host);
+ dw_mci_reset(host);
host-cmd = NULL;
host-data = NULL;
@@ -1982,14 +1981,9 @@ static void dw_mci_work_routine_card(struct
work_struct *work)
}
/* Power down slot */
- if (present == 0) {
+ if (present == 0)
/* Clear down the FIFO */
Didn't Need to change the Comment?
- dw_mci_fifo_reset(host);
-#ifdef CONFIG_MMC_DW_IDMAC
- dw_mci_idmac_reset(host);
-#endif
-
- }
+ dw_mci_reset(host);
spin_unlock_bh(host-lock);
@@ -2323,8 +2317,11 @@ static bool dw_mci_ctrl_reset(struct dw_mci *host, u32
reset)
return false;
}
-static inline bool dw_mci_fifo_reset(struct dw_mci *host)
+static inline bool dw_mci_reset(struct dw_mci *host)
{
+ u32 flags = SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET;
+ bool ret = false;
+
/*
* Reseting generates a block interrupt, hence setting
* the scatter-gather pointer to NULL.
@@ -2334,15 +2331,57 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
-}
+ if (host-use_dma)
+ flags |= SDMMC_CTRL_DMA_RESET;
-static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
-{
- return dw_mci_ctrl_reset(host,
- SDMMC_CTRL_FIFO_RESET |
- SDMMC_CTRL_RESET |
- SDMMC_CTRL_DMA_RESET);
+ if (dw_mci_ctrl_reset(host, flags)) {
+ /*
+ * In all cases we clear the RAWINTS register to clear any
+ * interrupts.
+ */
+ mci_writel(host, RINTSTS, 0x);
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-use_dma) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status;
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ) {
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req to
+
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
Hi, On 12/05/14 22:44, Sonny Rao wrote: Doug mentioned that James Hogan might have an answer. James, are there Imgtec SoCs which use dw_mmc and use DMA but don't use the IDMAC? If so, we can add that support into this reset procedure patch. Yes, the Toumaz TZ1090 SoC has the dw_mmc configured without an IDMAC, so it uses the IMG DMAC block instead. Cheers James -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On Sat, May 10, 2014 at 7:08 AM, Seungwon Jeon tgih@samsung.com wrote:
Hi Sonny,
Can you separate procedure?
Reset all are handled in fifo-reset.
And ciu reset is always needed for error handling?
Yes according to the document in the Controller/DMA/FIFO Reset Usage
section, the controller_reset bit should be set in all cases, so I
don't think that it should be separated.
Thanks,
Seungwon Jeon
On Sat, May 10, 2014, Sonny Rao wrote:
On Fri, May 9, 2014 at 12:32 AM, Jaehoon Chung jh80.ch...@samsung.com
wrote:
Hi, Sonny.
You can discard the my previous some comment.
As you mentioned, this reset sequence is recommended at Synopsys TRM.
Add the minor question.
On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
Hi, Sonny.
I have checked the Synopsys TRM..
On 05/09/2014 10:34 AM, Sonny Rao wrote:
On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com
wrote:
On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D
yuvaraj...@gmail.com wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host
databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct
dw_mci *host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on
the mode.
+* Note that this doesn't handle the generic DMA (not
IDMAC) case.
+*/
not IDMAC is confused..
The documentation describes three different possible modes. There's a
mode that doesn't use IDMAC but still does DMA. But as far as I can
tell this driver doesn't support that way. We can just remove that
wording if it's confusing.
How did it know whether dma is generic DMA or not?
That's a good question. I wasn't sure whether the driver supported it
or not. It looks like it definitely supports IDMAC through the
CONFIG_MMC_DW_IDMAC flag, but I wasn't sure if it was supported the
generic dma. Maybe if CONFIG_MMC_DW_IDMAC isn't specified but
host-dma_ops is not NULL then we are using the generic dma mode.
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies +
msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for
dma_req to
+ clear during reset,
__func__);
+
+ /* when using DMA next we reset the fifo
again */
+ dw_mci_ctrl_reset(host,
SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to
clear any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
you use the status or temp instead of rint. (you can reuse the
variable.)
And can use status = ~mci_readl(host,MINTSTS);
Just clear the RINTSTS register? why do you add these?
This will look at what is not masked, and only clear those bits.
Well, i known if clear the RINTSTS register,
recommended to use 0xfff and set the value for interrupt.
Can be used 0xfff?
Yeah we probably can. We just lose information about interrupts that
were masked, but maybe we just don't care about any of them anyway, so
it doesn't matter.
Best Regards,
Jaehoon Chung
+
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On Fri, May 9, 2014 at 8:36 PM, Sonny Rao sonny...@chromium.org wrote:
On Fri, May 9, 2014 at 12:32 AM, Jaehoon Chung jh80.ch...@samsung.com wrote:
Hi, Sonny.
You can discard the my previous some comment.
As you mentioned, this reset sequence is recommended at Synopsys TRM.
Add the minor question.
On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
Hi, Sonny.
I have checked the Synopsys TRM..
On 05/09/2014 10:34 AM, Sonny Rao wrote:
On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com
wrote:
On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D
yuvaraj...@gmail.com wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct
dw_mci *host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on
the mode.
+* Note that this doesn't handle the generic DMA (not IDMAC)
case.
+*/
not IDMAC is confused..
The documentation describes three different possible modes. There's a
mode that doesn't use IDMAC but still does DMA. But as far as I can
tell this driver doesn't support that way. We can just remove that
wording if it's confusing.
How did it know whether dma is generic DMA or not?
That's a good question. I wasn't sure whether the driver supported it
or not. It looks like it definitely supports IDMAC through the
CONFIG_MMC_DW_IDMAC flag, but I wasn't sure if it was supported the
generic dma. Maybe if CONFIG_MMC_DW_IDMAC isn't specified but
host-dma_ops is not NULL then we are using the generic dma mode.
Doug mentioned that James Hogan might have an answer. James, are
there Imgtec SoCs which use dw_mmc and use DMA but don't use the
IDMAC? If so, we can add that support into this reset procedure
patch.
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for
dma_req to
+ clear during reset, __func__);
+
+ /* when using DMA next we reset the fifo again
*/
+ dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to clear
any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
you use the status or temp instead of rint. (you can reuse the variable.)
And can use status = ~mci_readl(host,MINTSTS);
Just clear the RINTSTS register? why do you add these?
This will look at what is not masked, and only clear those bits.
Well, i known if clear the RINTSTS register,
recommended to use 0xfff and set the value for interrupt.
Can be used 0xfff?
Yeah we probably can. We just lose information about interrupts that
were masked, but maybe we just don't care about any of them anyway, so
it doesn't matter.
Best Regards,
Jaehoon Chung
+ if (rint)
+ mci_writel(host, RINTSTS, rint);
+
+ } else
+ dev_err(host-dev, %s: Reset bits didn't clear,
__func__);
Just display the error log? I didn't understand this.
If you displayed the error log, then it needs to return the error value.
+
+
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On Mon, May 12, 2014 at 2:44 PM, Sonny Rao sonny...@chromium.org wrote:
On Fri, May 9, 2014 at 8:36 PM, Sonny Rao sonny...@chromium.org wrote:
On Fri, May 9, 2014 at 12:32 AM, Jaehoon Chung jh80.ch...@samsung.com
wrote:
Hi, Sonny.
You can discard the my previous some comment.
As you mentioned, this reset sequence is recommended at Synopsys TRM.
Add the minor question.
On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
Hi, Sonny.
I have checked the Synopsys TRM..
On 05/09/2014 10:34 AM, Sonny Rao wrote:
On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com
wrote:
On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D
yuvaraj...@gmail.com wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct
dw_mci *host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on
the mode.
+* Note that this doesn't handle the generic DMA (not IDMAC)
case.
+*/
not IDMAC is confused..
The documentation describes three different possible modes. There's a
mode that doesn't use IDMAC but still does DMA. But as far as I can
tell this driver doesn't support that way. We can just remove that
wording if it's confusing.
How did it know whether dma is generic DMA or not?
That's a good question. I wasn't sure whether the driver supported it
or not. It looks like it definitely supports IDMAC through the
CONFIG_MMC_DW_IDMAC flag, but I wasn't sure if it was supported the
generic dma. Maybe if CONFIG_MMC_DW_IDMAC isn't specified but
host-dma_ops is not NULL then we are using the generic dma mode.
Doug mentioned that James Hogan might have an answer. James, are
there Imgtec SoCs which use dw_mmc and use DMA but don't use the
IDMAC? If so, we can add that support into this reset procedure
patch.
In any case, whether on not anybody is using it, it looks like it's
not that hard to support this mode for reset (I was just lazy the
first time), we just need to add a flag to our reset. I've made that
change and cleaned it up a little bit more and I'll resend this patch.
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies +
msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for
dma_req to
+ clear during reset,
__func__);
+
+ /* when using DMA next we reset the fifo again
*/
+ dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to clear
any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
you use the status or temp instead of rint. (you can reuse the variable.)
And can use status = ~mci_readl(host,MINTSTS);
Just clear the RINTSTS register? why do you add these?
This will look at what is not masked, and only clear those bits.
Well, i known if clear the RINTSTS register,
recommended to use 0xfff and set the value for interrupt.
Can be used 0xfff?
Yeah we probably can. We just lose information about interrupts that
were masked, but maybe we just don't care about any of them anyway, so
it doesn't
RE: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
Hi Sonny,
Can you separate procedure?
Reset all are handled in fifo-reset.
And ciu reset is always needed for error handling?
Thanks,
Seungwon Jeon
On Sat, May 10, 2014, Sonny Rao wrote:
On Fri, May 9, 2014 at 12:32 AM, Jaehoon Chung jh80.ch...@samsung.com wrote:
Hi, Sonny.
You can discard the my previous some comment.
As you mentioned, this reset sequence is recommended at Synopsys TRM.
Add the minor question.
On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
Hi, Sonny.
I have checked the Synopsys TRM..
On 05/09/2014 10:34 AM, Sonny Rao wrote:
On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com
wrote:
On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D
yuvaraj...@gmail.com wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct
dw_mci *host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on
the mode.
+* Note that this doesn't handle the generic DMA (not IDMAC)
case.
+*/
not IDMAC is confused..
The documentation describes three different possible modes. There's a
mode that doesn't use IDMAC but still does DMA. But as far as I can
tell this driver doesn't support that way. We can just remove that
wording if it's confusing.
How did it know whether dma is generic DMA or not?
That's a good question. I wasn't sure whether the driver supported it
or not. It looks like it definitely supports IDMAC through the
CONFIG_MMC_DW_IDMAC flag, but I wasn't sure if it was supported the
generic dma. Maybe if CONFIG_MMC_DW_IDMAC isn't specified but
host-dma_ops is not NULL then we are using the generic dma mode.
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies +
msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for
dma_req to
+ clear during reset,
__func__);
+
+ /* when using DMA next we reset the fifo again
*/
+ dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to clear
any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
you use the status or temp instead of rint. (you can reuse the variable.)
And can use status = ~mci_readl(host,MINTSTS);
Just clear the RINTSTS register? why do you add these?
This will look at what is not masked, and only clear those bits.
Well, i known if clear the RINTSTS register,
recommended to use 0xfff and set the value for interrupt.
Can be used 0xfff?
Yeah we probably can. We just lose information about interrupts that
were masked, but maybe we just don't care about any of them anyway, so
it doesn't matter.
Best Regards,
Jaehoon Chung
+ if (rint)
+ mci_writel(host, RINTSTS, rint);
+
+ } else
+ dev_err(host-dev, %s: Reset bits didn't clear,
__func__);
Just display the error log? I didn't understand this.
If you displayed the error log, then
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
Hi, Sonny.
You can discard the my previous some comment.
As you mentioned, this reset sequence is recommended at Synopsys TRM.
Add the minor question.
On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
Hi, Sonny.
I have checked the Synopsys TRM..
On 05/09/2014 10:34 AM, Sonny Rao wrote:
On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com wrote:
On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D yuvaraj...@gmail.com
wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on the
mode.
+* Note that this doesn't handle the generic DMA (not IDMAC)
case.
+*/
not IDMAC is confused..
The documentation describes three different possible modes. There's a
mode that doesn't use IDMAC but still does DMA. But as far as I can
tell this driver doesn't support that way. We can just remove that
wording if it's confusing.
How did it know whether dma is generic DMA or not?
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req
to
+ clear during reset, __func__);
+
+ /* when using DMA next we reset the fifo again */
+ dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
you use the status or temp instead of rint. (you can reuse the variable.)
And can use status = ~mci_readl(host,MINTSTS);
Just clear the RINTSTS register? why do you add these?
This will look at what is not masked, and only clear those bits.
Well, i known if clear the RINTSTS register,
recommended to use 0xfff and set the value for interrupt.
Can be used 0xfff?
Best Regards,
Jaehoon Chung
+ if (rint)
+ mci_writel(host, RINTSTS, rint);
+
+ } else
+ dev_err(host-dev, %s: Reset bits didn't clear,
__func__);
Just display the error log? I didn't understand this.
If you displayed the error log, then it needs to return the error value.
+
+ #ifdef CONFIG_MMC_DW_IDMAC
+ /* It is also recommended that we reset and reprogram idmac */
+ dw_mci_idmac_reset(host);
+ #endif
+
+ /* After a CTRL reset we need to have CIU set clock registers */
+ mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
Well, why do you send the clock update command?
I didn't see that update the value related with clock.
Best Regards,
Jaehoon Chung
+
+ return true;
}
static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 738fa24..037e47a 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -129,6 +129,7 @@
#define SDMMC_CMD_INDX(n) ((n) 0x1F)
/* Status register defines */
#define SDMMC_GET_FCNT(x) (((x)17) 0x1FFF)
+#define
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On Fri, May 9, 2014 at 12:32 AM, Jaehoon Chung jh80.ch...@samsung.com wrote:
Hi, Sonny.
You can discard the my previous some comment.
As you mentioned, this reset sequence is recommended at Synopsys TRM.
Add the minor question.
On 05/09/2014 01:27 PM, Jaehoon Chung wrote:
Hi, Sonny.
I have checked the Synopsys TRM..
On 05/09/2014 10:34 AM, Sonny Rao wrote:
On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com
wrote:
On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D yuvaraj...@gmail.com
wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct
dw_mci *host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on
the mode.
+* Note that this doesn't handle the generic DMA (not IDMAC)
case.
+*/
not IDMAC is confused..
The documentation describes three different possible modes. There's a
mode that doesn't use IDMAC but still does DMA. But as far as I can
tell this driver doesn't support that way. We can just remove that
wording if it's confusing.
How did it know whether dma is generic DMA or not?
That's a good question. I wasn't sure whether the driver supported it
or not. It looks like it definitely supports IDMAC through the
CONFIG_MMC_DW_IDMAC flag, but I wasn't sure if it was supported the
generic dma. Maybe if CONFIG_MMC_DW_IDMAC isn't specified but
host-dma_ops is not NULL then we are using the generic dma mode.
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req
to
+ clear during reset, __func__);
+
+ /* when using DMA next we reset the fifo again */
+ dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to clear
any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
you use the status or temp instead of rint. (you can reuse the variable.)
And can use status = ~mci_readl(host,MINTSTS);
Just clear the RINTSTS register? why do you add these?
This will look at what is not masked, and only clear those bits.
Well, i known if clear the RINTSTS register,
recommended to use 0xfff and set the value for interrupt.
Can be used 0xfff?
Yeah we probably can. We just lose information about interrupts that
were masked, but maybe we just don't care about any of them anyway, so
it doesn't matter.
Best Regards,
Jaehoon Chung
+ if (rint)
+ mci_writel(host, RINTSTS, rint);
+
+ } else
+ dev_err(host-dev, %s: Reset bits didn't clear,
__func__);
Just display the error log? I didn't understand this.
If you displayed the error log, then it needs to return the error value.
+
+ #ifdef CONFIG_MMC_DW_IDMAC
+ /* It is also recommended that we reset and reprogram idmac */
+ dw_mci_idmac_reset(host);
+ #endif
+
+ /* After a CTRL reset we need to have CIU set clock registers */
+ mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
Well,
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on the
mode.
+* Note that this doesn't handle the generic DMA (not IDMAC) case.
+*/
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req to
+ clear during reset, __func__);
+
+ /* when using DMA next we reset the fifo again */
+ dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
+ if (rint)
+ mci_writel(host, RINTSTS, rint);
+
+ } else
+ dev_err(host-dev, %s: Reset bits didn't clear, __func__);
+
+ #ifdef CONFIG_MMC_DW_IDMAC
+ /* It is also recommended that we reset and reprogram idmac */
+ dw_mci_idmac_reset(host);
+ #endif
+
+ /* After a CTRL reset we need to have CIU set clock registers */
+ mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
+
+ return true;
}
static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 738fa24..037e47a 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -129,6 +129,7 @@
#define SDMMC_CMD_INDX(n) ((n) 0x1F)
/* Status register defines */
#define SDMMC_GET_FCNT(x) (((x)17) 0x1FFF)
+#define SDMMC_STATUS_DMA_REQ BIT(31)
/* FIFOTH register defines */
#define SDMMC_SET_FIFOTH(m, r, t) (((m) 0x7) 28 | \
((r) 0xFFF) 16 | \
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D yuvaraj...@gmail.com
wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on the
mode.
+* Note that this doesn't handle the generic DMA (not IDMAC) case.
+*/
not IDMAC is confused..
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req to
+ clear during reset, __func__);
+
+ /* when using DMA next we reset the fifo again */
+ dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
Just clear the RINTSTS register? why do you add these?
+ if (rint)
+ mci_writel(host, RINTSTS, rint);
+
+ } else
+ dev_err(host-dev, %s: Reset bits didn't clear, __func__);
Just display the error log? I didn't understand this.
If you displayed the error log, then it needs to return the error value.
+
+ #ifdef CONFIG_MMC_DW_IDMAC
+ /* It is also recommended that we reset and reprogram idmac */
+ dw_mci_idmac_reset(host);
+ #endif
+
+ /* After a CTRL reset we need to have CIU set clock registers */
+ mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
+
+ return true;
}
static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 738fa24..037e47a 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -129,6 +129,7 @@
#define SDMMC_CMD_INDX(n) ((n) 0x1F)
/* Status register defines */
#define SDMMC_GET_FCNT(x) (((x)17) 0x1FFF)
+#define SDMMC_STATUS_DMA_REQ BIT(31)
/* FIFOTH register defines */
#define SDMMC_SET_FIFOTH(m, r, t) (((m) 0x7) 28 | \
((r) 0xFFF) 16 | \
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com wrote:
On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D yuvaraj...@gmail.com
wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on the
mode.
+* Note that this doesn't handle the generic DMA (not IDMAC) case.
+*/
not IDMAC is confused..
The documentation describes three different possible modes. There's a
mode that doesn't use IDMAC but still does DMA. But as far as I can
tell this driver doesn't support that way. We can just remove that
wording if it's confusing.
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req to
+ clear during reset, __func__);
+
+ /* when using DMA next we reset the fifo again */
+ dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
Just clear the RINTSTS register? why do you add these?
This will look at what is not masked, and only clear those bits.
+ if (rint)
+ mci_writel(host, RINTSTS, rint);
+
+ } else
+ dev_err(host-dev, %s: Reset bits didn't clear, __func__);
Just display the error log? I didn't understand this.
If you displayed the error log, then it needs to return the error value.
+
+ #ifdef CONFIG_MMC_DW_IDMAC
+ /* It is also recommended that we reset and reprogram idmac */
+ dw_mci_idmac_reset(host);
+ #endif
+
+ /* After a CTRL reset we need to have CIU set clock registers */
+ mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
+
+ return true;
}
static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 738fa24..037e47a 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -129,6 +129,7 @@
#define SDMMC_CMD_INDX(n) ((n) 0x1F)
/* Status register defines */
#define SDMMC_GET_FCNT(x) (((x)17) 0x1FFF)
+#define SDMMC_STATUS_DMA_REQ BIT(31)
/* FIFOTH register defines */
#define SDMMC_SET_FIFOTH(m, r, t) (((m) 0x7) 28 | \
((r) 0xFFF) 16 | \
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: dw_mmc: change to use recommended reset procedure
Hi, Sonny.
I have checked the Synopsys TRM..
On 05/09/2014 10:34 AM, Sonny Rao wrote:
On Thu, May 8, 2014 at 6:15 PM, Jaehoon Chung jh80.ch...@samsung.com wrote:
On 05/08/2014 06:40 PM, Yuvaraj Kumar wrote:
Any comments on this patch?
On Wed, Mar 26, 2014 at 5:16 PM, Yuvaraj Kumar C D yuvaraj...@gmail.com
wrote:
From: Sonny Rao sonny...@chromium.org
This patch changes the fifo reset code to follow the reset procedure
outlined in the documentation of Synopsys Mobile storage host databook
7.2.13.
Without this patch, we could able to see eMMC was not detected after
multiple reboots due to driver hangs while eMMC tuning for HS200.
Signed-off-by: Sonny Rao sonny...@chromium.org
Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.org
---
drivers/mmc/host/dw_mmc.c | 48
-
drivers/mmc/host/dw_mmc.h |1 +
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32dd81d..1d77431 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci
*host)
host-sg = NULL;
}
- return dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ /*
+* The recommended method for resetting is to always reset the
+* controller and the fifo, but differs slightly depending on the
mode.
+* Note that this doesn't handle the generic DMA (not IDMAC)
case.
+*/
not IDMAC is confused..
The documentation describes three different possible modes. There's a
mode that doesn't use IDMAC but still does DMA. But as far as I can
tell this driver doesn't support that way. We can just remove that
wording if it's confusing.
+ if (dw_mci_ctrl_reset(host, SDMMC_CTRL_RESET |
SDMMC_CTRL_FIFO_RESET)) {
+ unsigned long timeout = jiffies + msecs_to_jiffies(500);
+ u32 status, rint;
+
+ /* if using dma we wait for dma_req to clear */
+ if (host-using_dma) {
+ do {
+ status = mci_readl(host, STATUS);
+ if (!(status SDMMC_STATUS_DMA_REQ))
+ break;
+ cpu_relax();
+ } while (time_before(jiffies, timeout));
+
+ if (status SDMMC_STATUS_DMA_REQ)
+ dev_err(host-dev,
+ %s: Timeout waiting for dma_req
to
+ clear during reset, __func__);
+
+ /* when using DMA next we reset the fifo again */
+ dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET);
+ }
+ /*
+* In all cases we clear the RAWINTS register to clear any
+* interrupts.
+*/
+ rint = mci_readl(host, RINTSTS);
+ rint = rint (~mci_readl(host, MINTSTS));
you use the status or temp instead of rint. (you can reuse the variable.)
And can use status = ~mci_readl(host,MINTSTS);
Just clear the RINTSTS register? why do you add these?
This will look at what is not masked, and only clear those bits.
Well, i known if clear the RINTSTS register,
recommended to use 0xfff and set the value for interrupt.
+ if (rint)
+ mci_writel(host, RINTSTS, rint);
+
+ } else
+ dev_err(host-dev, %s: Reset bits didn't clear,
__func__);
Just display the error log? I didn't understand this.
If you displayed the error log, then it needs to return the error value.
+
+ #ifdef CONFIG_MMC_DW_IDMAC
+ /* It is also recommended that we reset and reprogram idmac */
+ dw_mci_idmac_reset(host);
+ #endif
+
+ /* After a CTRL reset we need to have CIU set clock registers */
+ mci_send_cmd(host-cur_slot, SDMMC_CMD_UPD_CLK, 0);
Well, why do you send the clock update command?
I didn't see that update the value related with clock.
Best Regards,
Jaehoon Chung
+
+ return true;
}
static inline bool dw_mci_ctrl_all_reset(struct dw_mci *host)
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 738fa24..037e47a 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -129,6 +129,7 @@
#define SDMMC_CMD_INDX(n) ((n) 0x1F)
/* Status register defines */
#define SDMMC_GET_FCNT(x) (((x)17) 0x1FFF)
+#define SDMMC_STATUS_DMA_REQ BIT(31)
/* FIFOTH register defines */
#define SDMMC_SET_FIFOTH(m, r, t) (((m) 0x7) 28 | \
((r) 0xFFF) 16 | \
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to
