Re: [PATCH] mmc: core: sdio: hold retuning if sdio in 1-bit mode

[Adrian Hunter <adrian.hunter@intel.com>]

On 26/10/23 14:58, Bough Chen wrote:
>> -----Original Message-----
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>> Sent: 2023年9月14日 21:03
>> To: Bough Chen <haibo.chen@nxp.com>
>> Cc: adrian.hunter@intel.com; linux-mmc@vger.kernel.org; dl-linux-imx
>> <linux-imx@nxp.com>; hkallweit1@gmail.com
>> Subject: Re: [PATCH] mmc: core: sdio: hold retuning if sdio in 1-bit mode
>>
>> On Wed, 30 Aug 2023 at 11:35, <haibo.chen@nxp.com> wrote:
>>>
>>> From: Haibo Chen <haibo.chen@nxp.com>
>>>
>>> tuning only support in 4-bit mode or 8 bit mode, so in 1-bit mode,
>>> need to hold retuning.
>>>
>>> Find this issue when use manual tuning method on imx93. When system
>>> resume back, SDIO WIFI try to switch back to 4 bit mode, first will
>>> trigger retuning, and all tuning command failed.
>>>
>>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
>>
>> Applied for fixes and by adding a fixes tag (Fixes: dfa13ebbe334
>> ("mmc: host: Add facility to support re-tuning")) and a stable tag, thanks!
>>
>> Kind regards
>> Uffe
>>
>>
>>> ---
>>>  drivers/mmc/core/sdio.c | 8 +++++++-
>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index
>>> f64b9ac76a5c..5914516df2f7 100644
>>> --- a/drivers/mmc/core/sdio.c
>>> +++ b/drivers/mmc/core/sdio.c
>>> @@ -1089,8 +1089,14 @@ static int mmc_sdio_resume(struct mmc_host
>> *host)
>>>                 }
>>>                 err = mmc_sdio_reinit_card(host);
>>>         } else if (mmc_card_wake_sdio_irq(host)) {
>>> -               /* We may have switched to 1-bit mode during suspend */
>>> +               /*
>>> +                * We may have switched to 1-bit mode during suspend,
>>> +                * need to hold retuning, because tuning only supprt
>>> +                * 4-bit mode or 8 bit mode.
>>> +                */
>>> +               mmc_retune_hold_now(host);
>>>                 err = sdio_enable_4bit_bus(host->card);
> 
> Hi Ulf,
> 
> Here still contain one bug, if now in UHS-I mode, card clock maybe is 200MHz, without tuning, sdio_enable_4bit_bus() may return error if host can't sample the cmd response correctly.
> 
> So here, in suspend better to switch out the UHS-I mode first, and downgrade the card clock rate(<50MHz), then switch from 4bit to 1 bit mode.

For eMMC and their host controllers, changing down modes has often required special support in host controller drivers, with the creation of a number of a callbacks.  UHS-I could be just as problematic.

> Then in resume, send command to switch back to 4 bit mode can execute safely without tuning.
> 
> I just meet this issue when do system PM on i.MX6ULL.  usdhc in i.MX6ULL will totally lost power after system suspend, which means the previous tuning status will lost when resume back.
> When send cmd to switch back to 4 bit mode during system resume, without tuning, usdhc can't sample the cmd response correctly under 200MHz, will trigger cmd error, cause the sdio resume fail.
> 
> Just as Adrian mentioned before, here add the mode switch maybe risky. Any concern? Or any way to pre-set the tuning config in host controller resume?

Some drivers do save and restore the tuning value.  That is definitely worth investigating.

Another possibility is to see whether it is possible to make a version of the switch to 4-bit that is resilient to CRC errors so it still works without tuning.

> 
> Best Regards
> Haibo Chen
> 
>>> +               mmc_retune_release(host);
>>>         }
>>>
>>>         if (err)
>>> --
>>> 2.34.1
>>>