| Message ID | 20220815073321.63382-3-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sd: Fix inconsistent sd3_bus_mode with failed voltage switch | expand |
> -----Original Message----- > From: Adrian Hunter <adrian.hunter@intel.com> > Sent: Monday, August 15, 2022 4:33 PM > To: Ulf Hansson <ulf.hansson@linaro.org> > Cc: linux-mmc <linux-mmc@vger.kernel.org>; Seunghui Lee > <sh043.lee@samsung.com>; DooHyun Hwang <dh0421.hwang@samsung.com> > Subject: [PATCH 2/2] mmc: sd: Fix inconsistent sd3_bus_mode with failed > voltage switch > > If re-initialization results is a different signal voltage, because the > voltage switch failed previously but not this time (or vice versa), then > sd3_bus_mode will be inconsistent with the card because the SD_SWITCH > command is done only upon first initialization. > > Fix by always reading SD_SWITCH information during re-initialization which > also means it does not need to be re-read later for the 1.8V fixup > workaround. > > Note, brief testing showed SD_SWITCH took about 1.8ms to 2ms which added > about 1% to 1.5% to the re-initialization time, so not particularly > significant. > > Reported-by: Seunghui Lee <sh043.lee@samsung.com> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/core/sd.c | 42 ++++++++++++++++-------------------------- > 1 file changed, 16 insertions(+), 26 deletions(-) > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index > bc84d7dfc8e1..06aa62ce0ed1 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -949,15 +949,16 @@ int mmc_sd_setup_card(struct mmc_host *host, struct > mmc_card *card, > > /* Erase init depends on CSD and SSR */ > mmc_init_erase(card); > - > - /* > - * Fetch switch information from card. > - */ > - err = mmc_read_switch(card); > - if (err) > - return err; > } > > + /* > + * Fetch switch information from card. Note, sd3_bus_mode can > change if > + * voltage switch outcome changes, so do this always. > + */ > + err = mmc_read_switch(card); > + if (err) > + return err; > + > /* > * For SPI, enable CRC as appropriate. > * This CRC enable is located AFTER the reading of the @@ -1480,26 > +1481,15 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, > if (!v18_fixup_failed && !mmc_host_is_spi(host) && > mmc_host_uhs(host) && > mmc_sd_card_using_v18(card) && > host->ios.signal_voltage != MMC_SIGNAL_VOLTAGE_180) { > - /* > - * Re-read switch information in case it has changed since > - * oldcard was initialized. > - */ > - if (oldcard) { > - err = mmc_read_switch(card); > - if (err) > - goto free_card; > - } > - if (mmc_sd_card_using_v18(card)) { > - if (mmc_host_set_uhs_voltage(host) || > - mmc_sd_init_uhs_card(card)) { > - v18_fixup_failed = true; > - mmc_power_cycle(host, ocr); > - if (!oldcard) > - mmc_remove_card(card); > - goto retry; > - } > - goto cont; > + if (mmc_host_set_uhs_voltage(host) || > + mmc_sd_init_uhs_card(card)) { > + v18_fixup_failed = true; > + mmc_power_cycle(host, ocr); > + if (!oldcard) > + mmc_remove_card(card); > + goto retry; > } > + goto cont; > } > > /* Initialization sequence for UHS-I cards */ > -- > 2.25.1 I've just tested this. It works well. Thank you for your work. Reviewed-by: Seunghui Lee <sh043.lee@samsung.com>
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index bc84d7dfc8e1..06aa62ce0ed1 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -949,15 +949,16 @@ int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card, /* Erase init depends on CSD and SSR */ mmc_init_erase(card); - - /* - * Fetch switch information from card. - */ - err = mmc_read_switch(card); - if (err) - return err; } + /* + * Fetch switch information from card. Note, sd3_bus_mode can change if + * voltage switch outcome changes, so do this always. + */ + err = mmc_read_switch(card); + if (err) + return err; + /* * For SPI, enable CRC as appropriate. * This CRC enable is located AFTER the reading of the @@ -1480,26 +1481,15 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, if (!v18_fixup_failed && !mmc_host_is_spi(host) && mmc_host_uhs(host) && mmc_sd_card_using_v18(card) && host->ios.signal_voltage != MMC_SIGNAL_VOLTAGE_180) { - /* - * Re-read switch information in case it has changed since - * oldcard was initialized. - */ - if (oldcard) { - err = mmc_read_switch(card); - if (err) - goto free_card; - } - if (mmc_sd_card_using_v18(card)) { - if (mmc_host_set_uhs_voltage(host) || - mmc_sd_init_uhs_card(card)) { - v18_fixup_failed = true; - mmc_power_cycle(host, ocr); - if (!oldcard) - mmc_remove_card(card); - goto retry; - } - goto cont; + if (mmc_host_set_uhs_voltage(host) || + mmc_sd_init_uhs_card(card)) { + v18_fixup_failed = true; + mmc_power_cycle(host, ocr); + if (!oldcard) + mmc_remove_card(card); + goto retry; } + goto cont; } /* Initialization sequence for UHS-I cards */
If re-initialization results is a different signal voltage, because the voltage switch failed previously but not this time (or vice versa), then sd3_bus_mode will be inconsistent with the card because the SD_SWITCH command is done only upon first initialization. Fix by always reading SD_SWITCH information during re-initialization which also means it does not need to be re-read later for the 1.8V fixup workaround. Note, brief testing showed SD_SWITCH took about 1.8ms to 2ms which added about 1% to 1.5% to the re-initialization time, so not particularly significant. Reported-by: Seunghui Lee <sh043.lee@samsung.com> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/core/sd.c | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-)