diff mbox series

mmc: renesas-sdhi: Stop transmission in case tuning block transfer fails

Message ID 20240220083853.34497-1-marek.vasut+renesas@mailbox.org
State Accepted
Commit 12859c2219a61bf4ce8b33de267d2b0e3e11ef37
Delegated to: Jaehoon Chung
Headers show
Series mmc: renesas-sdhi: Stop transmission in case tuning block transfer fails | expand

Commit Message

Marek Vasut Feb. 20, 2024, 8:38 a.m. UTC
The current code uses the state of tuning block received by SCC to
determine whether or not to send transmission stop command. This is
not correct. Use the state of tuning block transfer to determine
whether or not to send transmission stop command instead, because
the transmission stop command has to be sent in case the tuning
block transfer failed.

This requires two changes, separate variable to store and check the
state of tuning block received by SCC, and another separate variable
to store and check return value from transmission stop command.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Hai Pham <hai.pham.ud@renesas.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
Cc: Paul Barker <paul.barker.ct@bp.renesas.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/mmc/renesas-sdhi.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Paul Barker Feb. 20, 2024, 11:05 a.m. UTC | #1
On 20/02/2024 08:38, Marek Vasut wrote:
> The current code uses the state of tuning block received by SCC to
> determine whether or not to send transmission stop command. This is
> not correct. Use the state of tuning block transfer to determine
> whether or not to send transmission stop command instead, because
> the transmission stop command has to be sent in case the tuning
> block transfer failed.
> 
> This requires two changes, separate variable to store and check the
> state of tuning block received by SCC, and another separate variable
> to store and check return value from transmission stop command.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Hai Pham <hai.pham.ud@renesas.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Cc: Paul Barker <paul.barker.ct@bp.renesas.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/mmc/renesas-sdhi.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c
> index 316b75b35fe..c4d0733b621 100644
> --- a/drivers/mmc/renesas-sdhi.c
> +++ b/drivers/mmc/renesas-sdhi.c
> @@ -568,8 +568,8 @@ int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
>  	struct mmc *mmc = upriv->mmc;
>  	unsigned int tap_num;
>  	unsigned int taps = 0;
> -	int i, ret = 0;
> -	u32 caps;
> +	int i, ret = 0, sret;
> +	u32 caps, reg;
>  
>  	/* Only supported on Renesas RCar */
>  	if (!(priv->caps & TMIO_SD_CAP_RCAR_UHS))
> @@ -612,8 +612,8 @@ int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
>  		if (ret == 0)
>  			taps |= BIT(i);
>  
> -		ret = renesas_sdhi_compare_scc_data(priv);
> -		if (ret == 0)
> +		reg = renesas_sdhi_compare_scc_data(priv);
> +		if (reg == 0)
>  			priv->smpcmp |= BIT(i);
>  
>  		mdelay(1);
> @@ -624,9 +624,9 @@ int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
>  		 * eMMC.
>  		 */
>  		if (ret && (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200)) {
> -			ret = mmc_send_stop_transmission(mmc, false);
> -			if (ret < 0)
> -				dev_dbg(dev, "Tuning abort fail (%d)\n", ret);
> +			sret = mmc_send_stop_transmission(mmc, false);
> +			if (sret < 0)
> +				dev_dbg(dev, "Tuning abort fail (%d)\n", sret);
>  		}
>  	}
>  

Reviewed-by: Paul Barker <paul.barker.ct@bp.renesas.com>
Tested-by: Paul Barker <paul.barker.ct@bp.renesas.com>
  (tested on RZ/G2L with commit ad50a8151387 from
  https://source.denx.de/u-boot/custodians/u-boot-sh)
Jaehoon Chung April 3, 2024, 1:03 a.m. UTC | #2
On 2/20/24 17:38, Marek Vasut wrote:
> The current code uses the state of tuning block received by SCC to
> determine whether or not to send transmission stop command. This is
> not correct. Use the state of tuning block transfer to determine
> whether or not to send transmission stop command instead, because
> the transmission stop command has to be sent in case the tuning
> block transfer failed.
> 
> This requires two changes, separate variable to store and check the
> state of tuning block received by SCC, and another separate variable
> to store and check return value from transmission stop command.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> Reviewed-by: Paul Barker <paul.barker.ct@bp.renesas.com>
> Tested-by: Paul Barker <paul.barker.ct@bp.renesas.com>
> ---
> Cc: Hai Pham <hai.pham.ud@renesas.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Cc: Paul Barker <paul.barker.ct@bp.renesas.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

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

Sorry for late. Will apply to u-boot-mmc/master, Thanks!

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/renesas-sdhi.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c
> index 316b75b35fe..c4d0733b621 100644
> --- a/drivers/mmc/renesas-sdhi.c
> +++ b/drivers/mmc/renesas-sdhi.c
> @@ -568,8 +568,8 @@ int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
>  	struct mmc *mmc = upriv->mmc;
>  	unsigned int tap_num;
>  	unsigned int taps = 0;
> -	int i, ret = 0;
> -	u32 caps;
> +	int i, ret = 0, sret;
> +	u32 caps, reg;
>  
>  	/* Only supported on Renesas RCar */
>  	if (!(priv->caps & TMIO_SD_CAP_RCAR_UHS))
> @@ -612,8 +612,8 @@ int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
>  		if (ret == 0)
>  			taps |= BIT(i);
>  
> -		ret = renesas_sdhi_compare_scc_data(priv);
> -		if (ret == 0)
> +		reg = renesas_sdhi_compare_scc_data(priv);
> +		if (reg == 0)
>  			priv->smpcmp |= BIT(i);
>  
>  		mdelay(1);
> @@ -624,9 +624,9 @@ int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
>  		 * eMMC.
>  		 */
>  		if (ret && (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200)) {
> -			ret = mmc_send_stop_transmission(mmc, false);
> -			if (ret < 0)
> -				dev_dbg(dev, "Tuning abort fail (%d)\n", ret);
> +			sret = mmc_send_stop_transmission(mmc, false);
> +			if (sret < 0)
> +				dev_dbg(dev, "Tuning abort fail (%d)\n", sret);
>  		}
>  	}
>
diff mbox series

Patch

diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c
index 316b75b35fe..c4d0733b621 100644
--- a/drivers/mmc/renesas-sdhi.c
+++ b/drivers/mmc/renesas-sdhi.c
@@ -568,8 +568,8 @@  int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
 	struct mmc *mmc = upriv->mmc;
 	unsigned int tap_num;
 	unsigned int taps = 0;
-	int i, ret = 0;
-	u32 caps;
+	int i, ret = 0, sret;
+	u32 caps, reg;
 
 	/* Only supported on Renesas RCar */
 	if (!(priv->caps & TMIO_SD_CAP_RCAR_UHS))
@@ -612,8 +612,8 @@  int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
 		if (ret == 0)
 			taps |= BIT(i);
 
-		ret = renesas_sdhi_compare_scc_data(priv);
-		if (ret == 0)
+		reg = renesas_sdhi_compare_scc_data(priv);
+		if (reg == 0)
 			priv->smpcmp |= BIT(i);
 
 		mdelay(1);
@@ -624,9 +624,9 @@  int renesas_sdhi_execute_tuning(struct udevice *dev, uint opcode)
 		 * eMMC.
 		 */
 		if (ret && (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200)) {
-			ret = mmc_send_stop_transmission(mmc, false);
-			if (ret < 0)
-				dev_dbg(dev, "Tuning abort fail (%d)\n", ret);
+			sret = mmc_send_stop_transmission(mmc, false);
+			if (sret < 0)
+				dev_dbg(dev, "Tuning abort fail (%d)\n", sret);
 		}
 	}