>Hey MyungJoo, Kyungmin >Did you get a chance to think about how you >want this fix implemented? > >On 2019-02-19 10:42, Sibi Sankar wrote: >> Hey MyungJoo, >> >> On 12/14/18 7:15 AM, MyungJoo Ham wrote: >>>> From: Saravana Kannan <skan...@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 <si...@codeaurora.org> >>>> Signed-off-by: Saravana Kannan <skan...@codeaurora.org> >>>> Reviewed-by: Chanwoo Choi <cw00.c...@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? >>
If moving back to previous governor fails after failing for "next" governor, we may assume it's fatal and stop the device; we can simply return errors. In such a case, df->governor may need to be NULL as well. Cheers, MyungJoo
