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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 





[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux