mmc: dw_mmc: change to use recommended reset procedure
PATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
From: Sonny Rao <[email protected]>
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.
--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)
drivers/mmc/host/dw_mmc.c +47 -1
@@ -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)
drivers/mmc/host/dw_mmc.h +1 -0
@@ -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 | \
Signature · 1 line
1.7.10.4
PATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
Any comments on this patch?
Quoted 89 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +47
-1@@ -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)drivers/mmc/host/dw_mmc.h +1
-1@@ -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
Jaehoon Chung
maintainerPATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
Quoted 26 lines · Yuvaraj Kumar
Any comments on this patch?
Quoted 23 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +0
-0@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host) host->sg =NULL ; }
Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2227
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-1-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..
Quoted 19 lines
Quoted 19 lines
drivers/mmc/host/dw_mmc.c +19
-0+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 */
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2254
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+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?
Quoted 5 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2259
Quoted 5 lines
drivers/mmc/host/dw_mmc.c +5
-0+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.
Quoted 29 lines
Quoted 28 lines
drivers/mmc/host/dw_mmc.c +10
-0+ +#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)drivers/mmc/host/dw_mmc.h +1
-1@@ -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
PATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
Quoted 27 lines · Jaehoon Chung
Quoted 26 lines · Yuvaraj Kumar
Any comments on this patch?
Quoted 23 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +0
-0@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host) host->sg =NULL ; }
Quoted 9 lines
Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2227
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-1-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.
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
drivers/mmc/host/dw_mmc.c +21
-0+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 ); + }
Quoted 9 lines
Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2254
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +6
-0+/* +* 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.
Quoted 39 lines
Quoted 5 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2259
Quoted 5 lines
drivers/mmc/host/dw_mmc.c +5
-0+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.Quoted 29 lines
Quoted 28 lines
drivers/mmc/host/dw_mmc.c +10
-0+ +#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)drivers/mmc/host/dw_mmc.h +1
-1@@ -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
Jaehoon Chung
maintainerPATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
Hi, Sonny.
I have checked the Synopsys TRM..
Quoted 62 lines · Sonny Rao
Quoted 27 lines · Jaehoon Chung
Quoted 26 lines · Yuvaraj Kumar
Any comments on this patch?
Quoted 23 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +0
-0@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host) host->sg =NULL ; }Quoted 9 lines
Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2227
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-1-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.
Quoted 19 lines
Quoted 19 lines
Quoted 19 lines
drivers/mmc/host/dw_mmc.c +19
-0+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 */
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2254
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+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);"
Quoted 5 lines
Quoted 3 lines
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 "0xfffffff" and set the value for interrupt.
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2259
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-0+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.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2267
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+ +#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
Quoted 27 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
drivers/mmc/host/dw_mmc.c +2
-0+ +return true; }static inline bool dw_mci_ctrl_all_reset (struct dw_mci *host)drivers/mmc/host/dw_mmc.h +0
-0@@ -129,6 +129,7 @@#define SDMMC_CMD_INDX (n) ((n) & 0x1F)/* Status register defines */ #define SDMMC_GET_FCNT (x) (((x)>>17) & 0x1FFF)Quoted 9 lines
Quoted 8 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.h +1
-1+#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 majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Jaehoon Chung
maintainerPATCH 1/1 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.
Quoted 47 lines · Jaehoon Chung
Hi, Sonny.
I have checked the Synopsys TRM..
Quoted 42 lines · Sonny Rao
Quoted 27 lines · Jaehoon Chung
Quoted 26 lines · Yuvaraj Kumar
Any comments on this patch?
Quoted 23 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +0
-0@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host) host->sg =NULL ; }Quoted 9 lines
Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2227
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-1-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?
Quoted 32 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
drivers/mmc/host/dw_mmc.c +19
-0+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 */ Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2254
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+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);"
Quoted 1 line
Quoted 1 line
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
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 "
0xfffffff" and set the value for interrupt.
Can be used "0xfffffff"?
Best Regards,
Jaehoon Chung
Quoted 59 lines
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2259
Quoted 7 lines
drivers/mmc/host/dw_mmc.c +5
-0+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.Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2267
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+ +#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 ChungQuoted 22 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
drivers/mmc/host/dw_mmc.c +2
-0+ +return true; }static inline bool dw_mci_ctrl_all_reset (struct dw_mci *host)drivers/mmc/host/dw_mmc.h +0
-0@@ -129,6 +129,7 @@#define SDMMC_CMD_INDX (n) ((n) & 0x1F)/* Status register defines */ #define SDMMC_GET_FCNT (x) (((x)>>17) & 0x1FFF)Quoted 9 lines
Quoted 8 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.h +1
-1+#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
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
Quoted 51 lines · Jaehoon Chung
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.
Quoted 43 lines · Jaehoon Chung
Hi, Sonny.
I have checked the Synopsys TRM..
Quoted 38 lines · Sonny Rao
Quoted 36 lines · Jaehoon Chung
Quoted 26 lines · Yuvaraj Kumar
Any comments on this patch?
Quoted 23 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +0
-0@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host) host->sg =NULL ; }Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2227
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-1-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..
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
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.
Quoted 32 lines
Quoted 32 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
drivers/mmc/host/dw_mmc.c +19
-0+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 */ Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2254
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+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);"
Quoted 1 line
Quoted 1 line
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
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 "
0xfffffff" and set the value for interrupt.Can be used "
0xfffffff"?
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.
Quoted 62 lines
Best Regards,
Jaehoon ChungQuoted 59 lines
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2259
Quoted 7 lines
drivers/mmc/host/dw_mmc.c +5
-0+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.Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2267
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+ +#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 ChungQuoted 22 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
drivers/mmc/host/dw_mmc.c +2
-0+ +return true; }static inline bool dw_mci_ctrl_all_reset (struct dw_mci *host)drivers/mmc/host/dw_mmc.h +0
-0@@ -129,6 +129,7 @@#define SDMMC_CMD_INDX (n) ((n) & 0x1F)/* Status register defines */ #define SDMMC_GET_FCNT (x) (((x)>>17) & 0x1FFF)Quoted 9 lines
Quoted 8 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.h +1
-1+#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
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 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
Quoted 178 lines · Sonny Rao
Quoted 51 lines · Jaehoon Chung
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.
Quoted 43 lines · Jaehoon Chung
Hi, Sonny.
I have checked the Synopsys TRM..
Quoted 38 lines · Sonny Rao
Quoted 36 lines · Jaehoon Chung
Quoted 26 lines · Yuvaraj Kumar
Any comments on this patch?
Quoted 23 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +0
-0@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host) host->sg =NULL ; }Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2227
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-1-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..
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
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_IDMACflag, but I wasn't sure if it was supported the generic dma. Maybe ifCONFIG_MMC_DW_IDMACisn't specified buthost->dma_opsis not NULL then we are using the generic dma mode.Quoted 32 lines
Quoted 32 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
drivers/mmc/host/dw_mmc.c +19
-0+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 */ Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2254
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+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);"
Quoted 1 line
Quoted 1 line
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
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 "
0xfffffff" and set the value for interrupt.Can be used "
0xfffffff"?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.
Quoted 62 lines
Best Regards,
Jaehoon ChungQuoted 59 lines
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2259
Quoted 7 lines
drivers/mmc/host/dw_mmc.c +5
-0+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.Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2267
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+ +#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 ChungQuoted 22 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
drivers/mmc/host/dw_mmc.c +2
-0+ +return true; }static inline bool dw_mci_ctrl_all_reset (struct dw_mci *host)drivers/mmc/host/dw_mmc.h +0
-0@@ -129,6 +129,7 @@#define SDMMC_CMD_INDX (n) ((n) & 0x1F)/* Status register defines */ #define SDMMC_GET_FCNT (x) (((x)>>17) & 0x1FFF)Quoted 9 lines
Quoted 8 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.h +1
-1+#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
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
Quoted 5 lines · Seungwon Jeon
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.
Quoted 184 lines
Thanks,
Seungwon JeonQuoted 178 lines · Sonny Rao
Quoted 51 lines · Jaehoon Chung
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.
Quoted 43 lines · Jaehoon Chung
Hi, Sonny.
I have checked the Synopsys TRM..
Quoted 38 lines · Sonny Rao
Quoted 36 lines · Jaehoon Chung
Quoted 26 lines · Yuvaraj Kumar
Any comments on this patch?
Quoted 23 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +0
-0@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host) host->sg =NULL ; }Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2227
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-1-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..
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
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_IDMACflag, but I wasn't sure if it was supported the generic dma. Maybe ifCONFIG_MMC_DW_IDMACisn't specified buthost->dma_opsis not NULL then we are using the generic dma mode.Quoted 32 lines
Quoted 32 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
drivers/mmc/host/dw_mmc.c +19
-0+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 */ Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2254
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+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);"
Quoted 1 line
Quoted 1 line
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
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 "
0xfffffff" and set the value for interrupt.Can be used "
0xfffffff"?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.
Quoted 62 lines
Best Regards,
Jaehoon ChungQuoted 59 lines
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2259
Quoted 7 lines
drivers/mmc/host/dw_mmc.c +5
-0+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.Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2267
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+ +#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 ChungQuoted 22 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
drivers/mmc/host/dw_mmc.c +2
-0+ +return true; }static inline bool dw_mci_ctrl_all_reset (struct dw_mci *host)drivers/mmc/host/dw_mmc.h +0
-0@@ -129,6 +129,7 @@#define SDMMC_CMD_INDX (n) ((n) & 0x1F)/* Status register defines */ #define SDMMC_GET_FCNT (x) (((x)>>17) & 0x1FFF)Quoted 9 lines
Quoted 8 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.h +1
-1+#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
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
In reply to Sonny RaoQuoted 57 lines · Sonny Rao
Quoted 56 lines · Jaehoon Chung
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.
Quoted 47 lines · Jaehoon Chung
Hi, Sonny.
I have checked the Synopsys TRM..
Quoted 42 lines · Sonny Rao
Quoted 27 lines · Jaehoon Chung
Quoted 26 lines · Yuvaraj Kumar
Any comments on this patch?
Quoted 23 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +0
-0@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host) host->sg =NULL ; }Quoted 9 lines
Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2227
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-1-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.
Quoted 9 lines
Quoted 2 lines
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_IDMACflag, but I wasn't sure if it was supported the generic dma. Maybe ifCONFIG_MMC_DW_IDMACisn't specified buthost->dma_opsis 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.
Quoted 108 lines
Quoted 32 lines
Quoted 32 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
drivers/mmc/host/dw_mmc.c +19
-0+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 */ Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2254
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+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);"
Quoted 1 line
Quoted 1 line
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
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 "
0xfffffff" and set the value for interrupt.Can be used "
0xfffffff"?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.
Quoted 62 lines
Best Regards,
Jaehoon ChungQuoted 59 lines
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2259
Quoted 7 lines
drivers/mmc/host/dw_mmc.c +5
-0+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.Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2267
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+ +#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 ChungQuoted 22 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
drivers/mmc/host/dw_mmc.c +2
-0+ +return true; }static inline bool dw_mci_ctrl_all_reset (struct dw_mci *host)drivers/mmc/host/dw_mmc.h +0
-0@@ -129,6 +129,7 @@#define SDMMC_CMD_INDX (n) ((n) & 0x1F)/* Status register defines */ #define SDMMC_GET_FCNT (x) (((x)>>17) & 0x1FFF)Quoted 9 lines
Quoted 8 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.h +1
-1+#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
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
Quoted 68 lines · Sonny Rao
Quoted 57 lines · Sonny Rao
Quoted 56 lines · Jaehoon Chung
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.
Quoted 47 lines · Jaehoon Chung
Hi, Sonny.
I have checked the Synopsys TRM..
Quoted 42 lines · Sonny Rao
Quoted 27 lines · Jaehoon Chung
Quoted 26 lines · Yuvaraj Kumar
Any comments on this patch?
Quoted 23 lines · Yuvaraj Kumar C D
From: Sonny Rao <[email protected]>
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 <[email protected]>Signed-off-by Yuvaraj Kumar C D <[email protected]>--- drivers/mmc/host/dw_mmc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++- drivers/mmc/host/dw_mmc.h | 1 + 2 files changed, 48 insertions(+), 1 deletion(-)drivers/mmc/host/dw_mmc.c +0
-0@@ -2220,7 +2220,53 @@ static inline bool dw_mci_fifo_reset(struct dw_mci *host) host->sg =NULL ; }Quoted 9 lines
Quoted 6 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2227
Quoted 6 lines
drivers/mmc/host/dw_mmc.c +5
-1-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.
Quoted 9 lines
Quoted 2 lines
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_IDMACflag, but I wasn't sure if it was supported the generic dma. Maybe ifCONFIG_MMC_DW_IDMACisn't specified buthost->dma_opsis not NULL then we are using the generic dma mode.
Quoted 5 lines
Doug mentioned that James Hogan might have an answer. James, are there Imgtec SoCs which use
dw_mmcand 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.
Quoted 108 lines
Quoted 108 lines
Quoted 32 lines
Quoted 32 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
drivers/mmc/host/dw_mmc.c +19
-0+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 */ Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2254
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+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);"
Quoted 1 line
Quoted 1 line
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
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 "
0xfffffff" and set the value for interrupt.Can be used "
0xfffffff"?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.
Quoted 62 lines
Best Regards,
Jaehoon ChungQuoted 59 lines
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2259
Quoted 7 lines
drivers/mmc/host/dw_mmc.c +5
-0+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.Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/mmc/host/dw_mmc.c static inline bool dw_mci_fifo_reset(struct dw_mci *host) · line 2267
Quoted 8 lines
drivers/mmc/host/dw_mmc.c +8
-0+ +#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 ChungQuoted 22 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
drivers/mmc/host/dw_mmc.c +2
-0+ +return true; }static inline bool dw_mci_ctrl_all_reset (struct dw_mci *host)drivers/mmc/host/dw_mmc.h +0
-0@@ -129,6 +129,7 @@#define SDMMC_CMD_INDX (n) ((n) & 0x1F)/* Status register defines */ #define SDMMC_GET_FCNT (x) (((x)>>17) & 0x1FFF)Quoted 9 lines
Quoted 8 lines
Quoted 7 lines
drivers/mmc/host/dw_mmc.h +1
-1+#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
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 mmc: dw_mmc: change to use recommended reset procedure
In reply to Sonny RaoHi,
Quoted 4 lines · Sonny Rao
Doug mentioned that James Hogan might have an answer. James, are there Imgtec SoCs which use
dw_mmcand 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