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@xxxxxxxx> [240201 09:04]:
>>> Le 31/01/2024 à 11:30, Tony Lindgren a écrit :
>>>> * Romain Naour <romain.naour@xxxxxxxx> [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@xxxxxxxxxxx> [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@xxxxxxxx/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
>
