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);
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
