linux-kernel - Re: [PATCH RFC 04/10] base: power: Add generic OF-based power domain look-up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 23 Feb 2014 18:07:43 +0100
From:	Tomasz Figa <tomasz.figa@...il.com>
To:	Philipp Zabel <p.zabel@...gutronix.de>
CC:	linux-pm@...r.kernel.org, Mark Rutland <mark.rutland@....com>,
	devicetree@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
	Russell King <linux@....linux.org.uk>,
	Pawel Moll <pawel.moll@....com>,
	Len Brown <len.brown@...el.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Tomasz Figa <t.figa@...sung.com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	linux-kernel@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
	Kukjin Kim <kgene.kim@...sung.com>,
	Pavel Machek <pavel@....cz>, Kumar Gala <galak@...eaurora.org>,
	Stephen Warren <swarren@...dotorg.org>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH RFC 04/10] base: power: Add generic OF-based power domain
 look-up

Hi Philipp,

On 19.02.2014 17:53, Philipp Zabel wrote:
> Am Samstag, den 11.01.2014, 20:42 +0100 schrieb Tomasz Figa:

[snip]

>> +	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:

> arch/arm/mach-shmobile/pm-rmobile.c-void rmobile_add_device_to_domain_td(const char *domain_name,
> arch/arm/mach-shmobile/pm-rmobile.c-                                 struct platform_device *pdev,
> arch/arm/mach-shmobile/pm-rmobile.c-                                 struct gpd_timing_data *td)
> arch/arm/mach-shmobile/pm-rmobile.c-{
> arch/arm/mach-shmobile/pm-rmobile.c-    struct device *dev = &pdev->dev;
> arch/arm/mach-shmobile/pm-rmobile.c-
> arch/arm/mach-shmobile/pm-rmobile.c:    __pm_genpd_name_add_device(domain_name, dev, td);
> arch/arm/mach-shmobile/pm-rmobile.c-    if (pm_clk_no_clocks(dev))
> arch/arm/mach-shmobile/pm-rmobile.c-            pm_clk_add(dev, NULL);
> arch/arm/mach-shmobile/pm-rmobile.c-}
> arch/arm/mach-shmobile/pm-rmobile.c-
> arch/arm/mach-shmobile/pm-rmobile.c-void rmobile_add_devices_to_domains(struct pm_domain_device data[],
> arch/arm/mach-shmobile/pm-rmobile.c-                                int size)
> arch/arm/mach-shmobile/pm-rmobile.c-{
> arch/arm/mach-shmobile/pm-rmobile.c-    struct gpd_timing_data latencies = {
> arch/arm/mach-shmobile/pm-rmobile.c-            .stop_latency_ns = DEFAULT_DEV_LATENCY_NS,
> arch/arm/mach-shmobile/pm-rmobile.c-            .start_latency_ns = DEFAULT_DEV_LATENCY_NS,
> arch/arm/mach-shmobile/pm-rmobile.c-            .save_state_latency_ns = DEFAULT_DEV_LATENCY_NS,
> arch/arm/mach-shmobile/pm-rmobile.c-            .restore_state_latency_ns = DEFAULT_DEV_LATENCY_NS,
> arch/arm/mach-shmobile/pm-rmobile.c-    };
> arch/arm/mach-shmobile/pm-rmobile.c-    int j;
> arch/arm/mach-shmobile/pm-rmobile.c-
> arch/arm/mach-shmobile/pm-rmobile.c-    for (j = 0; j < size; j++)
> arch/arm/mach-shmobile/pm-rmobile.c-            rmobile_add_device_to_domain_td(data[j].domain_name,
> arch/arm/mach-shmobile/pm-rmobile.c-                                            data[j].pdev, &latencies);
> arch/arm/mach-shmobile/pm-rmobile.c-}

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.

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.

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/