diff mbox series

[v2,RESEND] mmc: dw_mmc: reset controller after data error

Message ID 20230619103347.278004-1-eugen.hristev@collabora.com
State Deferred
Delegated to: Tom Rini
Headers show
Series [v2,RESEND] mmc: dw_mmc: reset controller after data error | expand

Commit Message

Eugen Hristev June 19, 2023, 10:33 a.m. UTC 
From: Ziyuan Xu <xzy.xu@rock-chips.com>

Per dw_mmc databook, it's recommended to reset the host controller if
some data-related error occurred.
Implement a reset mechanism.

Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
Co-developed-by: Jason Zhu <jason.zhu@rock-chips.com>
Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com>
[eugen.hristev@collabora.com: modified a bit the variables initialization]
Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
---
 drivers/mmc/dw_mmc.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Comments

Eugen Hristev Sept. 21, 2023, 2:51 p.m. UTC | #1 
On 6/19/23 13:33, Eugen Hristev wrote:
> From: Ziyuan Xu <xzy.xu@rock-chips.com>
> 
> Per dw_mmc databook, it's recommended to reset the host controller if
> some data-related error occurred.
> Implement a reset mechanism.
> 
> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
> Co-developed-by: Jason Zhu <jason.zhu@rock-chips.com>
> Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com>
> [eugen.hristev@collabora.com: modified a bit the variables initialization]
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> ---

Hi,

Gentle ping.
This patch require any more changes?

Thanks,
Eugen

>   drivers/mmc/dw_mmc.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 5085a3b491da..d6cad998b0cd 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -138,7 +138,7 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
>   {
>   	struct mmc *mmc = host->mmc;
>   	int ret = 0;
> -	u32 timeout, mask, size, i, len = 0;
> +	u32 timeout, reset_timeout = 100, status, ctrl, mask, size, i, len = 0;
>   	u32 *buf = NULL;
>   	ulong start = get_timer(0);
>   	u32 fifo_depth = (((host->fifoth_val & RX_WMARK_MASK) >>
> @@ -159,6 +159,24 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
>   		/* Error during data transfer. */
>   		if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
>   			debug("%s: DATA ERROR!\n", __func__);
> +
> +			dwmci_wait_reset(host, DWMCI_RESET_ALL);
> +			dwmci_writel(host, DWMCI_CMD, DWMCI_CMD_PRV_DAT_WAIT |
> +				     DWMCI_CMD_UPD_CLK | DWMCI_CMD_START);
> +
> +			do {
> +				status = dwmci_readl(host, DWMCI_CMD);
> +				if (!reset_timeout--)
> +					break;
> +				udelay(100);
> +			} while (status & DWMCI_CMD_START);
> +
> +			if (!host->fifo_mode) {
> +				ctrl = dwmci_readl(host, DWMCI_BMOD);
> +				ctrl |= DWMCI_BMOD_IDMAC_RESET;
> +				dwmci_writel(host, DWMCI_BMOD, ctrl);
> +			}
> +
>   			ret = -EINVAL;
>   			break;
>   		}
Tom Fitzhenry Oct. 7, 2023, 2:37 p.m. UTC | #2 
I am able to reproduce this on RK3588 QuartzPro64.

I thought "[PATCH v2 RESEND] mmc: dw_mmc: reset controller after data
error"[0] might fix this, but after applying that, I am still able to
reproduce the issue.

0. https://lore.kernel.org/u-boot/20230619103347.278004-1-eugen.hristev@collabora.com/
Kever Yang Oct. 8, 2023, 1:18 a.m. UTC | #3 
Hi Tom,

     Could you have a try with rockchip vendor U-Boot, maybe still some 
other fixes are missing in mainline U-Boot mmc driver.


Thanks,

- Kever

On 2023/10/7 22:37, Tom Fitzhenry wrote:
> I am able to reproduce this on RK3588 QuartzPro64.
>
> I thought "[PATCH v2 RESEND] mmc: dw_mmc: reset controller after data
> error"[0] might fix this, but after applying that, I am still able to
> reproduce the issue.
>
> 0. https://lore.kernel.org/u-boot/20230619103347.278004-1-eugen.hristev@collabora.com/
Kever Yang Oct. 8, 2023, 1:19 a.m. UTC | #4 
On 2023/6/19 18:33, Eugen Hristev wrote:
> From: Ziyuan Xu <xzy.xu@rock-chips.com>
>
> Per dw_mmc databook, it's recommended to reset the host controller if
> some data-related error occurred.
> Implement a reset mechanism.
>
> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
> Co-developed-by: Jason Zhu <jason.zhu@rock-chips.com>
> Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com>
> [eugen.hristev@collabora.com: modified a bit the variables initialization]
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>


Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever

> ---
>   drivers/mmc/dw_mmc.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 5085a3b491da..d6cad998b0cd 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -138,7 +138,7 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
>   {
>   	struct mmc *mmc = host->mmc;
>   	int ret = 0;
> -	u32 timeout, mask, size, i, len = 0;
> +	u32 timeout, reset_timeout = 100, status, ctrl, mask, size, i, len = 0;
>   	u32 *buf = NULL;
>   	ulong start = get_timer(0);
>   	u32 fifo_depth = (((host->fifoth_val & RX_WMARK_MASK) >>
> @@ -159,6 +159,24 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
>   		/* Error during data transfer. */
>   		if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
>   			debug("%s: DATA ERROR!\n", __func__);
> +
> +			dwmci_wait_reset(host, DWMCI_RESET_ALL);
> +			dwmci_writel(host, DWMCI_CMD, DWMCI_CMD_PRV_DAT_WAIT |
> +				     DWMCI_CMD_UPD_CLK | DWMCI_CMD_START);
> +
> +			do {
> +				status = dwmci_readl(host, DWMCI_CMD);
> +				if (!reset_timeout--)
> +					break;
> +				udelay(100);
> +			} while (status & DWMCI_CMD_START);
> +
> +			if (!host->fifo_mode) {
> +				ctrl = dwmci_readl(host, DWMCI_BMOD);
> +				ctrl |= DWMCI_BMOD_IDMAC_RESET;
> +				dwmci_writel(host, DWMCI_BMOD, ctrl);
> +			}
> +
>   			ret = -EINVAL;
>   			break;
>   		}
Tom Fitzhenry Nov. 23, 2023, 5:26 a.m. UTC | #5 
Kever Yang <kever.yang@rock-chips.com> writes:
>     Could you have a try with rockchip vendor U-Boot, maybe still some
> other fixes are missing in mainline U-Boot mmc driver.

Sorry, I just realised I sent this message as a reply to the patch,
rather than against the bug report[0].

I have tested the vendor u-boot, and found it to be working. I have
replied against the bug thread[1].

0. https://lore.kernel.org/u-boot/20230619103347.278004-1-eugen.hristev@collabora.com/
1. https://lore.kernel.org/u-boot/87pm01oxd2.fsf@tom-fitzhenry.me.uk/
Jaehoon Chung Jan. 18, 2024, 1:21 a.m. UTC | #6 
On 6/19/23 19:33, Eugen Hristev wrote:
> From: Ziyuan Xu <xzy.xu@rock-chips.com>
> 
> Per dw_mmc databook, it's recommended to reset the host controller if
> some data-related error occurred.
> Implement a reset mechanism.
> 
> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
> Co-developed-by: Jason Zhu <jason.zhu@rock-chips.com>
> Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com>
> [eugen.hristev@collabora.com: modified a bit the variables initialization]
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/dw_mmc.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 5085a3b491da..d6cad998b0cd 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -138,7 +138,7 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
>  {
>  	struct mmc *mmc = host->mmc;
>  	int ret = 0;
> -	u32 timeout, mask, size, i, len = 0;
> +	u32 timeout, reset_timeout = 100, status, ctrl, mask, size, i, len = 0;
>  	u32 *buf = NULL;
>  	ulong start = get_timer(0);
>  	u32 fifo_depth = (((host->fifoth_val & RX_WMARK_MASK) >>
> @@ -159,6 +159,24 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
>  		/* Error during data transfer. */
>  		if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
>  			debug("%s: DATA ERROR!\n", __func__);
> +
> +			dwmci_wait_reset(host, DWMCI_RESET_ALL);
> +			dwmci_writel(host, DWMCI_CMD, DWMCI_CMD_PRV_DAT_WAIT |
> +				     DWMCI_CMD_UPD_CLK | DWMCI_CMD_START);
> +
> +			do {
> +				status = dwmci_readl(host, DWMCI_CMD);
> +				if (!reset_timeout--)
> +					break;
> +				udelay(100);
> +			} while (status & DWMCI_CMD_START);
> +
> +			if (!host->fifo_mode) {
> +				ctrl = dwmci_readl(host, DWMCI_BMOD);
> +				ctrl |= DWMCI_BMOD_IDMAC_RESET;
> +				dwmci_writel(host, DWMCI_BMOD, ctrl);
> +			}
> +
>  			ret = -EINVAL;
>  			break;
>  		}
diff mbox series

Patch

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 5085a3b491da..d6cad998b0cd 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -138,7 +138,7 @@  static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
 {
 	struct mmc *mmc = host->mmc;
 	int ret = 0;
-	u32 timeout, mask, size, i, len = 0;
+	u32 timeout, reset_timeout = 100, status, ctrl, mask, size, i, len = 0;
 	u32 *buf = NULL;
 	ulong start = get_timer(0);
 	u32 fifo_depth = (((host->fifoth_val & RX_WMARK_MASK) >>
@@ -159,6 +159,24 @@  static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
 		/* Error during data transfer. */
 		if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
 			debug("%s: DATA ERROR!\n", __func__);
+
+			dwmci_wait_reset(host, DWMCI_RESET_ALL);
+			dwmci_writel(host, DWMCI_CMD, DWMCI_CMD_PRV_DAT_WAIT |
+				     DWMCI_CMD_UPD_CLK | DWMCI_CMD_START);
+
+			do {
+				status = dwmci_readl(host, DWMCI_CMD);
+				if (!reset_timeout--)
+					break;
+				udelay(100);
+			} while (status & DWMCI_CMD_START);
+
+			if (!host->fifo_mode) {
+				ctrl = dwmci_readl(host, DWMCI_BMOD);
+				ctrl |= DWMCI_BMOD_IDMAC_RESET;
+				dwmci_writel(host, DWMCI_BMOD, ctrl);
+			}
+
 			ret = -EINVAL;
 			break;
 		}