[PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
Russell King - ARM Linux
linux at arm.linux.org.uk
Fri Feb 13 06:11:52 PST 2015
On Fri, Feb 13, 2015 at 01:29:25PM +0000, Russell King - ARM Linux wrote:
> On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote:
> > On 27 April 2014 15:29, Russell King <rmk+kernel at arm.linux.org.uk> wrote:
> > > The PMU device contains an interrupt controller, power control and
> > > resets. The interrupt controller is a little sub-standard in that
> > > there is no race free way to clear down pending interrupts, so we try
> > > to avoid problems by reducing the window as much as possible, and
> > > clearing as infrequently as possible.
> > >
> > > The interrupt support is implemented using an IRQ domain, and the
> > > parent interrupt referenced in the standard DT way.
> > >
> > > The power domains and reset support is closely related - there is a
> > > defined sequence for powering down a domain which is tightly coupled
> > > with asserting the reset. Hence, it makes sense to group these two
> > > together.
> > >
> > > This patch adds the core PMU driver: power domains must be defined in
> > > the DT file in order to make use of them. The reset controller can
> > > be referenced in the standard way for reset controllers.
> >
> > Hi Russell,
> >
> > This patch would be simplified if this was based upon the not yet
> > merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree
> > based power domain look-up".
> >
> > For example you would likely not need to add some of the marvel
> > specific DT bindings, and you wouldn’t need the bus_notifiers to add
> > devices to the power domain. I guess I just though it could be useful
> > input to consider while going forward, unless you already knew.
>
> In 3.19, I notice something of an odd behaviour.
>
> My vMeta driver has runtime PM support enabled. When I explicitly register
> the PM domain in the pmu driver via a bus notifier, I see:
>
> root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> domain status slaves
> /device runtime status
> ----------------------------------------------------------------------
> gpu-domain on
> /devices/platform/vivante/etnaviv-gpu,2d active
> vpu-domain off
> /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended
>
> But when I disable that, and let the generic code do the registration,
> I instead get:
>
> root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> domain status slaves
> /device runtime status
> ----------------------------------------------------------------------
> gpu-domain on
> /devices/platform/vivante/etnaviv-gpu,2d active
> vpu-domain on
> /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended
>
> The difference being that the vpu domain remains powered.
>
> The only difference code-wise seems to be when genpd_dev_pm_attach() is
> called. In the working case, it's before the device is considered for
> probing. In the non-working case, it's just before the device is probed.
>
> With debugging enabled in the PM domain code, with the former case I get:
>
> Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain
> platform f1c00000.video-decoder: adding to PM domain vpu-domain
> platform f1c00000.video-decoder: __pm_genpd_add_device()
>
> With the latter non-working case:
>
> Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain
> ...
> ap510-vmeta f1c00000.video-decoder: adding to PM domain vpu-domain
> ap510-vmeta f1c00000.video-decoder: __pm_genpd_add_device()
> vpu-domain: Power-on latency exceeded, new value 1578 ns
>
> Neither of these debug messages provide much hint as to what the
> difference is, or the cause of the PM domain code being de-sync'd
> with its devices.
>
> Maybe the PM code needs more debugging in it, and maybe the debugfs
> file should always be present if debugfs support is enabled?
The vmeta driver does this in its probe function:
pm_runtime_use_autosuspend(vi->dev);
pm_runtime_set_autosuspend_delay(vi->dev, 100);
pm_runtime_enable(vi->dev);
since it doesn't touch the hardware, and the hardware starts off at
boot time in "suspended" mode.
I think what's going on is that there's a difference in the expectations
from the PM domain code vs the runtime PM code. I refer to section 5
of the runtime PM documentation:
| 5. Runtime PM Initialization, Device Probing and Removal
|
| Initially, the runtime PM is disabled for all devices, which means that the
| majority of the runtime PM helper functions described in Section 4 will return
| -EAGAIN until pm_runtime_enable() is called for the device.
|
| In addition to that, the initial runtime PM status of all devices is
| 'suspended', but it need not reflect the actual physical state of the device.
| Thus, if the device is initially active (i.e. it is able to process I/O), its
| runtime PM status must be changed to 'active', with the help of
| pm_runtime_set_active(), before pm_runtime_enable() is called for the device.
However, the PM domain code seems to always power up the PM domain when
a device is attached to it:
int genpd_dev_pm_attach(struct device *dev)
{
...
pm_genpd_poweron(pd);
return 0;
}
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
So, the PM domain code ends up disagreeing with the runtime PM code about
the state of the device.
I think your commit (2ed127697eb1 "PM / Domains: Power on the PM domain
right after attach completes") is fundamentally wrong. The assertion
you make in there is built upon the assumption that every driver will
call pm_runtime_set_active(), which is not an assumption you can make.
Instead, you should be doing is to hook into __pm_runtime_set_status()
and use that to trigger the PM domain power up so that the runtime PM
and PM domain state is always in step with each other.
What I'm certain of is that the current situation is just totally crazy.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
More information about the linux-arm-kernel
mailing list