Hi Enric, On 2018년 07월 04일 17:16, Enric Balletbo i Serra wrote: > Hi Chanwoo, > > On 04/07/18 03:06, Chanwoo Choi wrote: >> Hi Enric, >> >> On 2018년 07월 03일 22:29, Enric Balletbo i Serra wrote: >>> When the devfreq driver and the governor driver are built as modules, >>> the call to devfreq_add_device() or governor_store() fails because the >>> governor driver is not loaded at the time the devfreq driver loads. The >>> devfreq driver has a build dependency on the governor but also should >>> have a runtime dependency. We need to make sure that the governor driver >>> is loaded before the devfreq driver. >>> >>> This patch fixes this bug by adding a try_then_request_governor() >>> function. First tries to find the governor, and then, if it is not found, >>> it requests the module and tries again. >>> >>> Fixes: 1b5c1be2c88e (PM / devfreq: map devfreq drivers to governor using >>> name) >>> Signed-off-by: Enric Balletbo i Serra <[email protected]> >>> --- >>> >>> Changes in v4: >>> - Kept "locked" devfreq_list from the return of find_devfreq_governor() to >>> the unlock of governor_store(). Requested by MyungJoo Ham. >>> >>> Changes in v3: >>> - Remove unneded change in dev_err message. >>> - Fix err returned value in case to not find the governor. >>> >>> Changes in v2: >>> - Add a new function to request the module and call that function from >>> devfreq_add_device and governor_store. >>> >>> drivers/devfreq/devfreq.c | 53 ++++++++++++++++++++++++++++++++++----- >>> 1 file changed, 47 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>> index 0b5b3abe054e..4ea6b19879a1 100644 >>> --- a/drivers/devfreq/devfreq.c >>> +++ b/drivers/devfreq/devfreq.c >>> @@ -11,6 +11,7 @@ >>> */ >>> >>> #include <linux/kernel.h> >>> +#include <linux/kmod.h> >>> #include <linux/sched.h> >>> #include <linux/errno.h> >>> #include <linux/err.h> >>> @@ -221,6 +222,46 @@ static struct devfreq_governor >>> *find_devfreq_governor(const char *name) >>> return ERR_PTR(-ENODEV); >>> } >>> >>> +/** >>> + * try_then_request_governor() - Try to find the governor and request the >>> + * module if is not found. >>> + * @name: name of the governor >> >> Usually, devfreq used 'governor_name' indicating the name of governor. >> you better to use 'governor_name' instead of 'name' for more readability. >> > > I don't mind to change if you want. But let me try to convince you first. I > used > name for two reasons: > 1. I saw that you are using governor_name sometimes, but find_devfreq_governor > uses name not governor_name. IMHO the function name in these two specific > cases > 'try_then_request_governor(name)' is enough readable.
OK. skip the my comment of changing the variable name. Thanks. > 2. If we want to use governor_name and then do not have the line exceeding the > 80 characters I need to split the function in two lines. For me the > readability > is better when you have all in one line. > > If I did not convince you, just let me now and I'll change for governor_name > :) > (snip) -- Best Regards, Chanwoo Choi Samsung Electronics
