* [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle
[not found] <CGME20220721052617epcas1p48967f8acf113372b2a9fc88cca40b2dc@epcas1p4.samsung.com>
@ 2022-07-21 5:59 ` Seunghui Lee
2022-07-26 2:56 ` Seunghui Lee
0 siblings, 1 reply; 7+ messages in thread
From: Seunghui Lee @ 2022-07-21 5:59 UTC (permalink / raw)
To: ulf.hansson, linux-mmc, adrian.hunter; +Cc: Seunghui Lee, DooHyun Hwang
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle
2022-07-21 5:59 ` [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle Seunghui Lee
@ 2022-07-26 2:56 ` Seunghui Lee
2022-07-26 10:56 ` Adrian Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Seunghui Lee @ 2022-07-26 2:56 UTC (permalink / raw)
To: ulf.hansson, linux-mmc, adrian.hunter; +Cc: 'DooHyun Hwang'
> -----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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle
2022-07-26 2:56 ` Seunghui Lee
@ 2022-07-26 10:56 ` Adrian Hunter
2022-08-08 8:24 ` Seunghui Lee
0 siblings, 1 reply; 7+ messages in thread
From: Adrian Hunter @ 2022-07-26 10:56 UTC (permalink / raw)
To: Seunghui Lee, linux-mmc, adrian.hunter; +Cc: 'DooHyun Hwang'
On 26/07/22 05:56, Seunghui Lee wrote:
>> -----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.
I have started to look at it, but my time is limited at the
moment.
Note the original patch is 5 years old and fixes a real
problem, so we don't want to just throw it away.
>
> 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.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle
2022-07-26 10:56 ` Adrian Hunter
@ 2022-08-08 8:24 ` Seunghui Lee
2022-08-09 5:36 ` Adrian Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Seunghui Lee @ 2022-08-08 8:24 UTC (permalink / raw)
To: 'Adrian Hunter', linux-mmc; +Cc: 'DooHyun Hwang'
> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: Tuesday, July 26, 2022 7:57 PM
> To: Seunghui Lee <sh043.lee@samsung.com>; linux-mmc@vger.kernel.org;
> adrian.hunter@intel.com
> Cc: 'DooHyun Hwang' <dh0421.hwang@samsung.com>
> Subject: Re: [PATCH] mmc: sd: Remove the patch that fix signal voltage
> when there is no power cycle
>
> On 26/07/22 05:56, Seunghui Lee wrote:
> >> -----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/co
> >> mmit/
> >> 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.
>
> I have started to look at it, but my time is limited at the moment.
>
> Note the original patch is 5 years old and fixes a real problem, so we
> don't want to just throw it away.
>
Dear Mr. hunter,
Could you check this with below patch?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/core/core.c?id=147186f531ae49c18b7a9091a2c40e83b3d95649
In the mmc_set_uhs_voltaga func(),
once it occurs error, it has power_cycle except R1_ERROR with CMD11.
So, When mmc_set_uhs_voltage() return error,
host and card can't leave 1.8V voltage.
Regards,
> >
> > 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.
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle
2022-08-08 8:24 ` Seunghui Lee
@ 2022-08-09 5:36 ` Adrian Hunter
2022-08-10 4:24 ` Seunghui Lee
0 siblings, 1 reply; 7+ messages in thread
From: Adrian Hunter @ 2022-08-09 5:36 UTC (permalink / raw)
To: Seunghui Lee, linux-mmc; +Cc: 'DooHyun Hwang'
On 8/08/22 11:24, Seunghui Lee wrote:
>> -----Original Message-----
>> From: Adrian Hunter <adrian.hunter@intel.com>
>> Sent: Tuesday, July 26, 2022 7:57 PM
>> To: Seunghui Lee <sh043.lee@samsung.com>; linux-mmc@vger.kernel.org;
>> adrian.hunter@intel.com
>> Cc: 'DooHyun Hwang' <dh0421.hwang@samsung.com>
>> Subject: Re: [PATCH] mmc: sd: Remove the patch that fix signal voltage
>> when there is no power cycle
>>
>> On 26/07/22 05:56, Seunghui Lee wrote:
>>>> -----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/co
>>>> mmit/
>>>> 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.
>>
>> I have started to look at it, but my time is limited at the moment.
>>
>> Note the original patch is 5 years old and fixes a real problem, so we
>> don't want to just throw it away.
>>
>
> Dear Mr. hunter,
>
> Could you check this with below patch?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/core/core.c?id=147186f531ae49c18b7a9091a2c40e83b3d95649
>
> In the mmc_set_uhs_voltaga func(),
> once it occurs error, it has power_cycle except R1_ERROR with CMD11.
> So, When mmc_set_uhs_voltage() return error,
> host and card can't leave 1.8V voltage.
>
> Regards,
>
>>>
>>> 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.
>>>
>
>
Does this help?
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index cee4c0b59f43..1abe8af48bfc 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -949,15 +949,15 @@ 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.
+ */
+ 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle
2022-08-09 5:36 ` Adrian Hunter
@ 2022-08-10 4:24 ` Seunghui Lee
2022-08-10 13:06 ` Adrian Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Seunghui Lee @ 2022-08-10 4:24 UTC (permalink / raw)
To: 'Adrian Hunter', linux-mmc; +Cc: 'DooHyun Hwang'
> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: Tuesday, August 9, 2022 2:37 PM
> To: Seunghui Lee <sh043.lee@samsung.com>; linux-mmc@vger.kernel.org
> Cc: 'DooHyun Hwang' <dh0421.hwang@samsung.com>
> Subject: Re: [PATCH] mmc: sd: Remove the patch that fix signal voltage
> when there is no power cycle
>
> On 8/08/22 11:24, Seunghui Lee wrote:
> >> -----Original Message-----
> >> From: Adrian Hunter <adrian.hunter@intel.com>
> >> Sent: Tuesday, July 26, 2022 7:57 PM
> >> To: Seunghui Lee <sh043.lee@samsung.com>; linux-mmc@vger.kernel.org;
> >> adrian.hunter@intel.com
> >> Cc: 'DooHyun Hwang' <dh0421.hwang@samsung.com>
> >> Subject: Re: [PATCH] mmc: sd: Remove the patch that fix signal
> >> voltage when there is no power cycle
> >>
> >> On 26/07/22 05:56, Seunghui Lee wrote:
> >>>> -----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/
> >>>> co
> >>>> mmit/
> >>>> 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.
> >>
> >> I have started to look at it, but my time is limited at the moment.
> >>
> >> Note the original patch is 5 years old and fixes a real problem, so
> >> we don't want to just throw it away.
> >>
> >
> > Dear Mr. hunter,
> >
> > Could you check this with below patch?
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com
> > mit/drivers/mmc/core/core.c?id=147186f531ae49c18b7a9091a2c40e83b3d9564
> > 9
> >
> > In the mmc_set_uhs_voltaga func(),
> > once it occurs error, it has power_cycle except R1_ERROR with CMD11.
> > So, When mmc_set_uhs_voltage() return error, host and card can't leave
> > 1.8V voltage.
> >
> > Regards,
> >
> >>>
> >>> 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.
> >>>
> >
> >
>
> Does this help?
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> cee4c0b59f43..1abe8af48bfc 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -949,15 +949,15 @@ 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.
> + */
> + 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
Dear Mr.Hunter,
Your suggestion works well :)
I've tested and verified the sd3_bus_mode's value updated.
So, May I update your suggestion as patch v2?
Or Would you like to your patch to contribute?
Regards,
Here is the test log.
[ 2347.726601] [4: kworker/4:98: 2227] [TEST] mmc_sd_get_cid: ocr 0x41040000(pocr 0x40000), retries 0.
[ 2347.726608] [4: kworker/4:98: 2227] mmc0: Skipping voltage switch
[ 2347.932495] [4: kworker/4:98: 2227] [TEST] mmc_read_switch: sd_switch ret 0, sd3_bus_mode=0x3.
[ 2347.932508] [4: kworker/4:98: 2227] [TEST] mmc_sd_init_card: 1116: caps=0x407f020f, sd3_bus_mode=0x3, v18_fixup_failed false, signal_voltage =0x0.
-> sd3_bus_mode is 0x3.
[ 2347.932514] [4: kworker/4:98: 2227] [TEST] mmc_sd_init_card: 1154: switch hs.
[ 2347.936315] [4: kworker/4:98: 2227] [TEST] mmc_sd_init_card: 1198: card->ocr 0x40000.
* next resume
[ 2348.156021] [0: kworker/0:6:26086] [TEST] mmc_sd_init_card: 1037: card->ocr 0x40000.
[ 2348.156065] [0: kworker/0:6:26086] [TEST] mmc_sd_get_cid: ocr 0x40000(pocr 0x40000), retries 10.
[ 2348.387214] [0: kworker/0:6:26086] [TEST] mmc_read_switch: sd_switch ret 0, sd3_bus_mode=0x1f.
-> sd3_bus_mode is 0x1f
[ 2348.387253] [0: kworker/0:6:26086] [TEST] mmc_sd_init_card: 1116: caps=0x407f020f, sd3_bus_mode=0x1f, v18_fixup_failed false, signal_voltage =0x1.
[ 2348.387273] [0: kworker/0:6:26086] [TEST] mmc_sd_init_card: 1146: rocr 0xc1ff8000, S18A, uhs.
[ 2348.388399] [0: kworker/0:6:26086] [TEST] mmc_sd_init_uhs_card: before update: caps 0x407f020f, sd3_bus_mode = 0x1f, sd_bus_speed = 3, card->ocr = 0x40000.
[ 2348.388427] [0: kworker/0:6:26086] [TEST] sd_update_bus_speed_mode: caps 0x407f020f, sd3_bus_mode = 0x1f, sd_bus_speed = 3, card->ocr = 0x40000.
[ 2348.390614] [0: kworker/0:6:26086] [TEST] sd_set_bus_speed_mode: sd_bus_speed=3, timing=6, uhs_max_dtr=208000000, card->ocr=0x40000.
[ 2348.393757] [0: kworker/0:6:26086] [TEST] mmc_sd_init_card: 1198: card->ocr 0x40000.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle
2022-08-10 4:24 ` Seunghui Lee
@ 2022-08-10 13:06 ` Adrian Hunter
0 siblings, 0 replies; 7+ messages in thread
From: Adrian Hunter @ 2022-08-10 13:06 UTC (permalink / raw)
To: Seunghui Lee, linux-mmc; +Cc: 'DooHyun Hwang'
On 10/08/22 07:24, Seunghui Lee wrote:
>> -----Original Message-----
>> From: Adrian Hunter <adrian.hunter@intel.com>
>> Sent: Tuesday, August 9, 2022 2:37 PM
>> To: Seunghui Lee <sh043.lee@samsung.com>; linux-mmc@vger.kernel.org
>> Cc: 'DooHyun Hwang' <dh0421.hwang@samsung.com>
>> Subject: Re: [PATCH] mmc: sd: Remove the patch that fix signal voltage
>> when there is no power cycle
>>
>> On 8/08/22 11:24, Seunghui Lee wrote:
>>>> -----Original Message-----
>>>> From: Adrian Hunter <adrian.hunter@intel.com>
>>>> Sent: Tuesday, July 26, 2022 7:57 PM
>>>> To: Seunghui Lee <sh043.lee@samsung.com>; linux-mmc@vger.kernel.org;
>>>> adrian.hunter@intel.com
>>>> Cc: 'DooHyun Hwang' <dh0421.hwang@samsung.com>
>>>> Subject: Re: [PATCH] mmc: sd: Remove the patch that fix signal
>>>> voltage when there is no power cycle
>>>>
>>>> On 26/07/22 05:56, Seunghui Lee wrote:
>>>>>> -----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/
>>>>>> co
>>>>>> mmit/
>>>>>> 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.
>>>>
>>>> I have started to look at it, but my time is limited at the moment.
>>>>
>>>> Note the original patch is 5 years old and fixes a real problem, so
>>>> we don't want to just throw it away.
>>>>
>>>
>>> Dear Mr. hunter,
>>>
>>> Could you check this with below patch?
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com
>>> mit/drivers/mmc/core/core.c?id=147186f531ae49c18b7a9091a2c40e83b3d9564
>>> 9
>>>
>>> In the mmc_set_uhs_voltaga func(),
>>> once it occurs error, it has power_cycle except R1_ERROR with CMD11.
>>> So, When mmc_set_uhs_voltage() return error, host and card can't leave
>>> 1.8V voltage.
>>>
>>> Regards,
>>>
>>>>>
>>>>> 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.
>>>>>
>>>
>>>
>>
>> Does this help?
>>
>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
>> cee4c0b59f43..1abe8af48bfc 100644
>> --- a/drivers/mmc/core/sd.c
>> +++ b/drivers/mmc/core/sd.c
>> @@ -949,15 +949,15 @@ 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.
>> + */
>> + 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
>
> Dear Mr.Hunter,
>
> Your suggestion works well :)
> I've tested and verified the sd3_bus_mode's value updated.
>
> So, May I update your suggestion as patch v2?
> Or Would you like to your patch to contribute?
I'll make a patch since there are related changes needed
and I would like to do some additional testing.
>
> Regards,
>
> Here is the test log.
>
> [ 2347.726601] [4: kworker/4:98: 2227] [TEST] mmc_sd_get_cid: ocr 0x41040000(pocr 0x40000), retries 0.
> [ 2347.726608] [4: kworker/4:98: 2227] mmc0: Skipping voltage switch
> [ 2347.932495] [4: kworker/4:98: 2227] [TEST] mmc_read_switch: sd_switch ret 0, sd3_bus_mode=0x3.
> [ 2347.932508] [4: kworker/4:98: 2227] [TEST] mmc_sd_init_card: 1116: caps=0x407f020f, sd3_bus_mode=0x3, v18_fixup_failed false, signal_voltage =0x0.
> -> sd3_bus_mode is 0x3.
> [ 2347.932514] [4: kworker/4:98: 2227] [TEST] mmc_sd_init_card: 1154: switch hs.
> [ 2347.936315] [4: kworker/4:98: 2227] [TEST] mmc_sd_init_card: 1198: card->ocr 0x40000.
>
> * next resume
> [ 2348.156021] [0: kworker/0:6:26086] [TEST] mmc_sd_init_card: 1037: card->ocr 0x40000.
> [ 2348.156065] [0: kworker/0:6:26086] [TEST] mmc_sd_get_cid: ocr 0x40000(pocr 0x40000), retries 10.
> [ 2348.387214] [0: kworker/0:6:26086] [TEST] mmc_read_switch: sd_switch ret 0, sd3_bus_mode=0x1f.
> -> sd3_bus_mode is 0x1f
> [ 2348.387253] [0: kworker/0:6:26086] [TEST] mmc_sd_init_card: 1116: caps=0x407f020f, sd3_bus_mode=0x1f, v18_fixup_failed false, signal_voltage =0x1.
> [ 2348.387273] [0: kworker/0:6:26086] [TEST] mmc_sd_init_card: 1146: rocr 0xc1ff8000, S18A, uhs.
> [ 2348.388399] [0: kworker/0:6:26086] [TEST] mmc_sd_init_uhs_card: before update: caps 0x407f020f, sd3_bus_mode = 0x1f, sd_bus_speed = 3, card->ocr = 0x40000.
> [ 2348.388427] [0: kworker/0:6:26086] [TEST] sd_update_bus_speed_mode: caps 0x407f020f, sd3_bus_mode = 0x1f, sd_bus_speed = 3, card->ocr = 0x40000.
> [ 2348.390614] [0: kworker/0:6:26086] [TEST] sd_set_bus_speed_mode: sd_bus_speed=3, timing=6, uhs_max_dtr=208000000, card->ocr=0x40000.
> [ 2348.393757] [0: kworker/0:6:26086] [TEST] mmc_sd_init_card: 1198: card->ocr 0x40000.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-10 13:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20220721052617epcas1p48967f8acf113372b2a9fc88cca40b2dc@epcas1p4.samsung.com>
2022-07-21 5:59 ` [PATCH] mmc: sd: Remove the patch that fix signal voltage when there is no power cycle Seunghui Lee
2022-07-26 2:56 ` Seunghui Lee
2022-07-26 10:56 ` Adrian Hunter
2022-08-08 8:24 ` Seunghui Lee
2022-08-09 5:36 ` Adrian Hunter
2022-08-10 4:24 ` Seunghui Lee
2022-08-10 13:06 ` Adrian Hunter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.