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

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

[haibo.chen]

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>
---
 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);
+		mmc_retune_release(host);
 	}
 
 	if (err)
-- 
2.34.1

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

[Adrian Hunter]

On 30/08/23 12:39, 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>

UHS-I is also not meant for 1-bit mode, but I guess switching modes
in this case is also risky.

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Fixes tag?

> ---
>  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);
> +		mmc_retune_release(host);
>  	}
>  
>  	if (err)

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

[Bough Chen]

> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: 2023年9月4日 16:12
> To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org;
> linux-mmc@vger.kernel.org
> Cc: 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 30/08/23 12:39, 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>
> 
> UHS-I is also not meant for 1-bit mode, but I guess switching modes in this case
> is also risky.
> 
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Fixes tag?

Fixes: 6b5eda369ac3 ("sdio: put active devices into 1-bit mode during suspend")

Not sure whether this is the correct fix tag, since this commit come from year 2010, at that time, re-tuning do not exist.

mmc_retune_hold_now() involve in 2017, from this commit: bf517d6fec71 ("mmc: core: Add mmc_retune_hold_now()),
mmc_retune logic involve in 2015, from this commit: dfa13ebbe334 ("mmc: host: Add facility to support re-tuning")

maybe use this: Fixes: dfa13ebbe334 ("mmc: host: Add facility to support re-tuning") ?

Best Regards
Haibo Chen

> 
> > ---
> >  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);
> > +		mmc_retune_release(host);
> >  	}
> >
> >  	if (err)

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

[Ulf Hansson]

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);
> +               mmc_retune_release(host);
>         }
>
>         if (err)
> --
> 2.34.1
>

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

[Bough Chen]

> -----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.
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?

Best Regards
Haibo Chen

> > +               mmc_retune_release(host);
> >         }
> >
> >         if (err)
> > --
> > 2.34.1
> >

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

[Adrian Hunter]

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
>>>