Hi Ulf,
On Tue, May 19, 2020 at 10:19 AM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On Mon, 18 May 2020 at 23:07, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > On Fri, May 15, 2020 at 4:05 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> > > If the tmio device is attached to a genpd (PM domain), that genpd may have
> > > ->start|stop() callback assigned to it. To make sure the device is
> > > accessible during ->probe(), genpd's ->start() callback must be invoked,
> > > which is currently managed by tmio_mmc_host_probe(). This is very likely to
> > > be too late for some cases, as registers may be read and written way before
> > > that.
> > >
> > > To fix this behaviour, let's drop the call to dev_pm_domain_start() from
> > > tmio_mmc_host_probe() - and let the tmio clients manage this instead.
> > >
> > > Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> >
> > So this moves the call to dev_pm_domain_start(). No new calls are added.
> > If I get it right, dev_pm_domain_start() just calls into
> > genpd_dev_pm_start() through a function pointer, and starts the device
> > through the system-specific PM Domain handler. On R-Car SoCs, that
> > is pm_clk_resume(), i.e. enabling the module clock through the clock
> > domain.
>
> Correct.
>
> > I have two questions there:
> > 1. What if the device is already started?
> > There seems to be no reference counting involved.
>
> The device can't be started as runtime PM hasn't been enabled yet for
> the device - and this is controlled by the tmio/renesas driver.
OK.
> > 2. Who stops the device again?
>
> Beyond this point there are two main cases to consider.
>
> 1. The driver is probed successfully and thus up and running. Then,
> when the device becomes runtime suspended (because of request
> inactivity, for example), the device will be "stopped" through genpd.
OK.
> 2. The driver failed to probe or the ->remove() callback is invoked
> for the device. This will trigger the platform bus to call
> dev_pm_domain_detach(). In this path, genpd invokes the
> genpd->detach_dev() callback for the device, which allows the genpd
> provider to deal with the clean up. In this particular case, I assume
> pm_clk_destroy() is going to be called for the device.
OK.
> > I always thought the PM Domain was powered on (if still off), and the
> > device started, by calling pm_runtime_get_sync().
>
> Correct.
>
> However, deploying that kind of pattern in a driver can be a bit
> messy, while considering that CONFIG_PM may be set or unset and the
> driver should work in both configurations. In principle, it leads to
> boilerplate code in drivers, especially if the driver has runtime PM
> callbacks assigned to it as shown in commit 1b32999e205b ("mmc: tmio:
> Avoid boilerplate code in ->runtime_suspend()").
>
> Does it make sense?
Now it does, thanks!
Note that for devices handled by renesas_sdhi, CONFIG_PM is always
set (cfr. drivers/soc/renesas/Kconfig), and there will always by a genpd.
So the "messy" part matters for TMIO and UniPhier only.
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
