Re: [PATCH] mmc: tmio: Fixup runtime PM management during probe and remove

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

 



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.

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 ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[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