| Message ID | c60806d5480b7311ced8db07ff239aa5af7a050d.1573702497.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | cpufreq: Register cpufreq drivers only after CPU devices are registered | expand |
On Thu, Nov 14, 2019 at 9:06 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > The cpufreq core heavily depends on the availability of the struct > device for CPUs and if they aren't available at the time cpufreq driver > is registered, we will never succeed in making cpufreq work. > > This happens due to following sequence of events: > > - cpufreq_register_driver() > - subsys_interface_register() > - return 0; //successful registration of driver > > ... at a later point of time > > - register_cpu(); > - device_register(); > - bus_probe_device(); > - sif->add_dev(); > - cpufreq_add_dev(); > - get_cpu_device(); //FAILS > - per_cpu(cpu_sys_devices, num) = &cpu->dev; //used by get_cpu_device() > - return 0; //CPU registered successfully > > Because the per-cpu variable cpu_sys_devices is set only after the CPU > device is regsitered, cpufreq will never be able to get it when > cpufreq_add_dev() is called. > > This patch avoids this failure by making sure device structure of at > least CPU0 is available when the cpufreq driver is registered, else > return -EPROBE_DEFER. > > Reported-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> Tested-by: Amit Kucheria <amit.kucheria@linaro.org> > --- > @Amit: I have added your sob without asking as you were involved in > getting us to this patch, you did a lot of testing yesterday to find the > root cause. SoB for the SoB :-) > @Rafael: This fixes the issues reported by Bjorn on Amit's series and so > should land before Amit's series, if at all this is acceptable to you. > Thanks. My series was going to get merged through the thermal tree. Currently it is hosted here[1] for linux-next testing. We could sign-off this patch to the thermal tree or bring the series into the PM tree. Up to Rafael and you. [1] https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/log/?h=thermal/linux-next > drivers/cpufreq/cpufreq.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 681c1b5f0a1a..05293b43e56d 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -2641,6 +2641,13 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data) > if (cpufreq_disabled()) > return -ENODEV; > > + /* > + * The cpufreq core depends heavily on the availability of device > + * structure, make sure they are available before proceeding further. > + */ > + if (!get_cpu_device(0)) > + return -EPROBE_DEFER; > + > if (!driver_data || !driver_data->verify || !driver_data->init || > !(driver_data->setpolicy || driver_data->target_index || > driver_data->target) || > -- > 2.21.0.rc0.269.g1a574e7a288b >
On 14-11-19, 12:33, Amit Kucheria wrote: > My series was going to get merged through the thermal tree. Currently > it is hosted here[1] for linux-next testing. We could sign-off this > patch to the thermal tree or bring the series into the PM tree. Up to > Rafael and you. Your thermal patches need to get based of this patch in that case. Maybe just ask Rui to drop your last patch (for qcom-cpufreq-hw.c) and merge things as planned. After rc1 is out, you can ask Rafael to merge that one on top of rc1 ? Everything can still go in 5.5 that way.
On Thu, Nov 14, 2019 at 4:36 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > The cpufreq core heavily depends on the availability of the struct > device for CPUs and if they aren't available at the time cpufreq driver > is registered, we will never succeed in making cpufreq work. > > This happens due to following sequence of events: > > - cpufreq_register_driver() > - subsys_interface_register() > - return 0; //successful registration of driver > > ... at a later point of time > > - register_cpu(); > - device_register(); > - bus_probe_device(); > - sif->add_dev(); > - cpufreq_add_dev(); > - get_cpu_device(); //FAILS > - per_cpu(cpu_sys_devices, num) = &cpu->dev; //used by get_cpu_device() > - return 0; //CPU registered successfully > > Because the per-cpu variable cpu_sys_devices is set only after the CPU > device is regsitered, cpufreq will never be able to get it when > cpufreq_add_dev() is called. > > This patch avoids this failure by making sure device structure of at > least CPU0 is available when the cpufreq driver is registered, else > return -EPROBE_DEFER. > > Reported-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > @Amit: I have added your sob without asking as you were involved in > getting us to this patch, you did a lot of testing yesterday to find the > root cause. Co-developed-by is for that, so I used it in the commit. > @Rafael: This fixes the issues reported by Bjorn on Amit's series and so > should land before Amit's series, if at all this is acceptable to you. Well, yes it is. Applied for 5.5 with a reworded subject, thanks!
On 14-11-19, 09:40, Rafael J. Wysocki wrote: > On Thu, Nov 14, 2019 at 4:36 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > The cpufreq core heavily depends on the availability of the struct > > device for CPUs and if they aren't available at the time cpufreq driver > > is registered, we will never succeed in making cpufreq work. > > > > This happens due to following sequence of events: > > > > - cpufreq_register_driver() > > - subsys_interface_register() > > - return 0; //successful registration of driver > > > > ... at a later point of time > > > > - register_cpu(); > > - device_register(); > > - bus_probe_device(); > > - sif->add_dev(); > > - cpufreq_add_dev(); > > - get_cpu_device(); //FAILS > > - per_cpu(cpu_sys_devices, num) = &cpu->dev; //used by get_cpu_device() > > - return 0; //CPU registered successfully > > > > Because the per-cpu variable cpu_sys_devices is set only after the CPU > > device is regsitered, cpufreq will never be able to get it when > > cpufreq_add_dev() is called. > > > > This patch avoids this failure by making sure device structure of at > > least CPU0 is available when the cpufreq driver is registered, else > > return -EPROBE_DEFER. > > > > Reported-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > @Amit: I have added your sob without asking as you were involved in > > getting us to this patch, you did a lot of testing yesterday to find the > > root cause. > > Co-developed-by is for that, so I used it in the commit. > > > @Rafael: This fixes the issues reported by Bjorn on Amit's series and so > > should land before Amit's series, if at all this is acceptable to you. > > Well, yes it is. > > Applied for 5.5 with a reworded subject, thanks! Thanks.
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 681c1b5f0a1a..05293b43e56d 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2641,6 +2641,13 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data) if (cpufreq_disabled()) return -ENODEV; + /* + * The cpufreq core depends heavily on the availability of device + * structure, make sure they are available before proceeding further. + */ + if (!get_cpu_device(0)) + return -EPROBE_DEFER; + if (!driver_data || !driver_data->verify || !driver_data->init || !(driver_data->setpolicy || driver_data->target_index || driver_data->target) ||
The cpufreq core heavily depends on the availability of the struct device for CPUs and if they aren't available at the time cpufreq driver is registered, we will never succeed in making cpufreq work. This happens due to following sequence of events: - cpufreq_register_driver() - subsys_interface_register() - return 0; //successful registration of driver ... at a later point of time - register_cpu(); - device_register(); - bus_probe_device(); - sif->add_dev(); - cpufreq_add_dev(); - get_cpu_device(); //FAILS - per_cpu(cpu_sys_devices, num) = &cpu->dev; //used by get_cpu_device() - return 0; //CPU registered successfully Because the per-cpu variable cpu_sys_devices is set only after the CPU device is regsitered, cpufreq will never be able to get it when cpufreq_add_dev() is called. This patch avoids this failure by making sure device structure of at least CPU0 is available when the cpufreq driver is registered, else return -EPROBE_DEFER. Reported-by: Bjorn Andersson <bjorn.andersson@linaro.org> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- @Amit: I have added your sob without asking as you were involved in getting us to this patch, you did a lot of testing yesterday to find the root cause. @Rafael: This fixes the issues reported by Bjorn on Amit's series and so should land before Amit's series, if at all this is acceptable to you. Thanks. drivers/cpufreq/cpufreq.c | 7 +++++++ 1 file changed, 7 insertions(+)