On Thursday 04 December 2014 09:28:34 Lina Iyer wrote: > On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote: > >On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote: > >> On 12/03/2014 09:35 PM, Arnd Bergmann wrote: > >> > On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote: > >> >>>>> +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. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html