Re: sdhci-omap: issues with PM features since 5.16

Re: sdhci-omap: issues with PM features since 5.16

[Tony Lindgren]

Hi,

Adding the linux-mmc folks to Cc too.

* Romain Naour <romain.naour@smile.fr> [240130 11:20]:
> Le 29/01/2024 à 18:42, Romain Naour a écrit :
> > Le 29/01/2024 à 12:17, Tony Lindgren a écrit :
> >> So I'm still guessing your issue is with emmc not getting reinitialized
> >> properly as there's no mmc-pwrseq-emmc configured. Can you give it a
> >> try? See am5729-beagleboneai.dts for an example.
> 
> I can't add such mmc-pwrseq-emmc on the custom board, there is no gpio available
> to reset the emmc device.
> 
> To resume:
> - the emmc doesn't work with mmc-hs200-1_8v mode with PM runtime enabled
> - the emmc works with mmc-hs200-1_8v mode without PM runtime (patch series reverted)
> - the emmc works with mmc-ddr-1_8v mode with PM runtime enabled
> 
> AFAIU the hs200 mode requires some pin iodelay tuning (SDHCI_OMAP_REQUIRE_IODELAY)
> is sdhci_omap_runtime_{suspend,resume} needs to take care of that?
> 
> The mmc2 clock seems idle when mmc-hs200-1_8v and PM runtime are used.
> 
> omapconf dump prcm l3init
> 
> (mmc2 clock idle)
> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01070000 |
> 
> (mmc2 clock running)
> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01040002 |
> 
> Thoughts?

OK so if the emmc reset gpio is not available, seems we should do something
like the following patch to not set MMC_CAP_POWER_OFF_CARD unless the
cap-power-off-card devicetree property is set.

Care to give it a try and see if it helps?

Regards,

Tony

8< ----------------
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -1339,8 +1339,11 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 	/* R1B responses is required to properly manage HW busy detection. */
 	mmc->caps |= MMC_CAP_NEED_RSP_BUSY;
 
-	/* Allow card power off and runtime PM for eMMC/SD card devices */
-	mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_AGGRESSIVE_PM;
+	/*
+	 * Allow runtime PM for eMMC/SD card devices. Note that to power off
+	 * the card, the devicetree property cap-power-off-card must be set.
+	 */
+	mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
 
 	ret = sdhci_setup_host(host);
 	if (ret)
-- 
2.43.0

Re: sdhci-omap: issues with PM features since 5.16

[Romain Naour]

Hello,

Le 31/01/2024 à 11:30, Tony Lindgren a écrit :
> Hi,
> 
> Adding the linux-mmc folks to Cc too.
> 
> * Romain Naour <romain.naour@smile.fr> [240130 11:20]:
>> Le 29/01/2024 à 18:42, Romain Naour a écrit :
>>> Le 29/01/2024 à 12:17, Tony Lindgren a écrit :
>>>> So I'm still guessing your issue is with emmc not getting reinitialized
>>>> properly as there's no mmc-pwrseq-emmc configured. Can you give it a
>>>> try? See am5729-beagleboneai.dts for an example.
>>
>> I can't add such mmc-pwrseq-emmc on the custom board, there is no gpio available
>> to reset the emmc device.
>>
>> To resume:
>> - the emmc doesn't work with mmc-hs200-1_8v mode with PM runtime enabled
>> - the emmc works with mmc-hs200-1_8v mode without PM runtime (patch series reverted)
>> - the emmc works with mmc-ddr-1_8v mode with PM runtime enabled
>>
>> AFAIU the hs200 mode requires some pin iodelay tuning (SDHCI_OMAP_REQUIRE_IODELAY)
>> is sdhci_omap_runtime_{suspend,resume} needs to take care of that?
>>
>> The mmc2 clock seems idle when mmc-hs200-1_8v and PM runtime are used.
>>
>> omapconf dump prcm l3init
>>
>> (mmc2 clock idle)
>> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01070000 |
>>
>> (mmc2 clock running)
>> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01040002 |
>>
>> Thoughts?
> 
> OK so if the emmc reset gpio is not available, seems we should do something
> like the following patch to not set MMC_CAP_POWER_OFF_CARD unless the
> cap-power-off-card devicetree property is set.
> 
> Care to give it a try and see if it helps?

Same problem without MMC_CAP_POWER_OFF_CARD flag (even by removing
MMC_CAP_AGGRESSIVE_PM too).

I did some test with mmc capabilities mask but no progress so far.

Best regards,
Romain


> 
> Regards,
> 
> Tony
> 
> 8< ----------------
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -1339,8 +1339,11 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>  	/* R1B responses is required to properly manage HW busy detection. */
>  	mmc->caps |= MMC_CAP_NEED_RSP_BUSY;
>  
> -	/* Allow card power off and runtime PM for eMMC/SD card devices */
> -	mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_AGGRESSIVE_PM;
> +	/*
> +	 * Allow runtime PM for eMMC/SD card devices. Note that to power off
> +	 * the card, the devicetree property cap-power-off-card must be set.
> +	 */
> +	mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
>  
>  	ret = sdhci_setup_host(host);
>  	if (ret)

Re: sdhci-omap: issues with PM features since 5.16

[Tony Lindgren]

* Romain Naour <romain.naour@smile.fr> [240201 09:04]:
> Le 31/01/2024 à 11:30, Tony Lindgren a écrit :
> > * Romain Naour <romain.naour@smile.fr> [240130 11:20]:
> >> Le 29/01/2024 à 18:42, Romain Naour a écrit :
> >>> Le 29/01/2024 à 12:17, Tony Lindgren a écrit :
> >>>> So I'm still guessing your issue is with emmc not getting reinitialized
> >>>> properly as there's no mmc-pwrseq-emmc configured. Can you give it a
> >>>> try? See am5729-beagleboneai.dts for an example.
> >>
> >> I can't add such mmc-pwrseq-emmc on the custom board, there is no gpio available
> >> to reset the emmc device.
> >>
> >> To resume:
> >> - the emmc doesn't work with mmc-hs200-1_8v mode with PM runtime enabled
> >> - the emmc works with mmc-hs200-1_8v mode without PM runtime (patch series reverted)
> >> - the emmc works with mmc-ddr-1_8v mode with PM runtime enabled
> >>
> >> AFAIU the hs200 mode requires some pin iodelay tuning (SDHCI_OMAP_REQUIRE_IODELAY)
> >> is sdhci_omap_runtime_{suspend,resume} needs to take care of that?

On PM runtime resume, sdhci_omap_runtime_resume() gets called and calls
sdhci_runtime_resume_host(), and calls mmc->ops->set_ios().

Then sdhci_omap_set_ios() calls sdhci_omap_set_timing() to set the iodelay.
Maybe add some printk to sdhci_omap_set_timing() to verify the right modes
get set on PM runtime resume?

> >> The mmc2 clock seems idle when mmc-hs200-1_8v and PM runtime are used.
> >>
> >> omapconf dump prcm l3init
> >>
> >> (mmc2 clock idle)
> >> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01070000 |
> >>
> >> (mmc2 clock running)
> >> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01040002 |
> >>
> >> Thoughts?

For the clocks above, that is as expected. The clocks get idled when the
MMC controller is idle.

> > OK so if the emmc reset gpio is not available, seems we should do something
> > like the following patch to not set MMC_CAP_POWER_OFF_CARD unless the
> > cap-power-off-card devicetree property is set.
> > 
> > Care to give it a try and see if it helps?
> 
> Same problem without MMC_CAP_POWER_OFF_CARD flag (even by removing
> MMC_CAP_AGGRESSIVE_PM too).
> 
> I did some test with mmc capabilities mask but no progress so far.

OK. So this issue seems to be related to the PM runtime resume not
restoring something properly as you suggested earlier.

Regards,

Tony

Re: sdhci-omap: issues with PM features since 5.16

[Romain Naour]

Le 02/02/2024 à 05:36, Tony Lindgren a écrit :
> * Romain Naour <romain.naour@smile.fr> [240201 09:04]:
>> Le 31/01/2024 à 11:30, Tony Lindgren a écrit :
>>> * Romain Naour <romain.naour@smile.fr> [240130 11:20]:
>>>> Le 29/01/2024 à 18:42, Romain Naour a écrit :
>>>>> Le 29/01/2024 à 12:17, Tony Lindgren a écrit :
>>>>>> So I'm still guessing your issue is with emmc not getting reinitialized
>>>>>> properly as there's no mmc-pwrseq-emmc configured. Can you give it a
>>>>>> try? See am5729-beagleboneai.dts for an example.
>>>>
>>>> I can't add such mmc-pwrseq-emmc on the custom board, there is no gpio available
>>>> to reset the emmc device.
>>>>
>>>> To resume:
>>>> - the emmc doesn't work with mmc-hs200-1_8v mode with PM runtime enabled
>>>> - the emmc works with mmc-hs200-1_8v mode without PM runtime (patch series reverted)
>>>> - the emmc works with mmc-ddr-1_8v mode with PM runtime enabled
>>>>
>>>> AFAIU the hs200 mode requires some pin iodelay tuning (SDHCI_OMAP_REQUIRE_IODELAY)
>>>> is sdhci_omap_runtime_{suspend,resume} needs to take care of that?
> 
> On PM runtime resume, sdhci_omap_runtime_resume() gets called and calls
> sdhci_runtime_resume_host(), and calls mmc->ops->set_ios().
> 
> Then sdhci_omap_set_ios() calls sdhci_omap_set_timing() to set the iodelay.
> Maybe add some printk to sdhci_omap_set_timing() to verify the right modes
> get set on PM runtime resume?
> 
>>>> The mmc2 clock seems idle when mmc-hs200-1_8v and PM runtime are used.
>>>>
>>>> omapconf dump prcm l3init
>>>>
>>>> (mmc2 clock idle)
>>>> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01070000 |
>>>>
>>>> (mmc2 clock running)
>>>> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01040002 |
>>>>
>>>> Thoughts?
> 
> For the clocks above, that is as expected. The clocks get idled when the
> MMC controller is idle.
> 
>>> OK so if the emmc reset gpio is not available, seems we should do something
>>> like the following patch to not set MMC_CAP_POWER_OFF_CARD unless the
>>> cap-power-off-card devicetree property is set.
>>>
>>> Care to give it a try and see if it helps?
>>
>> Same problem without MMC_CAP_POWER_OFF_CARD flag (even by removing
>> MMC_CAP_AGGRESSIVE_PM too).
>>
>> I did some test with mmc capabilities mask but no progress so far.
> 
> OK. So this issue seems to be related to the PM runtime resume not
> restoring something properly as you suggested earlier.

Adding your PM reply with the mailing list in Cc:

Le 06/02/2024 à 09:25, Tony Lindgren a écrit :
> * Tony Lindgren <tony@atomide.com> [240202 10:30]:
[...]
>
> When you get a chance, maybe give the following debug patch a try.
> I'm mostly seeing value of 2 and sometimes 0, but that could be
> for a different mmc controller instance as I just used pr_info.
> So you may need to tweak the debug patch to use dev_dbg to leave
> out other controllers.
>
> #define MMC_POWER_OFF           0
> #define MMC_POWER_UP            1
> #define MMC_POWER_ON            2
> #define MMC_POWER_UNDEFINED     3
>
> So on MMC_POWER_OFF, in sdhci_runtime_resume_host() the flag
> host->reinit_uhs = true does not get set, and maybe with hs200
> that causes the failure?

With the debug line added, I don't see any MMC_POWER_OFF for the emmc but only
MMC_POWER_ON lines:

XXX sdhci_runtime_resume_host: mmc->ios.power_mode: 2

>
> If you're seeing MMC_POWER_OFF values, maybe also try changing
> sdhci_omap_runtime_resume() to call sdhci_runtime_resume_host(host, 1)
> and see if that helps as requesting a soft reset causes sdhci_init() to
> set host->reinit_uhs = true.. That change feels like a workaround
> though.

I tried anyway with soft reset, the cache flush error is gone but now I have
this dump in dmesg each time the emmc is reset:

[ 3978.852783] mmc1: Reset 0x6 never completed.
[ 3978.852783] mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
[ 3978.852783] mmc1: sdhci: Sys addr:  0x00000000 | Version:  0x00003302
[ 3978.852813] mmc1: sdhci: Blk size:  0x00000000 | Blk cnt:  0x00000000
[ 3978.852813] mmc1: sdhci: Argument:  0x00000000 | Trn mode: 0x00000000
[ 3978.852813] mmc1: sdhci: Present:   0x01f00000 | Host ctl: 0x00000000
[ 3978.852813] mmc1: sdhci: Power:     0x00000000 | Blk gap:  0x00000000
[ 3978.852813] mmc1: sdhci: Wake-up:   0x00000000 | Clock:    0x00000000
[ 3978.852844] mmc1: sdhci: Timeout:   0x00000000 | Int stat: 0x00000000
[ 3978.852844] mmc1: sdhci: Int enab:  0x00000000 | Sig enab: 0x00000000
[ 3978.852844] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
[ 3978.852844] mmc1: sdhci: Caps:      0x24e90080 | Caps_1:   0x00000f77
[ 3978.852844] mmc1: sdhci: Cmd:       0x00000000 | Max curr: 0x00000000
[ 3978.852874] mmc1: sdhci: Resp[0]:   0x00000000 | Resp[1]:  0x00000000
[ 3978.852874] mmc1: sdhci: Resp[2]:   0x00000000 | Resp[3]:  0x00000000
[ 3978.852874] mmc1: sdhci: Host ctl2: 0x00000000
[ 3978.852874] mmc1: sdhci: ADMA Err:  0x00000000 | ADMA Ptr: 0x00000000
[ 3978.852874] mmc1: sdhci: ============================================

With sdhci soft reset enabled, there are some MMC_POWER_OFF in dmesg

[ 3978.852905] XXX sdhci_runtime_resume_host: mmc->ios.power_mode: 0
[ 3982.217590] XXX sdhci_runtime_resume_host: mmc->ios.power_mode: 2

The iodelay pin setting is still applied after the emmc is reset:

# omapconf dump 0x4A00348c 0x4A0034ac
|----------------------------|
| Address (hex) | Data (hex) |
|----------------------------|
| 0x4A00348C    | 0x00070101 |
| 0x4A003490    | 0x00070101 |
| 0x4A003494    | 0x00070101 |
| 0x4A003498    | 0x00070101 |
| 0x4A00349C    | 0x00060101 |
| 0x4A0034A0    | 0x00070101 |
| 0x4A0034A4    | 0x00070101 |
| 0x4A0034A8    | 0x00070101 |
| 0x4A0034AC    | 0x00070101 |
|----------------------------|

>
> Regards,
>
> Tony
>
> 8< ------
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3848,6 +3848,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host,
int soft_reset)
>  	}
>
>  	sdhci_init(host, soft_reset);
> +	pr_info("XXX %s: mmc->ios.power_mode: %i\n", __func__, mmc->ios.power_mode);
>
>  	if (mmc->ios.power_mode != MMC_POWER_UNDEFINED &&
>  	    mmc->ios.power_mode != MMC_POWER_OFF) {


Best regards,
Romain


> 
> Regards,
> 
> Tony

Re: sdhci-omap: issues with PM features since 5.16

[Adam Ford]

On Thu, Feb 8, 2024 at 5:32 AM Romain Naour <romain.naour@smile.fr> wrote:
>
> Le 02/02/2024 à 05:36, Tony Lindgren a écrit :
> > * Romain Naour <romain.naour@smile.fr> [240201 09:04]:
> >> Le 31/01/2024 à 11:30, Tony Lindgren a écrit :
> >>> * Romain Naour <romain.naour@smile.fr> [240130 11:20]:
> >>>> Le 29/01/2024 à 18:42, Romain Naour a écrit :
> >>>>> Le 29/01/2024 à 12:17, Tony Lindgren a écrit :
> >>>>>> So I'm still guessing your issue is with emmc not getting reinitialized
> >>>>>> properly as there's no mmc-pwrseq-emmc configured. Can you give it a
> >>>>>> try? See am5729-beagleboneai.dts for an example.
> >>>>
> >>>> I can't add such mmc-pwrseq-emmc on the custom board, there is no gpio available
> >>>> to reset the emmc device.
> >>>>
> >>>> To resume:
> >>>> - the emmc doesn't work with mmc-hs200-1_8v mode with PM runtime enabled
> >>>> - the emmc works with mmc-hs200-1_8v mode without PM runtime (patch series reverted)
> >>>> - the emmc works with mmc-ddr-1_8v mode with PM runtime enabled
> >>>>
> >>>> AFAIU the hs200 mode requires some pin iodelay tuning (SDHCI_OMAP_REQUIRE_IODELAY)
> >>>> is sdhci_omap_runtime_{suspend,resume} needs to take care of that?
> >
> > On PM runtime resume, sdhci_omap_runtime_resume() gets called and calls
> > sdhci_runtime_resume_host(), and calls mmc->ops->set_ios().
> >
> > Then sdhci_omap_set_ios() calls sdhci_omap_set_timing() to set the iodelay.
> > Maybe add some printk to sdhci_omap_set_timing() to verify the right modes
> > get set on PM runtime resume?
> >
> >>>> The mmc2 clock seems idle when mmc-hs200-1_8v and PM runtime are used.
> >>>>
> >>>> omapconf dump prcm l3init
> >>>>
> >>>> (mmc2 clock idle)
> >>>> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01070000 |
> >>>>
> >>>> (mmc2 clock running)
> >>>> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01040002 |
> >>>>
> >>>> Thoughts?
> >
> > For the clocks above, that is as expected. The clocks get idled when the
> > MMC controller is idle.
> >
> >>> OK so if the emmc reset gpio is not available, seems we should do something
> >>> like the following patch to not set MMC_CAP_POWER_OFF_CARD unless the
> >>> cap-power-off-card devicetree property is set.
> >>>
> >>> Care to give it a try and see if it helps?
> >>
> >> Same problem without MMC_CAP_POWER_OFF_CARD flag (even by removing
> >> MMC_CAP_AGGRESSIVE_PM too).
> >>
> >> I did some test with mmc capabilities mask but no progress so far.
> >
> > OK. So this issue seems to be related to the PM runtime resume not
> > restoring something properly as you suggested earlier.

I was testing the newer MMC driver on an AM3517 a few months ago, and
I noticed that SD cards were returning weird or empty data after some
indeterminate amount of time.  If I read the IOS file immediately
after boot, it worked.  If I forced some sort of file IO, it might
work, but after it went idle, it appeared to have strange data.  I
don't think this bug is limited to one platform.

adam

>
> Adding your PM reply with the mailing list in Cc:
>
> Le 06/02/2024 à 09:25, Tony Lindgren a écrit :
> > * Tony Lindgren <tony@atomide.com> [240202 10:30]:
> [...]
> >
> > When you get a chance, maybe give the following debug patch a try.
> > I'm mostly seeing value of 2 and sometimes 0, but that could be
> > for a different mmc controller instance as I just used pr_info.
> > So you may need to tweak the debug patch to use dev_dbg to leave
> > out other controllers.
> >
> > #define MMC_POWER_OFF           0
> > #define MMC_POWER_UP            1
> > #define MMC_POWER_ON            2
> > #define MMC_POWER_UNDEFINED     3
> >
> > So on MMC_POWER_OFF, in sdhci_runtime_resume_host() the flag
> > host->reinit_uhs = true does not get set, and maybe with hs200
> > that causes the failure?
>
> With the debug line added, I don't see any MMC_POWER_OFF for the emmc but only
> MMC_POWER_ON lines:
>
> XXX sdhci_runtime_resume_host: mmc->ios.power_mode: 2
>
> >
> > If you're seeing MMC_POWER_OFF values, maybe also try changing
> > sdhci_omap_runtime_resume() to call sdhci_runtime_resume_host(host, 1)
> > and see if that helps as requesting a soft reset causes sdhci_init() to
> > set host->reinit_uhs = true.. That change feels like a workaround
> > though.
>
> I tried anyway with soft reset, the cache flush error is gone but now I have
> this dump in dmesg each time the emmc is reset:
>
> [ 3978.852783] mmc1: Reset 0x6 never completed.
> [ 3978.852783] mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
> [ 3978.852783] mmc1: sdhci: Sys addr:  0x00000000 | Version:  0x00003302
> [ 3978.852813] mmc1: sdhci: Blk size:  0x00000000 | Blk cnt:  0x00000000
> [ 3978.852813] mmc1: sdhci: Argument:  0x00000000 | Trn mode: 0x00000000
> [ 3978.852813] mmc1: sdhci: Present:   0x01f00000 | Host ctl: 0x00000000
> [ 3978.852813] mmc1: sdhci: Power:     0x00000000 | Blk gap:  0x00000000
> [ 3978.852813] mmc1: sdhci: Wake-up:   0x00000000 | Clock:    0x00000000
> [ 3978.852844] mmc1: sdhci: Timeout:   0x00000000 | Int stat: 0x00000000
> [ 3978.852844] mmc1: sdhci: Int enab:  0x00000000 | Sig enab: 0x00000000
> [ 3978.852844] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
> [ 3978.852844] mmc1: sdhci: Caps:      0x24e90080 | Caps_1:   0x00000f77
> [ 3978.852844] mmc1: sdhci: Cmd:       0x00000000 | Max curr: 0x00000000
> [ 3978.852874] mmc1: sdhci: Resp[0]:   0x00000000 | Resp[1]:  0x00000000
> [ 3978.852874] mmc1: sdhci: Resp[2]:   0x00000000 | Resp[3]:  0x00000000
> [ 3978.852874] mmc1: sdhci: Host ctl2: 0x00000000
> [ 3978.852874] mmc1: sdhci: ADMA Err:  0x00000000 | ADMA Ptr: 0x00000000
> [ 3978.852874] mmc1: sdhci: ============================================
>
> With sdhci soft reset enabled, there are some MMC_POWER_OFF in dmesg
>
> [ 3978.852905] XXX sdhci_runtime_resume_host: mmc->ios.power_mode: 0
> [ 3982.217590] XXX sdhci_runtime_resume_host: mmc->ios.power_mode: 2
>
> The iodelay pin setting is still applied after the emmc is reset:
>
> # omapconf dump 0x4A00348c 0x4A0034ac
> |----------------------------|
> | Address (hex) | Data (hex) |
> |----------------------------|
> | 0x4A00348C    | 0x00070101 |
> | 0x4A003490    | 0x00070101 |
> | 0x4A003494    | 0x00070101 |
> | 0x4A003498    | 0x00070101 |
> | 0x4A00349C    | 0x00060101 |
> | 0x4A0034A0    | 0x00070101 |
> | 0x4A0034A4    | 0x00070101 |
> | 0x4A0034A8    | 0x00070101 |
> | 0x4A0034AC    | 0x00070101 |
> |----------------------------|
>
> >
> > Regards,
> >
> > Tony
> >
> > 8< ------
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -3848,6 +3848,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host,
> int soft_reset)
> >       }
> >
> >       sdhci_init(host, soft_reset);
> > +     pr_info("XXX %s: mmc->ios.power_mode: %i\n", __func__, mmc->ios.power_mode);
> >
> >       if (mmc->ios.power_mode != MMC_POWER_UNDEFINED &&
> >           mmc->ios.power_mode != MMC_POWER_OFF) {
>
>
> Best regards,
> Romain
>
>
> >
> > Regards,
> >
> > Tony
>
>

Re: sdhci-omap: issues with PM features since 5.16

[Tony Lindgren]

* Adam Ford <aford173@gmail.com> [240208 12:57]:
> I was testing the newer MMC driver on an AM3517 a few months ago, and
> I noticed that SD cards were returning weird or empty data after some
> indeterminate amount of time.  If I read the IOS file immediately
> after boot, it worked.  If I forced some sort of file IO, it might
> work, but after it went idle, it appeared to have strange data.  I
> don't think this bug is limited to one platform.

OK interesting. So the zero values for ios are expected when the card
is idle. What do you mean by weird values when idle?

Can you please try catch the weird values or a failure maybe with
something like:

while true; do
      cat /sys/kernel/debug/mmc0/ios
      fdisk -l /dev/mmcblk0
      cat /sys/kernel/debug/mmc0/ios
      sleep 20
done

Regards,

Tony

Re: sdhci-omap: issues with PM features since 5.16

[Adam Ford]

On Mon, Feb 12, 2024 at 1:28 AM Tony Lindgren <tony@atomide.com> wrote:
>
> * Adam Ford <aford173@gmail.com> [240208 12:57]:
> > I was testing the newer MMC driver on an AM3517 a few months ago, and
> > I noticed that SD cards were returning weird or empty data after some
> > indeterminate amount of time.  If I read the IOS file immediately
> > after boot, it worked.  If I forced some sort of file IO, it might
> > work, but after it went idle, it appeared to have strange data.  I
> > don't think this bug is limited to one platform.
>
> OK interesting. So the zero values for ios are expected when the card
> is idle. What do you mean by weird values when idle?

I meant zero-values when I said weird.  It's weird to me, because
other platforms don't return zero.
If tha's normal, I'll likely submit an update to omap3.dtsi to migrate
the MMC driver to the newer one.
I'd been meaning to do that for a while, but this threw me off, then i
forgot about it until I saw this thread.

>
> Can you please try catch the weird values or a failure maybe with
> something like:
>
> while true; do
>       cat /sys/kernel/debug/mmc0/ios
>       fdisk -l /dev/mmcblk0
>       cat /sys/kernel/debug/mmc0/ios
>       sleep 20
> done

It appears the first time, that runs after fdisk, the values seems
reasonable, but subsequent iterations, the values are zero again:

clock:          0 Hz
vdd:            0 (invalid)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     0 (off)
bus width:      0 (1 bits)
timing spec:    0 (legacy)
signal voltage: 0 (3.30 V)
driver type:    0 (driver type B)
Disk /dev/mmcblk0: 3.75 GiB, 4025483264 bytes, 7862272 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x00000000

Device         Boot  Start     End Sectors  Size Id Type
/dev/mmcblk0p1 *         1  512000  512000  250M  c W95 FAT32 (LBA)
/dev/mmcblk0p2      512001 1560576 1048576  512M 83 Linux
clock:          50000000 Hz
vdd:            21 (3.3 ~ 3.4 V)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     2 (on)
bus width:      2 (4 bits)
timing spec:    2 (sd high-speed)
signal voltage: 0 (3.30 V)
driver type:    0 (driver type B)
clock:          0 Hz
vdd:            0 (invalid)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     0 (off)
bus width:      0 (1 bits)
timing spec:    0 (legacy)
signal voltage: 0 (3.30 V)
driver type:    0 (driver type B)
Disk /dev/mmcblk0: 3.75 GiB, 4025483264 bytes, 7862272 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x00000000

Device         Boot  Start     End Sectors  Size Id Type
/dev/mmcblk0p1 *         1  512000  512000  250M  c W95 FAT32 (LBA)
/dev/mmcblk0p2      512001 1560576 1048576  512M 83 Linux
clock:          0 Hz
vdd:            0 (invalid)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     0 (off)
bus width:      0 (1 bits)
timing spec:    0 (legacy)
signal voltage: 0 (3.30 V)
driver type:    0 (driver type B)
clock:          0 Hz
vdd:            0 (invalid)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     0 (off)
bus width:      0 (1 bits)
timing spec:    0 (legacy)
signal voltage: 0 (3.30 V)
driver type:    0 (driver type B)
Disk /dev/mmcblk0: 3.75 GiB, 4025483264 bytes, 7862272 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x00000000

Device         Boot  Start     End Sectors  Size Id Type
/dev/mmcblk0p1 *         1  512000  512000  250M  c W95 FAT32 (LBA)
/dev/mmcblk0p2      512001 1560576 1048576  512M 83 Linux
clock:          0 Hz
vdd:            0 (invalid)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     0 (off)
bus width:      0 (1 bits)
timing spec:    0 (legacy)
signal voltage: 0 (3.30 V)
driver type:    0 (driver type B)

This was on an AM3517 with an SD card.  I don't have eMMC on this
platform to try.

adam
>
> Regards,
>
> Tony

Re: sdhci-omap: issues with PM features since 5.16

[Tony Lindgren]

* Adam Ford <aford173@gmail.com> [240212 13:24]:
> On Mon, Feb 12, 2024 at 1:28 AM Tony Lindgren <tony@atomide.com> wrote:
> >
> > * Adam Ford <aford173@gmail.com> [240208 12:57]:
> > > I was testing the newer MMC driver on an AM3517 a few months ago, and
> > > I noticed that SD cards were returning weird or empty data after some
> > > indeterminate amount of time.  If I read the IOS file immediately
> > > after boot, it worked.  If I forced some sort of file IO, it might
> > > work, but after it went idle, it appeared to have strange data.  I
> > > don't think this bug is limited to one platform.
> >
> > OK interesting. So the zero values for ios are expected when the card
> > is idle. What do you mean by weird values when idle?
> 
> I meant zero-values when I said weird.  It's weird to me, because
> other platforms don't return zero.
> If tha's normal, I'll likely submit an update to omap3.dtsi to migrate
> the MMC driver to the newer one.
> I'd been meaning to do that for a while, but this threw me off, then i
> forgot about it until I saw this thread.

Yes I'd like to enable sdhci-omap and deprecate omap_hsmmc too.

Assuming no issues and if the hs200 issue is a separate issue, maybe
post the patches and then let's get some Tested-by for them before we
put them into Linux next.

> > Can you please try catch the weird values or a failure maybe with
> > something like:
> >
> > while true; do
> >       cat /sys/kernel/debug/mmc0/ios
> >       fdisk -l /dev/mmcblk0
> >       cat /sys/kernel/debug/mmc0/ios
> >       sleep 20
> > done
> 
> It appears the first time, that runs after fdisk, the values seems
> reasonable, but subsequent iterations, the values are zero again:

Seems expected to me. Before fdisk -l the mmc controller may have
autoidled showing mostly zero values, and after any access to the mmc
card the card is enabled and the values are the normal enabled values.

If, however, fdisk -l does not work, or the values right after fdisk -l
are mostly zero, then that would indicate a problem. I guess leaving
out the reading of ios before fdisk -l would make it easier to see.

Regards,

Tony

Re: sdhci-omap: issues with PM features since 5.16

[Adam Ford]

On Tue, Feb 13, 2024 at 12:03 AM Tony Lindgren <tony@atomide.com> wrote:
>
> * Adam Ford <aford173@gmail.com> [240212 13:24]:
> > On Mon, Feb 12, 2024 at 1:28 AM Tony Lindgren <tony@atomide.com> wrote:
> > >
> > > * Adam Ford <aford173@gmail.com> [240208 12:57]:
> > > > I was testing the newer MMC driver on an AM3517 a few months ago, and
> > > > I noticed that SD cards were returning weird or empty data after some
> > > > indeterminate amount of time.  If I read the IOS file immediately
> > > > after boot, it worked.  If I forced some sort of file IO, it might
> > > > work, but after it went idle, it appeared to have strange data.  I
> > > > don't think this bug is limited to one platform.
> > >
> > > OK interesting. So the zero values for ios are expected when the card
> > > is idle. What do you mean by weird values when idle?
> >
> > I meant zero-values when I said weird.  It's weird to me, because
> > other platforms don't return zero.
> > If tha's normal, I'll likely submit an update to omap3.dtsi to migrate
> > the MMC driver to the newer one.
> > I'd been meaning to do that for a while, but this threw me off, then i
> > forgot about it until I saw this thread.
>
> Yes I'd like to enable sdhci-omap and deprecate omap_hsmmc too.
>
> Assuming no issues and if the hs200 issue is a separate issue, maybe
> post the patches and then let's get some Tested-by for them before we
> put them into Linux next.

I just sent a patch against omap3.dtsi.
I am not sure how to handle the backwards compatible device tree breakage.

Maybe since it's been around long enough, it might be ok.

adam
>
> > > Can you please try catch the weird values or a failure maybe with
> > > something like:
> > >
> > > while true; do
> > >       cat /sys/kernel/debug/mmc0/ios
> > >       fdisk -l /dev/mmcblk0
> > >       cat /sys/kernel/debug/mmc0/ios
> > >       sleep 20
> > > done
> >
> > It appears the first time, that runs after fdisk, the values seems
> > reasonable, but subsequent iterations, the values are zero again:
>
> Seems expected to me. Before fdisk -l the mmc controller may have
> autoidled showing mostly zero values, and after any access to the mmc
> card the card is enabled and the values are the normal enabled values.
>
> If, however, fdisk -l does not work, or the values right after fdisk -l
> are mostly zero, then that would indicate a problem. I guess leaving
> out the reading of ios before fdisk -l would make it easier to see.
>
> Regards,
>
> Tony

Re: sdhci-omap: issues with PM features since 5.16

[Tony Lindgren]

* Adam Ford <aford173@gmail.com> [240213 12:43]:
> On Tue, Feb 13, 2024 at 12:03 AM Tony Lindgren <tony@atomide.com> wrote:
> > Assuming no issues and if the hs200 issue is a separate issue, maybe
> > post the patches and then let's get some Tested-by for them before we
> > put them into Linux next.
> 
> I just sent a patch against omap3.dtsi.
> I am not sure how to handle the backwards compatible device tree breakage.

Thanks, I replied to the patch on the properties for sdhci.

Regards,

Tony

Re: sdhci-omap: issues with PM features since 5.16

[Romain Naour]

Hello Tony,

Le 08/02/2024 à 12:29, Romain Naour a écrit :
> Le 02/02/2024 à 05:36, Tony Lindgren a écrit :
>> * Romain Naour <romain.naour@smile.fr> [240201 09:04]:
>>> Le 31/01/2024 à 11:30, Tony Lindgren a écrit :
>>>> * Romain Naour <romain.naour@smile.fr> [240130 11:20]:
>>>>> Le 29/01/2024 à 18:42, Romain Naour a écrit :
>>>>>> Le 29/01/2024 à 12:17, Tony Lindgren a écrit :
>>>>>>> So I'm still guessing your issue is with emmc not getting reinitialized
>>>>>>> properly as there's no mmc-pwrseq-emmc configured. Can you give it a
>>>>>>> try? See am5729-beagleboneai.dts for an example.
>>>>>
>>>>> I can't add such mmc-pwrseq-emmc on the custom board, there is no gpio available
>>>>> to reset the emmc device.
>>>>>
>>>>> To resume:
>>>>> - the emmc doesn't work with mmc-hs200-1_8v mode with PM runtime enabled
>>>>> - the emmc works with mmc-hs200-1_8v mode without PM runtime (patch series reverted)
>>>>> - the emmc works with mmc-ddr-1_8v mode with PM runtime enabled
>>>>>
>>>>> AFAIU the hs200 mode requires some pin iodelay tuning (SDHCI_OMAP_REQUIRE_IODELAY)
>>>>> is sdhci_omap_runtime_{suspend,resume} needs to take care of that?
>>
>> On PM runtime resume, sdhci_omap_runtime_resume() gets called and calls
>> sdhci_runtime_resume_host(), and calls mmc->ops->set_ios().
>>
>> Then sdhci_omap_set_ios() calls sdhci_omap_set_timing() to set the iodelay.
>> Maybe add some printk to sdhci_omap_set_timing() to verify the right modes
>> get set on PM runtime resume?
>>
>>>>> The mmc2 clock seems idle when mmc-hs200-1_8v and PM runtime are used.
>>>>>
>>>>> omapconf dump prcm l3init
>>>>>
>>>>> (mmc2 clock idle)
>>>>> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01070000 |
>>>>>
>>>>> (mmc2 clock running)
>>>>> | CM_L3INIT_MMC2_CLKCTRL           | 0x4A009330   | 0x01040002 |
>>>>>
>>>>> Thoughts?
>>
>> For the clocks above, that is as expected. The clocks get idled when the
>> MMC controller is idle.
>>
>>>> OK so if the emmc reset gpio is not available, seems we should do something
>>>> like the following patch to not set MMC_CAP_POWER_OFF_CARD unless the
>>>> cap-power-off-card devicetree property is set.
>>>>
>>>> Care to give it a try and see if it helps?
>>>
>>> Same problem without MMC_CAP_POWER_OFF_CARD flag (even by removing
>>> MMC_CAP_AGGRESSIVE_PM too).
>>>
>>> I did some test with mmc capabilities mask but no progress so far.
>>
>> OK. So this issue seems to be related to the PM runtime resume not
>> restoring something properly as you suggested earlier.
> 
> Adding your PM reply with the mailing list in Cc:
> 
> Le 06/02/2024 à 09:25, Tony Lindgren a écrit :
>> * Tony Lindgren <tony@atomide.com> [240202 10:30]:
> [...]
>>
>> When you get a chance, maybe give the following debug patch a try.
>> I'm mostly seeing value of 2 and sometimes 0, but that could be
>> for a different mmc controller instance as I just used pr_info.
>> So you may need to tweak the debug patch to use dev_dbg to leave
>> out other controllers.
>>
>> #define MMC_POWER_OFF           0
>> #define MMC_POWER_UP            1
>> #define MMC_POWER_ON            2
>> #define MMC_POWER_UNDEFINED     3
>>
>> So on MMC_POWER_OFF, in sdhci_runtime_resume_host() the flag
>> host->reinit_uhs = true does not get set, and maybe with hs200
>> that causes the failure?
> 
> With the debug line added, I don't see any MMC_POWER_OFF for the emmc but only
> MMC_POWER_ON lines:
> 
> XXX sdhci_runtime_resume_host: mmc->ios.power_mode: 2
> 
>>
>> If you're seeing MMC_POWER_OFF values, maybe also try changing
>> sdhci_omap_runtime_resume() to call sdhci_runtime_resume_host(host, 1)
>> and see if that helps as requesting a soft reset causes sdhci_init() to
>> set host->reinit_uhs = true.. That change feels like a workaround
>> though.
> 
> I tried anyway with soft reset, the cache flush error is gone but now I have
> this dump in dmesg each time the emmc is reset:
> 
> [ 3978.852783] mmc1: Reset 0x6 never completed.
> [ 3978.852783] mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
> [ 3978.852783] mmc1: sdhci: Sys addr:  0x00000000 | Version:  0x00003302
> [ 3978.852813] mmc1: sdhci: Blk size:  0x00000000 | Blk cnt:  0x00000000
> [ 3978.852813] mmc1: sdhci: Argument:  0x00000000 | Trn mode: 0x00000000
> [ 3978.852813] mmc1: sdhci: Present:   0x01f00000 | Host ctl: 0x00000000
> [ 3978.852813] mmc1: sdhci: Power:     0x00000000 | Blk gap:  0x00000000
> [ 3978.852813] mmc1: sdhci: Wake-up:   0x00000000 | Clock:    0x00000000
> [ 3978.852844] mmc1: sdhci: Timeout:   0x00000000 | Int stat: 0x00000000
> [ 3978.852844] mmc1: sdhci: Int enab:  0x00000000 | Sig enab: 0x00000000
> [ 3978.852844] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
> [ 3978.852844] mmc1: sdhci: Caps:      0x24e90080 | Caps_1:   0x00000f77
> [ 3978.852844] mmc1: sdhci: Cmd:       0x00000000 | Max curr: 0x00000000
> [ 3978.852874] mmc1: sdhci: Resp[0]:   0x00000000 | Resp[1]:  0x00000000
> [ 3978.852874] mmc1: sdhci: Resp[2]:   0x00000000 | Resp[3]:  0x00000000
> [ 3978.852874] mmc1: sdhci: Host ctl2: 0x00000000
> [ 3978.852874] mmc1: sdhci: ADMA Err:  0x00000000 | ADMA Ptr: 0x00000000
> [ 3978.852874] mmc1: sdhci: ============================================
> 
> With sdhci soft reset enabled, there are some MMC_POWER_OFF in dmesg
> 
> [ 3978.852905] XXX sdhci_runtime_resume_host: mmc->ios.power_mode: 0
> [ 3982.217590] XXX sdhci_runtime_resume_host: mmc->ios.power_mode: 2
> 
> The iodelay pin setting is still applied after the emmc is reset:
> 
> # omapconf dump 0x4A00348c 0x4A0034ac
> |----------------------------|
> | Address (hex) | Data (hex) |
> |----------------------------|
> | 0x4A00348C    | 0x00070101 |
> | 0x4A003490    | 0x00070101 |
> | 0x4A003494    | 0x00070101 |
> | 0x4A003498    | 0x00070101 |
> | 0x4A00349C    | 0x00060101 |
> | 0x4A0034A0    | 0x00070101 |
> | 0x4A0034A4    | 0x00070101 |
> | 0x4A0034A8    | 0x00070101 |
> | 0x4A0034AC    | 0x00070101 |
> |----------------------------|

I finally had some time to rework on this issue and it seems I found something :)

https://lore.kernel.org/linux-omap/20240315234444.816978-1-romain.naour@smile.fr/T/#u

Best regards,
Romain


> 
>>
>> Regards,
>>
>> Tony
>>
>> 8< ------
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -3848,6 +3848,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host,
> int soft_reset)
>>  	}
>>
>>  	sdhci_init(host, soft_reset);
>> +	pr_info("XXX %s: mmc->ios.power_mode: %i\n", __func__, mmc->ios.power_mode);
>>
>>  	if (mmc->ios.power_mode != MMC_POWER_UNDEFINED &&
>>  	    mmc->ios.power_mode != MMC_POWER_OFF) {
> 
> 
> Best regards,
> Romain
> 
> 
>>
>> Regards,
>>
>> Tony
> 

Re: sdhci-omap: issues with PM features since 5.16

[Tony Lindgren]

* Romain Naour <romain.naour@smile.fr> [240315 23:49]:
> I finally had some time to rework on this issue and it seems I found something :)
> 
> https://lore.kernel.org/linux-omap/20240315234444.816978-1-romain.naour@smile.fr/T/#u

Great good to hear you got it sorted out :)

Tony