Hi Tomasz,

Am Sonntag, den 23.02.2014, 18:07 +0100 schrieb Tomasz Figa:

Hi Philipp,

On 19.02.2014 17:53, Philipp Zabel wrote:

quoted

Am Samstag, den 11.01.2014, 20:42 +0100 schrieb Tomasz Figa:

[snip]

quoted

quoted

+	pd = of_genpd_get_from_provider(&pd_args);
+	if (IS_ERR(pd))
+		return PTR_ERR(pd);
+
+	dev_dbg(dev, "adding to power domain %s\n", pd->name);
+
+	while (1) {
+		ret = pm_genpd_add_device(pd, dev);

Since pm_genpd_add_device is used here, no gpd_timing_data can be
provided. Do you have a plan to solve this? Should the timing data be
provided from the device tree?

Hmm, a quick grep over kernel sources for genpd_.*_add_device
gives just a single user of __pm_genpd_name_add_device(), with custom 
timing data:

I had added this to my work progress i.MX patches to silence the noisy
"... latency exceeded, new value ..." warnings emitted by the power
domain framework: http://patchwork.ozlabs.org/patch/320084/

[...]

Moreover the timings used there are just defaults, which makes me wonder 
if there is any reason to specify them explicitly. Even more interesting 
is the fact that genpd code can measure those latencies itself.

Do you have a particular use case for those timing data or just 
wondering? I don't think we need to implement support for them right 
away, if there is no real need to do so. The code and bindings can be 
extended later to handle them, if needed.

You are right, this is just superficial.

As for whether DT is appropriate place to define them, I'm not quite 
sure. Stop and start latencies look like hardware parameters, but state 
save and restore are likely to be driver-specific, as it depends on 
driver code how much time it takes to save and restore needed state 
(e.g. driver with register cache will not need to do any state save), if 
I understand these timing data correctly.

I have one more, on i.MX6 I manually need to enable the clocks of
devices in the power domain during the power-up sequence so that the
reset signals can propagate.
So far, I have implemented this by registering the device clocks of
devices in the power domain with pm_clk_add and then let the genpd
power_on callback temporarily enable them using pm_clk_resume:

http://patchwork.ozlabs.org/patch/320085/

Whether this is needed seems to me to be a property of the power domain.
Do you think this is something we could add to of_genpd_add_to_domain,
depending on some flag set in struct generic_pm_domain?
I'd like to avoid having to register my own bus notifier and to deal
with ordering issues between that and of_genpd_notifier_call.

regards
Philipp