Messages in this thread | | | Subject | Re: [PATCH v4] PM / devfreq: Restart previous governor if new governor fails to start | From | Sibi Sankar <> | Date | Tue, 19 Feb 2019 10:42:01 +0530 |
| |
Hey MyungJoo,
On 12/14/18 7:15 AM, MyungJoo Ham wrote: >> From: Saravana Kannan <skannan@codeaurora.org> >> >> If the new governor fails to start, switch back to old governor so that the >> devfreq state is not left in some weird limbo. >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> Signed-off-by: Saravana Kannan <skannan@codeaurora.org> >> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> > > Hello, > > In overall, the idea and the implementation looks good. > > However, I have a question: > > What if the following line fails? > > + df->governor->event_handler(df, DEVFREQ_GOV_START, > + NULL); > > Don't we still need something to handle for such events?
The original discussion went as follows: governor_store is expected to be used only on cases where devfreq_add_device() succeeded i.e prev->governor is expected to be present and DEVFREQ_GOV_START is expected to succeed. Hence falling back to the previous governor seems like a sensible idea.
This would also prevent DEVFREQ_GOV_STOP from being called on a governor were DEVFREQ_GOV_START had failed which is ideal.
That being said DEVFREQ_GOV_START can still fail for the prev-governor due to some change in state of the system. Do you want to handle this case by clearing the state of governor rather than switching to previous governor?
> > Cheers, > MyungJoo > >
-- Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
| |