Re:[PATCH] mmc:Remove thr legacy mode clock setting operation

Re:[PATCH] mmc:Remove thr legacy mode clock setting operation

[谢飞]

Hi，Peng.：


Has you meet any issues without removing this line? or this is just code 
inpsection?
          In practical use, when some EMMC particles switch to the hs mode below, an error message cmd8 may appear. The reason for this is that the clock operation of setting the legacy mode was performed before switching to the hs mode, which caused the clock to switch too early. At this time, the communication between the host and card is abnormal


That is to say, switching the clocks of the host and card to high speed before switching to hs mode can cause communication issues between the host and card






BTW the upper "else" will met issue if you directly remove this line.

Delete this line directly. There will indeed be issues with the else above. I did not find it on my end because I did not open the compilation macro definition above. This is the patch I resubmitted







Best wishes




Thanks








At 2023-06-26 14:54:47, xf_hqu@163.com wrote:
>From: xiefei <xf_hqu@163.com>
>
>Due to the need to read the register value before
>switching to hs mode, the standard protocol does
>not explicitly specify that the setting before
>switching to hs mode is in legacy mode. Therefore,
>the code at this point may cause communication
>abnormalities between the host and card
>
>Signed-off-by: xiefei <xf_hqu@163.com>
>---
> drivers/mmc/mmc.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>index 1af6af82e6..96cc7e7332 100644
>--- a/drivers/mmc/mmc.c
>+++ b/drivers/mmc/mmc.c
>@@ -2136,9 +2136,7 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
> 	    mmc->selected_mode == MMC_HS_400 ||
> 	    mmc->selected_mode == MMC_HS_400_ES)
> 		mmc_set_card_speed(mmc, MMC_HS, true);
>-	else
> #endif
>-		mmc_set_clock(mmc, mmc->legacy_speed, MMC_CLK_ENABLE);
> 
> 	for_each_mmc_mode_by_pref(card_caps, mwt) {
> 		for_each_supported_width(card_caps & mwt->widths,
>-- 
>2.17.1

Re:[PATCH] mmc:Remove thr legacy mode clock setting operation

[谢飞]

Hi，Peng.：


Has you meet any issues without removing this line? or this is just code 
inpsection?
          In practical use, when some EMMC particles switch to the hs mode below, an error message cmd8 may appear. The reason for this is that the clock operation of setting the legacy mode was performed before switching to the hs mode, which caused the clock to switch too early. At this time, the communication between the host and card is abnormal


That is to say, switching the clocks of the host and card to high speed before switching to hs mode can cause communication issues between the host and card






BTW the upper "else" will met issue if you directly remove this line.

Delete this line directly. There will indeed be issues with the else above. I did not find it on my end because I did not open the compilation macro definition above. This is the patch I resubmitted







Best wishes




Thanks














在 2023-06-26 16:59:42，xf_hqu@163.com 写道：
>From: xiefei <xf_hqu@163.com>
>
>Due to the need to read the register value before
>switching to hs mode, the standard protocol does
>not explicitly specify that the setting before
>switching to hs mode is in legacy mode. Therefore,
>the code at this point may cause communication
>abnormalities between the host and card
>
>Signed-off-by: xiefei <xf_hqu@163.com>
>---
> drivers/mmc/mmc.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>index 1af6af82e6..96cc7e7332 100644
>--- a/drivers/mmc/mmc.c
>+++ b/drivers/mmc/mmc.c
>@@ -2136,9 +2136,7 @@ static int mmc_select_mode_and_width(struct mmc *mmc, uint card_caps)
> 	    mmc->selected_mode == MMC_HS_400 ||
> 	    mmc->selected_mode == MMC_HS_400_ES)
> 		mmc_set_card_speed(mmc, MMC_HS, true);
>-	else
> #endif
>-		mmc_set_clock(mmc, mmc->legacy_speed, MMC_CLK_ENABLE);
> 
> 	for_each_mmc_mode_by_pref(card_caps, mwt) {
> 		for_each_supported_width(card_caps & mwt->widths,
>-- 
>2.17.1