On 4 December 2013 17:38, Bjørn Mork <bjorn@xxxxxxx> wrote:
> Yes, that patch looks like it fix the problem, and I cannot spot any
> obvious missing cleanups.
Great!!
> But I must say that the whole kobj + free thing makes me feel a bit
> uneasy. I assume there are good reasons why "cpufreq" isn't just a
> device with a release method?
I didn't get it completely.. How exactly you wanted it to be..
> And I do hope there is something gained by the special suspend handling,
> because keeping this partial device looks really messy. If it's all just
> for some file permissions, then there must be better ways?
We didn't found many and so Srivatsa went with this way.. If you have a
better way then we are all for it :)
> Isn't consistent file permissions on device creation really a userspace issue?
Yes, normally it is.. But suspend/resume or hibernation isn't a userspace
problem at all. We had a discussion earlier on it and realized that kernel
is screwing it up and so it should get it back. Userspace doesn't have to
know how cpufreq removed these directories/files and added them back
on resume.
> Anyway, the patch works so you can add
>
> Tested-by: Bjørn Mork <bjorn@xxxxxxx>
Great. Rafael can take this with a revert of your patch now..
> BTW, a simple way to test these things is cancelling a userspace
> hibernate on an ACPI system. It will always make the acpi_cpufreq
> driver fail because it attemps to call _PPC inbetween
> acpi_ec_block_transactions and acpi_ec_unblock_transactions. There is
> no acpi_ec_unblock_transactions_early call in this case.
I did test it on my thinkpad with following hack:
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 57c80a7..ebaec3d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1046,6 +1046,10 @@ static int __cpufreq_add_dev(struct device
*dev, struct subsys_interface *sif,
/* call driver. From then on the cpufreq must be able
* to accept all calls to ->verify and ->setpolicy for this CPU
*/
+ if (frozen) {
+ ret = -EBUSY;
+ goto err_set_policy_cpu;
+ }
+
ret = cpufreq_driver->init(policy);
if (ret) {
pr_debug("initialization failed\n");
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
