Thread (29 messages) 29 messages, 5 authors, 2014-12-17

[PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver

From: Lina Iyer <hidden>
Date: 2014-12-05 15:45:26
Also in: linux-arm-msm, linux-devicetree, linux-pm

On Thu, Dec 04 2014 at 11:20 -0700, Arnd Bergmann wrote:
On Thursday 04 December 2014 09:28:34 Lina Iyer wrote:
quoted
On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote:
quoted
On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
quoted
On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
quoted
On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
quoted
quoted
quoted
quoted
+static int __init qcom_spm_init(void)
+{
+    int ret;
+
+    /*
+     * cpuidle driver need to registered before the cpuidle device
+     * for any cpu. Register the device for the the cpuidle driver.
+     */
+    ret = platform_device_register(&qcom_cpuidle_drv);
+    if (ret)
+        return ret;
Stephen pointed out that we would have the platform device lying around
on a non-QCOM device when using multi_v7_defconfig.
Perhaps I am missing the point, but this is not supposed to happen, no ?
This would happen, since the file would compile on multi_v7 and we would
initialize and register this device regardless. The cpuidle-qcom.c
driver probe would bail out looking for a matching compatible property.
So we would not register a cpuidle driver but the device would lay
around.
I think the problem is registering a platform_device. I've complained
about this before, but it still seems to get copied all over the
place. Please don't do this but have a driver that looks at DT to
figure out whether to access hardware or not.
We did this approach but, I can remember why, someone was complaining
about it also

The platform device/driver paradigm allowed us to split the arch
specific parts by passing the pm ops through the platform data.

Would make sense to have a single common place for the ARM arch where we
initialize the platform device for cpuidle ?
No. It's really not a device, and if you pretend that it is, you get
into problems like this.
Arnd, the problem is that the provides function pointers to the SoC code
that the cpuilde driver uses to call based on the idle state.

After much discussions, we came down to using function pointers from
translating from DT strings, other than using that again, I dont see a
good way of achieving the ability of cpuidle driver staying a separate
driver from the SPM driver.

Daniel, thoughts?
Maybe the problem is trying too hard to separate two things that really
belong together then. In general, the strategy of coming up with subsystems
for a class of devices and them turning platform code into drivers for
that subsystem has worked out really well, but I think for cpufreq, cpuidle
and smp, it really hasn't, and in the third case we haven't even tried
coming up with a subsystem for that reason.

Having all cpuidle code generally live in drivers/cpuidle is still a good
idea IMO, but using a platform device just for the purpose of passing
data between some platform specific code and another platform specific
driver hasn't worked out that well here.
To achieve both, I cant think of a better way to initialize the cpuidle
driver without the use of reference count (0 ==>
platform_driver_register).

I tried creating dummy platform device in the DT but something or
another gives in to an ugly implementation.

Any other ideas?

Lina.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help