On Mon, 18 May 2020 at 23:07, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Ulf,
>
> 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.
> 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.
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.
>
> 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?
[...]
Kind regards
Uffe
