Hi Matthias, On 19. 2. 15. 오전 4:28, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Thu, Feb 14, 2019 at 11:17:36PM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> As I commented on the first patch, it is not possible to call some codes >> according to the intention of each governor between 'devfreq_moniotr_*()' >> and some codes which are executed before or after 'devfreq_moniotr_*()' >> >> For example, if some governor requires the following sequence, >> after this patch, it is not possible. >> >> case DEVFREQ_GOV_xxx: >> /* execute some code before devfreq_monitor_xxx() */ >> devfreq_monitor_xxx() >> /* execute some code after devfreq_monitor_xxx() */ > > As for the suspend/resume case I agree that the patch introduces this > limitation, but I'm not convinced that this is an actual problem. > > For governor_start(): why can't the governor execute the code > before polling started, does it make any difference to the governor > that a work is scheduled? The some governors might want to do something before starting the work and do something after work started. I think that the existing style is more flexible. And, It has one more problem when changing the governor on the fly from simple_ondemand to other governors like performance, powersave and so on. Even if other governors don't need to monitor the utilization, the work timer will be executed continually because the devfreq device has the polling_ms value. It is not necessary of the other governors such as performance, powersave. It means that only specific governor like simple_ondemand have to call the devfreq_monitor_start() by itself instead of calling it on devfreq core. > > For governor_stop(): why would the governor require polling to be > active during stop? If it needs update_devfreq() to run (called by > devfreq_monitor()) it can call it directly, instead of waiting for the > monitor to run at some later time. As I knew, if the current governors are performance/powersave/ userspace, the monitoring is already stopped and not used. Because they don't need to handle the codes related to the work like queue_delayed_work(), cancel_delayed_work_sync(). And, In case of the existing style for calling devfreq_monitor_*(), other governors like performance/powersave/userspace/passice don't need to call the devfreq_monitor_stop() because they didn't use the work timer. > > Cheers > > Matthias > > > > > -- Best Regards, Chanwoo Choi Samsung Electronics
