RE: [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle

["Seunghui Lee" <sh043.lee@samsung.com>]

> -----Original Message-----
> From: Seunghui Lee <sh043.lee@samsung.com>
> Sent: Thursday, July 21, 2022 2:59 PM
> To: ulf.hansson@linaro.org; linux-mmc@vger.kernel.org;
> adrian.hunter@intel.com
> Cc: Seunghui Lee <sh043.lee@samsung.com>; DooHyun Hwang
> <dh0421.hwang@samsung.com>
> Subject: [PATCH] mmc: sd: Remove the patch that fix signal voltage when
> there is no power cycle
> 
> At first, all error flow of mmc_set_uhs_voltage() has power cycle except
> R1_ERROR and no start_signal_voltage_switch() func pointer.
> 
> There is the performance regression issue of SDR104 SD card from the
> market VOC. Normally, once a SDR104 SD card fails to switch voltage, it
> works HS mode.
> And then it initializes SDR104 mode after system resume or error handling.
> 
> However, with below patch,
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/
> drivers/mmc/core/sd.c?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
> Once a SD card does, it initializes SDR25 mode forever after system resume
> or error handling(re-initialized).
> Host updates sd3_bus_mode by calling mmc_read_switch(), the value of
> sd3_bus_mode doesn't set for SDR104, SDR50 and DDR50 mode.
> 
> So, if host doesn't update sd3_bus_mode, the SD card works SDR104 mode
> after system resume or error-handling.
> 
> Here is an example.
> 
> AS-IS : test log
> // normal case : sd3_bus_mode = 0x1F, sd_bus_speed = SDR104, clock 208MHz
> [  111.907789] [1:    kworker/1:3:  772] [TEST] mmc_sd_init_card: 1119:
> caps=0x407f020f, sd3_bus_mode=0x1f, v18_fixup_failed false, signal_voltage
> =0x1.
> [  111.907824] [1:    kworker/1:3:  772] [TEST] mmc_sd_init_card: 1149: rocr
> 0xc1ff8000, S18A, uhs.
> [  111.908707] [1:    kworker/1:3:  772] [TEST] sd_update_bus_speed_mode:
> caps 0x407f020f, sd3_bus_mode = 0x1f, sd_bus_speed = 3, card->ocr =
> 0x40000.
> [  111.912484] [1:    kworker/1:3:  772] [TEST] sd_set_bus_speed_mode:
> sd_bus_speed=3, timing=6, uhs_max_dtr=208000000, card->ocr=0x40000.
> // resume : issue occurs : SDcard doesn't release busy for checking 10
> times
> [  112.096550] [5:    kworker/5:2:  207] [TEST] mmc_sd_init_card: 1040:
> card->ocr 0x40000.
> [  112.096560] [5:    kworker/5:2:  207] [TEST] mmc_sd_get_cid: ocr
> 0x40000(pocr 0x40000), retries 10.
> ...
> [  114.531129] [5:    kworker/5:2:  207] [TEST] mmc_power_cycle.
> [  114.579500] [5:    kworker/5:2:  207] [TEST] mmc_sd_get_cid: ocr
> 0x41040000(pocr 0x40000), retries 0.
> [  114.579506] [5:    kworker/5:2:  207] mmc0: Skipping voltage switch
> [  114.757575] [5:    kworker/5:2:  207] [TEST] mmc_sd_init_card: 1119:
> caps=0x407f020f, sd3_bus_mode=0x1f, v18_fixup_failed false, signal_voltage
> =0x0.
> [  114.757583] [5:    kworker/5:2:  207] [TEST] mmc_sd_init_card: 1128:
> switch with oldcard.
> [  114.759742] [5:    kworker/5:2:  207] [TEST] mmc_read_switch: sd_switch
> ret 0, sd3_bus_mode=3.
> // sd3_bus_mode = 0x3 supports HS, SDR25 and SDR12
> [  114.759750] [5:    kworker/5:2:  207] [TEST] mmc_sd_init_card: 1157:
> switch hs.
> // next resume : the SDcard initializes to SDR25(HS) mode(sd_bus_speed = 1)
> by sd3_bus_mode setting with clk 50MHz
> [  114.968346] [5:    kworker/5:2:  207] [TEST] mmc_sd_init_card: 1040:
> card->ocr 0x40000.
> [  114.968359] [5:    kworker/5:2:  207] [TEST] mmc_sd_get_cid: ocr
> 0x40000(pocr 0x40000), retries 10.
> [  115.167346] [5:    kworker/5:2:  207] [TEST] mmc_sd_init_card: 1119:
> caps=0x407f020f, sd3_bus_mode=0x3, v18_fixup_failed false, signal_voltage
> =0x1.
> [  115.167366] [5:    kworker/5:2:  207] [TEST] mmc_sd_init_card: 1149: rocr
> 0xc1ff8000, S18A, uhs.
> [  115.168041] [5:    kworker/5:2:  207] [TEST] mmc_sd_init_uhs_card: before
> update: caps 0x407f020f, sd3_bus_mode = 3, sd_bus_speed = 3, card->ocr =
> 0x40000.
> [  115.168051] [5:    kworker/5:2:  207] [TEST] sd_update_bus_speed_mode:
> caps 0x407f020f, sd3_bus_mode = 3, sd_bus_speed = 1, card->ocr = 0x40000.
> [  115.169176] [5:    kworker/5:2:  207] [TEST] sd_set_bus_speed_mode:
> sd_bus_speed=1, timing=4, uhs_max_dtr=50000000, card->ocr=0x40000.
> 
> TO-BE : TEST log with this commit
> // resume : issue occurs : SDcard doesn't release busy for checking 10
> times
> [ 1843.594805] [4:    kworker/4:5:21512] [TEST] mmc_sd_get_cid: ocr
> 0x41040000(pocr 0x40000), retries 0.
> [ 1843.594812] [4:    kworker/4:5:21512] mmc0: Skipping voltage switch
> [ 1843.772555] [4:    kworker/4:5:21512] [TEST] mmc_sd_init_card: 1122:
> caps=0x407f020f, sd3_bus_mode=0x1f, v18_fixup_failed false, signal_voltage
> =0x0.
> // no update sd3_bus_mode value
> [ 1843.772563] [4:    kworker/4:5:21512] [TEST] mmc_sd_init_card: 1164:
> switch hs.
> // next resume : the SDcard initializes to SDR104
> [ 1844.191295] [5:   kworker/5:93: 2282] [TEST] mmc_sd_init_card: 1122:
> caps=0x407f020f, sd3_bus_mode=0x1f, v18_fixup_failed false, signal_voltage
> =0x1.
> [ 1844.191315] [5:   kworker/5:93: 2282] [TEST] mmc_sd_init_card: 1154:
> rocr 0xc1ff8000, S18A, uhs.
> [ 1844.192175] [5:   kworker/5:93: 2282] [TEST] mmc_sd_init_uhs_card:
> before update: caps 0x407f020f, sd3_bus_mode = 0x1f, sd_bus_speed = 3,
> card->ocr = 0x40000.
> [ 1844.192187] [5:   kworker/5:93: 2282] [TEST] sd_update_bus_speed_mode:
> caps 0x407f020f, sd3_bus_mode = 0x1f, sd_bus_speed = 3, card->ocr =
> 0x40000.
> [ 1844.198697] [5:   kworker/5:93: 2282] [TEST] sd_set_bus_speed_mode:
> sd_bus_speed=3, timing=6, uhs_max_dtr=208000000, card->ocr=0x40000.
> 
> Signed-off-by: Seunghui Lee <sh043.lee@samsung.com>
> Tested-by: DooHyun Hwang <dh0421.hwang@samsung.com>
> ---
>  drivers/mmc/core/sd.c | 47 ++-----------------------------------------
>  1 file changed, 2 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> cee4c0b59f43..4e3d39956185 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1001,18 +1001,6 @@ unsigned mmc_sd_get_max_clock(struct mmc_card *card)
>  	return max_dtr;
>  }
> 
> -static bool mmc_sd_card_using_v18(struct mmc_card *card) -{
> -	/*
> -	 * According to the SD spec., the Bus Speed Mode (function group 1)
> bits
> -	 * 2 to 4 are zero if the card is initialized at 3.3V signal level.
> Thus
> -	 * they can be used to determine if the card has already switched
> to
> -	 * 1.8V signaling.
> -	 */
> -	return card->sw_caps.sd3_bus_mode &
> -	       (SD_MODE_UHS_SDR50 | SD_MODE_UHS_SDR104 | SD_MODE_UHS_DDR50);
> -}
> -
>  static int sd_write_ext_reg(struct mmc_card *card, u8 fno, u8 page, u16
> offset,
>  			    u8 reg_data)
>  {
> @@ -1400,10 +1388,9 @@ static int mmc_sd_init_card(struct mmc_host *host,
> u32 ocr,
>  	int err;
>  	u32 cid[4];
>  	u32 rocr = 0;
> -	bool v18_fixup_failed = false;
> 
>  	WARN_ON(!host->claimed);
> -retry:
> +
>  	err = mmc_sd_get_cid(host, ocr, cid, &rocr);
>  	if (err)
>  		return err;
> @@ -1472,36 +1459,6 @@ static int mmc_sd_init_card(struct mmc_host *host,
> u32 ocr,
>  	if (err)
>  		goto free_card;
> 
> -	/*
> -	 * If the card has not been power cycled, it may still be using
> 1.8V
> -	 * signaling. Detect that situation and try to initialize a UHS-I
> (1.8V)
> -	 * transfer mode.
> -	 */
> -	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 done;
> -		}
> -	}
> -
>  	/* Initialization sequence for UHS-I cards */
>  	if (rocr & SD_ROCR_S18A && mmc_host_uhs(host)) {
>  		err = mmc_sd_init_uhs_card(card);
> @@ -1566,7 +1523,7 @@ static int mmc_sd_init_card(struct mmc_host *host,
> u32 ocr,
>  		err = -EINVAL;
>  		goto free_card;
>  	}
> -done:
> +
>  	host->card = card;
>  	return 0;
> 
> --
> 2.29.0

Dear All,

Please review this commit.

Once the SDR104 SD card fails to switch voltage,
there is no chance to work SDR104 bus speed again
due to update sd3_bus_mode.

To fix this regression issue, do not update sd3_bus_mode.
And then it has the chance to work SDR104 again.

AS-IS:
voltage_switch fail -> mmc_read_switch() -> HS mode
next system resume
voltage switch success -> SDR25 mode

TO-BE:
Voltage switch fail -> HS mode
Next system resume
Voltage switch success -> SDR104 mode

And plus, mmc_set_uhs_voltage() has power_cycle now.
It means that if voltage switch fails,
the card initializes 3.3V signal level.

Regards,
Seunghui Lee.