On Thu, Feb 8, 2024 at 5:32 AM Romain Naour <romain.naour@xxxxxxxx> wrote:
>
> 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.
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@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 |
> |----------------------------|
>
> >
> > 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
>
>
