[PATCH] staging:rts_pstor:Fix SDIO issue

Dan Carpenter dan.carpenter at oracle.com
Sun Oct 9 13:57:45 UTC 2011


What I'm saying is, it's confusing to add another variable which is
only subtly different from retval.  Here is what it looks like after
we apply the patch.

	int reset_pass = 0;

	/* skip 45 lines */

	retval = sd_change_bank_voltage(chip, SD_IO_3V3);
	if (retval != STATUS_SUCCESS) {
		TRACE_RET(chip, STATUS_FAIL);
	}

	/* skip 30 lines */

	if (!reset_pass)
		TRACE_RET(chip, STATUS_FAIL);

The reviewer would have to read through 80 lines of code to find that
we don't care about the return value from sd_change_bank_voltage().
Don't do it that way.

Here is how I would write what it.  I can't actually test this since
I don't have the hardware...

diff --git a/drivers/staging/rts_pstor/sd.c b/drivers/staging/rts_pstor/sd.c
index fb62eaf..46bf0e1 100644
--- a/drivers/staging/rts_pstor/sd.c
+++ b/drivers/staging/rts_pstor/sd.c
@@ -3134,39 +3134,36 @@ int reset_sd_card(struct rtsx_chip *chip)
 
 	if (chip->sd_ctl & RESET_MMC_FIRST) {
 		retval = reset_mmc(chip);
-		if ((retval != STATUS_SUCCESS) && !sd_check_err_code(chip, SD_NO_CARD)) {
+		if (retval != STATUS_SUCCESS) {
+			if (sd_check_err_code(chip, SD_NO_CARD))
+				TRACE_RET(chip, STATUS_FAIL);
+
 			retval = reset_sd(chip);
 			if (retval != STATUS_SUCCESS) {
-				if (CHECK_PID(chip, 0x5209)) {
-					retval = sd_change_bank_voltage(chip, SD_IO_3V3);
-					if (retval != STATUS_SUCCESS) {
-						TRACE_RET(chip, STATUS_FAIL);
-					}
-				}
+				if (CHECK_PID(chip, 0x5209))
+					sd_change_bank_voltage(chip, SD_IO_3V3);
+				TRACE_RET(chip, STATUS_FAIL);
 			}
 		}
 	} else {
 		retval = reset_sd(chip);
 		if (retval != STATUS_SUCCESS) {
-			if (sd_check_err_code(chip, SD_NO_CARD)) {
+			if (sd_check_err_code(chip, SD_NO_CARD))
 				TRACE_RET(chip, STATUS_FAIL);
-			}
 
 			if (CHECK_PID(chip, 0x5209)) {
 				retval = sd_change_bank_voltage(chip, SD_IO_3V3);
-				if (retval != STATUS_SUCCESS) {
+				if (retval != STATUS_SUCCESS)
 					TRACE_RET(chip, STATUS_FAIL);
-				}
 			}
 
-			if (!chip->sd_io) {
-				retval = reset_mmc(chip);
-			}
-		}
-	}
+			if (chip->sd_io)
+				TRACE_RET(chip, STATUS_FAIL);
 
-	if (retval != STATUS_SUCCESS) {
-		TRACE_RET(chip, STATUS_FAIL);
+			retval = reset_mmc(chip);
+			if (retval != STATUS_SUCCESS)
+				TRACE_RET(chip, STATUS_FAIL);
+		}
 	}
 
 	retval = sd_set_clock_divider(chip, SD_CLK_DIVIDE_0);



More information about the devel mailing list