On Fri, 13 Sep 2019 at 12:26, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > Hi Ulf, > > On Fri, Sep 13, 2019 at 11:44 AM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > On Fri, 13 Sep 2019 at 11:38, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > > On Fri, Sep 13, 2019 at 11:32 AM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > > > On Fri, 13 Sep 2019 at 09:44, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > > > > On Fri, Sep 13, 2019 at 9:41 AM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > > > > > On Fri, 13 Sep 2019 at 08:37, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > > > > > > On Thu, Sep 12, 2019 at 10:04 PM Niklas Soderlund > > > > > > > <niklas.soderlund@xxxxxxxxxxxx> wrote: > > > > > > > > On 2019-09-12 19:05:47 +0100, Wolfram Sang wrote: > > > > > > > > > On Wed, Sep 11, 2019 at 04:16:56PM +0200, Ulf Hansson wrote: > > > > > > > > > > On Mon, 9 Sep 2019 at 12:46, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > > > > > > > During probe, tmio variant drivers calls pm_runtime_enable() before they > > > > > > > > > > > call tmio_mmc_host_probe(). This doesn't work as expected, because > > > > > > > > > > > tmio_mmc_host_probe() calls pm_runtime_set_active(), which fails to set the > > > > > > > > > > > status to RPM_ACTIVE for the device, when its been enabled for runtime PM. > > > > > > > > > > > > > > > > > > > > > > Fix this by calling pm_runtime_enable() from tmio_mmc_host_probe() instead. > > > > > > > > > > > To avoid the device from being runtime suspended during the probe phase, > > > > > > > > > > > let's also increase the runtime PM usage count in tmio_mmc_host_probe(). > > > > > > > > > > > Consequentially, each tmio variant driver can decide themselves when to > > > > > > > > > > > call pm_runtime_put(), to allow the device to become runtime suspended. > > > > > > > > > > > > > > > > > > > > > > Additionally, if the tmio variant driver decided to call pm_runtime_put() > > > > > > > > > > > during probe, it's is expected that it also calls pm_runtime_get_sync() to > > > > > > > > > > > restore the usage count, before it calls tmio_mmc_host_remove(). > > > > > > > > > > > > > > > > > > > > > > Fixes: 7ff213193310 ("mmc: tmio: move runtime PM enablement to the driver implementations") > > > > > > > > > > > Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> > > > > > > > > > > > > > > > > > > > > So I decided to apply this for my fixes branch, as to get it tested > > > > > > > > > > for a few days. > > > > > > > > > > > > > > > > > > > > If you have any objections, please tell. > > > > > > > > > > > > > > > > > > Sadly, I can't test until next week because I am still on the road. Yet, > > > > > > > > > I recall Niklas said at LPC that the patch looks good to him, at least. > > > > > > > > > > > > > > > > > > > > > > > > > Yes I think it looks good and was planing to test it. Unfortunately I'm > > > > > > > > also on the road until the end of next week ;-( > > > > > > > > > > > > > > So I decided to give it a try on my boards. Note that apart from eMMC, > > > > > > > I do not have any SD cards inserted. > > > > > > > > > > > > Thanks for testing! > > > > > > > > > > [...] > > > > > > > > > > > Let's have a look at that in the next steps though and fix the probe > > > > > > problems first. I can post a patch or two in an hour or so, have you > > > > > > the possibility to test this today? > > > > > > > > > > Probably I can. Else it'll happen on Monday. > > > > > > > > Currently compile testing, posting three patch in few minutes. > > > > > > > > If we can't make it today, we will probably don't make it for 5.3. On > > > > the other hand, the problem has been there for a while anyway. > > > > > > Oh, I thought you were aiming for v5.4... > > > > > > Including it in v5.3 may be a stretch, as Wolfram and Niklas can't test it > > > before the release of v5.3. > > > > The point is, v5.3 is broken anyway and older kernels as well. > > > > Anyway, the patches is out, let's see what you find out. > > Thanks! > > I've reverted this one, and applied: > > [PATCH 1/3] Revert "mmc: tmio: move runtime PM enablement to the > [PATCH 2/3] mmc: tmio: Fixup runtime PM management during probe > [PATCH 3/3] mmc: tmio: Fixup runtime PM management during remove > > All SDHI devices are properly started again. > > After boot, clock use counts are now consistently at 2, for all boards, > i.e.: > > -clk_summary: sdhi0 3 3 0 > 97500000 0 0 50000 > +clk_summary: sdhi0 2 2 0 > 97500000 0 0 50000 > > After s2ram, clock use counts are the same as before s2ram, so no > imbalance detected. Good. Great, thanks for testing! > > S2ram still cause regulator imbalances, but that's not genpd related. > > -regulator_summary: fixed-1.8V 2 1 0 > unknown 1800mV 0mA 1800mV 1800mV > -regulator_summary: ee140000.sd 1 > 0mA 1800mV 1950mV > -regulator_summary: fixed-3.3V 2 1 0 > unknown 3300mV 0mA 3300mV 3300mV > -regulator_summary: ee140000.sd 1 > 0mA 3300mV 3400mV > +regulator_summary: fixed-1.8V 1 1 0 > unknown 1800mV 0mA 1800mV 1800mV > +regulator_summary: ee140000.sd 0 > 0mA 1800mV 1950mV > +regulator_summary: fixed-3.3V 1 1 0 > unknown 3300mV 0mA 3300mV 3300mV > +regulator_summary: ee140000.sd 0 > 0mA 3300mV 3400mV > > No change in use counts after subsequent s2ram cycles. > > And eMMC on Salvator-XS is still working. > > A good state to enter the weekend ;-) Perfect! I am queuing up the three patches for 5.3. Yes, it's a bit risky, but I am quite confident that we have improved things. Perhaps there are additional changes needed, at least we need to look into the regulator imbalances at some point. Thanks and have a nice weekend! Kind regards Uffe