| Message ID | 20200107013523.27177-1-cw00.choi@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Chanwoo Choi |
| Headers | show |
| Series | PM / devfreq: Add missing function description and rename static functions | expand |
On 1/7/20 10:35 AM, Chanwoo Choi wrote: > Rename internal function name used by devfreq core without 'devfreq_' > prefix in order to separate them from the exported functions. > And add missing function description for improving the readability. > > Lastly, add the comments of devfreq_add_device to increase > the understanding of behavior of devfreq_add_device. > > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> > --- > drivers/devfreq/devfreq.c | 81 +++++++++++++++++++++++++++++++-------- > 1 file changed, 64 insertions(+), 17 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index acd21345a070..254f11b31824 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -48,7 +48,7 @@ static LIST_HEAD(devfreq_list); > static DEFINE_MUTEX(devfreq_list_lock); > > /** > - * find_device_devfreq() - find devfreq struct using device pointer > + * find_device_devfreq() - Find devfreq struct using device pointer > * @dev: device pointer used to lookup device devfreq. > * > * Search the list of device devfreqs and return the matched device's > @@ -73,6 +73,13 @@ static struct devfreq *find_device_devfreq(struct device *dev) > return ERR_PTR(-ENODEV); > } > > +/** > + * find_available_min_freq() - Find available min frequency via OPP interface > + * @devfreq: the devfreq instance > + * > + * Find available minimum frequency among the active OPP entries > + * because could either enable or disable the frequency by using OPP interface. > + */ > static unsigned long find_available_min_freq(struct devfreq *devfreq) > { > struct dev_pm_opp *opp; > @@ -87,6 +94,13 @@ static unsigned long find_available_min_freq(struct devfreq *devfreq) > return min_freq; > } > > +/** > + * find_available_max_freq() - Find available max frequency via OPP interface > + * @devfreq: the devfreq instance > + * > + * Find available maximum frequency among the active OPP entries > + * because could either enable or disable the frequency by using OPP interface. > + */ > static unsigned long find_available_max_freq(struct devfreq *devfreq) > { > struct dev_pm_opp *opp; > @@ -150,11 +164,11 @@ static void get_freq_range(struct devfreq *devfreq, > } > > /** > - * devfreq_get_freq_level() - Lookup freq_table for the frequency > + * get_freq_level() - Lookup freq_table for the frequency > * @devfreq: the devfreq instance > * @freq: the target frequency > */ > -static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) > +static int get_freq_level(struct devfreq *devfreq, unsigned long freq) > { > int lev; > > @@ -165,6 +179,13 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) > return -EINVAL; > } > > +/** > + * set_freq_table() - Fill out the freq_table of devfreq instance > + * @devfreq: the devfreq instance > + * > + * If freq_table array is NULL, fill out the freq_table array > + * by using OPP interface because OPP is mandatory. > + */ > static int set_freq_table(struct devfreq *devfreq) > { > struct devfreq_dev_profile *profile = devfreq->profile; > @@ -218,7 +239,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) > if (!devfreq->previous_freq) > goto out; > > - prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq); > + prev_lev = get_freq_level(devfreq, devfreq->previous_freq); > if (prev_lev < 0) { > ret = prev_lev; > goto out; > @@ -227,7 +248,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) > devfreq->stats.time_in_state[prev_lev] += > cur_time - devfreq->stats.last_update; > > - lev = devfreq_get_freq_level(devfreq, freq); > + lev = get_freq_level(devfreq, freq); > if (lev < 0) { > ret = lev; > goto out; > @@ -246,7 +267,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) > EXPORT_SYMBOL(devfreq_update_status); > > /** > - * find_devfreq_governor() - find devfreq governor from name > + * find_devfreq_governor() - Find devfreq governor from name > * @name: name of the governor > * > * Search the list of devfreq governors and return the matched > @@ -314,8 +335,17 @@ static struct devfreq_governor *try_then_request_governor(const char *name) > return governor; > } > > -static int devfreq_notify_transition(struct devfreq *devfreq, > - struct devfreq_freqs *freqs, unsigned int state) > +/** > + * notify_transition() - Send the transition notification > + * @name: name of the governor > + * @freqs: the data containing the both old and new frequency > + * @state: the kind of notification > + * > + * Send the transition notification to the registered receivers > + * in order to inform the frequency change. > + */ > +static int notify_transition(struct devfreq *devfreq, > + struct devfreq_freqs *freqs, unsigned int state) > { > if (!devfreq) > return -EINVAL; > @@ -337,8 +367,17 @@ static int devfreq_notify_transition(struct devfreq *devfreq, > return 0; > } > > -static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq, > - u32 flags) > +/** > + * set_target() - Set target frequency of devfreq instance > + * @devfreq: the devfreq instance > + * @new_freq: the target frequency > + * @flags: flags handed from devfreq framework > + * > + * Set the target frequency of which is decided by governor > + * and then is adjusted with constraints. > + */ > +static int set_target(struct devfreq *devfreq, > + unsigned long new_freq, u32 flags) > { > struct devfreq_freqs freqs; > unsigned long cur_freq; > @@ -351,17 +390,17 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq, > > freqs.old = cur_freq; > freqs.new = new_freq; > - devfreq_notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE); > + notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE); > > err = devfreq->profile->target(devfreq->dev.parent, &new_freq, flags); > if (err) { > freqs.new = cur_freq; > - devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); > + notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); > return err; > } > > freqs.new = new_freq; > - devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); > + notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); > > if (devfreq_update_status(devfreq, new_freq)) > dev_err(&devfreq->dev, > @@ -413,7 +452,7 @@ int update_devfreq(struct devfreq *devfreq) > flags |= DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use LUB */ > } > > - return devfreq_set_target(devfreq, freq, flags); > + return set_target(devfreq, freq, flags); > > } > EXPORT_SYMBOL(update_devfreq); > @@ -421,7 +460,6 @@ EXPORT_SYMBOL(update_devfreq); > /** > * devfreq_monitor() - Periodically poll devfreq objects. > * @work: the work struct used to run devfreq_monitor periodically. > - * > */ > static void devfreq_monitor(struct work_struct *work) > { > @@ -739,11 +777,13 @@ struct devfreq *devfreq_add_device(struct device *dev, > static atomic_t devfreq_no = ATOMIC_INIT(-1); > int err = 0; > > + /* Check the parameter is valid */ > if (!dev || !profile || !governor_name) { > dev_err(dev, "%s: Invalid parameters.\n", __func__); > return ERR_PTR(-EINVAL); > } > > + /* Check the device is already added or not */ > mutex_lock(&devfreq_list_lock); > devfreq = find_device_devfreq(dev); > mutex_unlock(&devfreq_list_lock); > @@ -754,6 +794,7 @@ struct devfreq *devfreq_add_device(struct device *dev, > goto err_out; > } > > + /* Initialize the devfreq instance */ > devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL); > if (!devfreq) { > err = -ENOMEM; > @@ -798,6 +839,7 @@ struct devfreq *devfreq_add_device(struct device *dev, > devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); > atomic_set(&devfreq->suspend_count, 0); > > + /* Register a device for devfreq instance */ > dev_set_name(&devfreq->dev, "devfreq%d", > atomic_inc_return(&devfreq_no)); > err = device_register(&devfreq->dev); > @@ -807,6 +849,7 @@ struct devfreq *devfreq_add_device(struct device *dev, > goto err_out; > } > > + /* Initialize the statistics of devfreq device behavior */ > devfreq->stats.trans_table = devm_kzalloc(&devfreq->dev, > array3_size(sizeof(unsigned int), > devfreq->profile->max_state, > @@ -831,10 +874,12 @@ struct devfreq *devfreq_add_device(struct device *dev, > devfreq->stats.total_trans = 0; > devfreq->stats.last_update = get_jiffies_64(); > > + /* Initialize notifiers for informing the transition of devfreq */ > srcu_init_notifier_head(&devfreq->transition_notifier_list); > > mutex_unlock(&devfreq->lock); > > + /* Initialize PM QoS for applying the constraints */ > err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req, > DEV_PM_QOS_MIN_FREQUENCY, 0); > if (err < 0) > @@ -859,6 +904,7 @@ struct devfreq *devfreq_add_device(struct device *dev, > > mutex_lock(&devfreq_list_lock); > > + /* Find the devfreq governor and start */ > governor = try_then_request_governor(devfreq->governor_name); > if (IS_ERR(governor)) { > dev_err(dev, "%s: Unable to find governor for the device\n", > @@ -876,6 +922,7 @@ struct devfreq *devfreq_add_device(struct device *dev, > goto err_init; > } > > + /* Add the devfreq instance to global devfreq list finally */ > list_add(&devfreq->node, &devfreq_list); > > mutex_unlock(&devfreq_list_lock); > @@ -1076,7 +1123,7 @@ int devfreq_suspend_device(struct devfreq *devfreq) > > if (devfreq->suspend_freq) { > mutex_lock(&devfreq->lock); > - ret = devfreq_set_target(devfreq, devfreq->suspend_freq, 0); > + ret = set_target(devfreq, devfreq->suspend_freq, 0); > mutex_unlock(&devfreq->lock); > if (ret) > return ret; > @@ -1106,7 +1153,7 @@ int devfreq_resume_device(struct devfreq *devfreq) > > if (devfreq->resume_freq) { > mutex_lock(&devfreq->lock); > - ret = devfreq_set_target(devfreq, devfreq->resume_freq, 0); > + ret = set_target(devfreq, devfreq->resume_freq, 0); > mutex_unlock(&devfreq->lock); > if (ret) > return ret; > Applied it.
On 1/8/20 2:03 PM, Chanwoo Choi wrote: > On 1/7/20 10:35 AM, Chanwoo Choi wrote: >> Rename internal function name used by devfreq core without 'devfreq_' >> prefix in order to separate them from the exported functions. >> And add missing function description for improving the readability. >> >> Lastly, add the comments of devfreq_add_device to increase >> the understanding of behavior of devfreq_add_device. >> >> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> >> --- >> drivers/devfreq/devfreq.c | 81 +++++++++++++++++++++++++++++++-------- >> 1 file changed, 64 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >> index acd21345a070..254f11b31824 100644 >> --- a/drivers/devfreq/devfreq.c >> +++ b/drivers/devfreq/devfreq.c >> @@ -48,7 +48,7 @@ static LIST_HEAD(devfreq_list); >> static DEFINE_MUTEX(devfreq_list_lock); >> >> /** >> - * find_device_devfreq() - find devfreq struct using device pointer >> + * find_device_devfreq() - Find devfreq struct using device pointer >> * @dev: device pointer used to lookup device devfreq. >> * >> * Search the list of device devfreqs and return the matched device's >> @@ -73,6 +73,13 @@ static struct devfreq *find_device_devfreq(struct device *dev) >> return ERR_PTR(-ENODEV); >> } >> >> +/** >> + * find_available_min_freq() - Find available min frequency via OPP interface >> + * @devfreq: the devfreq instance >> + * >> + * Find available minimum frequency among the active OPP entries >> + * because could either enable or disable the frequency by using OPP interface. >> + */ >> static unsigned long find_available_min_freq(struct devfreq *devfreq) >> { >> struct dev_pm_opp *opp; >> @@ -87,6 +94,13 @@ static unsigned long find_available_min_freq(struct devfreq *devfreq) >> return min_freq; >> } >> >> +/** >> + * find_available_max_freq() - Find available max frequency via OPP interface >> + * @devfreq: the devfreq instance >> + * >> + * Find available maximum frequency among the active OPP entries >> + * because could either enable or disable the frequency by using OPP interface. >> + */ >> static unsigned long find_available_max_freq(struct devfreq *devfreq) >> { >> struct dev_pm_opp *opp; >> @@ -150,11 +164,11 @@ static void get_freq_range(struct devfreq *devfreq, >> } >> >> /** >> - * devfreq_get_freq_level() - Lookup freq_table for the frequency >> + * get_freq_level() - Lookup freq_table for the frequency >> * @devfreq: the devfreq instance >> * @freq: the target frequency >> */ >> -static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) >> +static int get_freq_level(struct devfreq *devfreq, unsigned long freq) >> { >> int lev; >> >> @@ -165,6 +179,13 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) >> return -EINVAL; >> } >> >> +/** >> + * set_freq_table() - Fill out the freq_table of devfreq instance >> + * @devfreq: the devfreq instance >> + * >> + * If freq_table array is NULL, fill out the freq_table array >> + * by using OPP interface because OPP is mandatory. >> + */ >> static int set_freq_table(struct devfreq *devfreq) >> { >> struct devfreq_dev_profile *profile = devfreq->profile; >> @@ -218,7 +239,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >> if (!devfreq->previous_freq) >> goto out; >> >> - prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq); >> + prev_lev = get_freq_level(devfreq, devfreq->previous_freq); >> if (prev_lev < 0) { >> ret = prev_lev; >> goto out; >> @@ -227,7 +248,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >> devfreq->stats.time_in_state[prev_lev] += >> cur_time - devfreq->stats.last_update; >> >> - lev = devfreq_get_freq_level(devfreq, freq); >> + lev = get_freq_level(devfreq, freq); >> if (lev < 0) { >> ret = lev; >> goto out; >> @@ -246,7 +267,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >> EXPORT_SYMBOL(devfreq_update_status); >> >> /** >> - * find_devfreq_governor() - find devfreq governor from name >> + * find_devfreq_governor() - Find devfreq governor from name >> * @name: name of the governor >> * >> * Search the list of devfreq governors and return the matched >> @@ -314,8 +335,17 @@ static struct devfreq_governor *try_then_request_governor(const char *name) >> return governor; >> } >> >> -static int devfreq_notify_transition(struct devfreq *devfreq, >> - struct devfreq_freqs *freqs, unsigned int state) >> +/** >> + * notify_transition() - Send the transition notification >> + * @name: name of the governor >> + * @freqs: the data containing the both old and new frequency >> + * @state: the kind of notification >> + * >> + * Send the transition notification to the registered receivers >> + * in order to inform the frequency change. >> + */ >> +static int notify_transition(struct devfreq *devfreq, >> + struct devfreq_freqs *freqs, unsigned int state) >> { >> if (!devfreq) >> return -EINVAL; >> @@ -337,8 +367,17 @@ static int devfreq_notify_transition(struct devfreq *devfreq, >> return 0; >> } >> >> -static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq, >> - u32 flags) >> +/** >> + * set_target() - Set target frequency of devfreq instance >> + * @devfreq: the devfreq instance >> + * @new_freq: the target frequency >> + * @flags: flags handed from devfreq framework >> + * >> + * Set the target frequency of which is decided by governor >> + * and then is adjusted with constraints. >> + */ >> +static int set_target(struct devfreq *devfreq, >> + unsigned long new_freq, u32 flags) >> { >> struct devfreq_freqs freqs; >> unsigned long cur_freq; >> @@ -351,17 +390,17 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq, >> >> freqs.old = cur_freq; >> freqs.new = new_freq; >> - devfreq_notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE); >> + notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE); >> >> err = devfreq->profile->target(devfreq->dev.parent, &new_freq, flags); >> if (err) { >> freqs.new = cur_freq; >> - devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); >> + notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); >> return err; >> } >> >> freqs.new = new_freq; >> - devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); >> + notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); >> >> if (devfreq_update_status(devfreq, new_freq)) >> dev_err(&devfreq->dev, >> @@ -413,7 +452,7 @@ int update_devfreq(struct devfreq *devfreq) >> flags |= DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use LUB */ >> } >> >> - return devfreq_set_target(devfreq, freq, flags); >> + return set_target(devfreq, freq, flags); >> >> } >> EXPORT_SYMBOL(update_devfreq); >> @@ -421,7 +460,6 @@ EXPORT_SYMBOL(update_devfreq); >> /** >> * devfreq_monitor() - Periodically poll devfreq objects. >> * @work: the work struct used to run devfreq_monitor periodically. >> - * >> */ >> static void devfreq_monitor(struct work_struct *work) >> { >> @@ -739,11 +777,13 @@ struct devfreq *devfreq_add_device(struct device *dev, >> static atomic_t devfreq_no = ATOMIC_INIT(-1); >> int err = 0; >> >> + /* Check the parameter is valid */ >> if (!dev || !profile || !governor_name) { >> dev_err(dev, "%s: Invalid parameters.\n", __func__); >> return ERR_PTR(-EINVAL); >> } >> >> + /* Check the device is already added or not */ >> mutex_lock(&devfreq_list_lock); >> devfreq = find_device_devfreq(dev); >> mutex_unlock(&devfreq_list_lock); >> @@ -754,6 +794,7 @@ struct devfreq *devfreq_add_device(struct device *dev, >> goto err_out; >> } >> >> + /* Initialize the devfreq instance */ >> devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL); >> if (!devfreq) { >> err = -ENOMEM; >> @@ -798,6 +839,7 @@ struct devfreq *devfreq_add_device(struct device *dev, >> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); >> atomic_set(&devfreq->suspend_count, 0); >> >> + /* Register a device for devfreq instance */ >> dev_set_name(&devfreq->dev, "devfreq%d", >> atomic_inc_return(&devfreq_no)); >> err = device_register(&devfreq->dev); >> @@ -807,6 +849,7 @@ struct devfreq *devfreq_add_device(struct device *dev, >> goto err_out; >> } >> >> + /* Initialize the statistics of devfreq device behavior */ >> devfreq->stats.trans_table = devm_kzalloc(&devfreq->dev, >> array3_size(sizeof(unsigned int), >> devfreq->profile->max_state, >> @@ -831,10 +874,12 @@ struct devfreq *devfreq_add_device(struct device *dev, >> devfreq->stats.total_trans = 0; >> devfreq->stats.last_update = get_jiffies_64(); >> >> + /* Initialize notifiers for informing the transition of devfreq */ >> srcu_init_notifier_head(&devfreq->transition_notifier_list); >> >> mutex_unlock(&devfreq->lock); >> >> + /* Initialize PM QoS for applying the constraints */ >> err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req, >> DEV_PM_QOS_MIN_FREQUENCY, 0); >> if (err < 0) >> @@ -859,6 +904,7 @@ struct devfreq *devfreq_add_device(struct device *dev, >> >> mutex_lock(&devfreq_list_lock); >> >> + /* Find the devfreq governor and start */ >> governor = try_then_request_governor(devfreq->governor_name); >> if (IS_ERR(governor)) { >> dev_err(dev, "%s: Unable to find governor for the device\n", >> @@ -876,6 +922,7 @@ struct devfreq *devfreq_add_device(struct device *dev, >> goto err_init; >> } >> >> + /* Add the devfreq instance to global devfreq list finally */ >> list_add(&devfreq->node, &devfreq_list); >> >> mutex_unlock(&devfreq_list_lock); >> @@ -1076,7 +1123,7 @@ int devfreq_suspend_device(struct devfreq *devfreq) >> >> if (devfreq->suspend_freq) { >> mutex_lock(&devfreq->lock); >> - ret = devfreq_set_target(devfreq, devfreq->suspend_freq, 0); >> + ret = set_target(devfreq, devfreq->suspend_freq, 0); >> mutex_unlock(&devfreq->lock); >> if (ret) >> return ret; >> @@ -1106,7 +1153,7 @@ int devfreq_resume_device(struct devfreq *devfreq) >> >> if (devfreq->resume_freq) { >> mutex_lock(&devfreq->lock); >> - ret = devfreq_set_target(devfreq, devfreq->resume_freq, 0); >> + ret = set_target(devfreq, devfreq->resume_freq, 0); >> mutex_unlock(&devfreq->lock); >> if (ret) >> return ret; >> > > Applied it. > I'm sorry. Drop this patch because it is not enough to add the missing description for functions. I'll add the more description and then resend it.
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index acd21345a070..254f11b31824 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -48,7 +48,7 @@ static LIST_HEAD(devfreq_list); static DEFINE_MUTEX(devfreq_list_lock); /** - * find_device_devfreq() - find devfreq struct using device pointer + * find_device_devfreq() - Find devfreq struct using device pointer * @dev: device pointer used to lookup device devfreq. * * Search the list of device devfreqs and return the matched device's @@ -73,6 +73,13 @@ static struct devfreq *find_device_devfreq(struct device *dev) return ERR_PTR(-ENODEV); } +/** + * find_available_min_freq() - Find available min frequency via OPP interface + * @devfreq: the devfreq instance + * + * Find available minimum frequency among the active OPP entries + * because could either enable or disable the frequency by using OPP interface. + */ static unsigned long find_available_min_freq(struct devfreq *devfreq) { struct dev_pm_opp *opp; @@ -87,6 +94,13 @@ static unsigned long find_available_min_freq(struct devfreq *devfreq) return min_freq; } +/** + * find_available_max_freq() - Find available max frequency via OPP interface + * @devfreq: the devfreq instance + * + * Find available maximum frequency among the active OPP entries + * because could either enable or disable the frequency by using OPP interface. + */ static unsigned long find_available_max_freq(struct devfreq *devfreq) { struct dev_pm_opp *opp; @@ -150,11 +164,11 @@ static void get_freq_range(struct devfreq *devfreq, } /** - * devfreq_get_freq_level() - Lookup freq_table for the frequency + * get_freq_level() - Lookup freq_table for the frequency * @devfreq: the devfreq instance * @freq: the target frequency */ -static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) +static int get_freq_level(struct devfreq *devfreq, unsigned long freq) { int lev; @@ -165,6 +179,13 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) return -EINVAL; } +/** + * set_freq_table() - Fill out the freq_table of devfreq instance + * @devfreq: the devfreq instance + * + * If freq_table array is NULL, fill out the freq_table array + * by using OPP interface because OPP is mandatory. + */ static int set_freq_table(struct devfreq *devfreq) { struct devfreq_dev_profile *profile = devfreq->profile; @@ -218,7 +239,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) if (!devfreq->previous_freq) goto out; - prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq); + prev_lev = get_freq_level(devfreq, devfreq->previous_freq); if (prev_lev < 0) { ret = prev_lev; goto out; @@ -227,7 +248,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) devfreq->stats.time_in_state[prev_lev] += cur_time - devfreq->stats.last_update; - lev = devfreq_get_freq_level(devfreq, freq); + lev = get_freq_level(devfreq, freq); if (lev < 0) { ret = lev; goto out; @@ -246,7 +267,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) EXPORT_SYMBOL(devfreq_update_status); /** - * find_devfreq_governor() - find devfreq governor from name + * find_devfreq_governor() - Find devfreq governor from name * @name: name of the governor * * Search the list of devfreq governors and return the matched @@ -314,8 +335,17 @@ static struct devfreq_governor *try_then_request_governor(const char *name) return governor; } -static int devfreq_notify_transition(struct devfreq *devfreq, - struct devfreq_freqs *freqs, unsigned int state) +/** + * notify_transition() - Send the transition notification + * @name: name of the governor + * @freqs: the data containing the both old and new frequency + * @state: the kind of notification + * + * Send the transition notification to the registered receivers + * in order to inform the frequency change. + */ +static int notify_transition(struct devfreq *devfreq, + struct devfreq_freqs *freqs, unsigned int state) { if (!devfreq) return -EINVAL; @@ -337,8 +367,17 @@ static int devfreq_notify_transition(struct devfreq *devfreq, return 0; } -static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq, - u32 flags) +/** + * set_target() - Set target frequency of devfreq instance + * @devfreq: the devfreq instance + * @new_freq: the target frequency + * @flags: flags handed from devfreq framework + * + * Set the target frequency of which is decided by governor + * and then is adjusted with constraints. + */ +static int set_target(struct devfreq *devfreq, + unsigned long new_freq, u32 flags) { struct devfreq_freqs freqs; unsigned long cur_freq; @@ -351,17 +390,17 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq, freqs.old = cur_freq; freqs.new = new_freq; - devfreq_notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE); + notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE); err = devfreq->profile->target(devfreq->dev.parent, &new_freq, flags); if (err) { freqs.new = cur_freq; - devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); + notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); return err; } freqs.new = new_freq; - devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); + notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE); if (devfreq_update_status(devfreq, new_freq)) dev_err(&devfreq->dev, @@ -413,7 +452,7 @@ int update_devfreq(struct devfreq *devfreq) flags |= DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use LUB */ } - return devfreq_set_target(devfreq, freq, flags); + return set_target(devfreq, freq, flags); } EXPORT_SYMBOL(update_devfreq); @@ -421,7 +460,6 @@ EXPORT_SYMBOL(update_devfreq); /** * devfreq_monitor() - Periodically poll devfreq objects. * @work: the work struct used to run devfreq_monitor periodically. - * */ static void devfreq_monitor(struct work_struct *work) { @@ -739,11 +777,13 @@ struct devfreq *devfreq_add_device(struct device *dev, static atomic_t devfreq_no = ATOMIC_INIT(-1); int err = 0; + /* Check the parameter is valid */ if (!dev || !profile || !governor_name) { dev_err(dev, "%s: Invalid parameters.\n", __func__); return ERR_PTR(-EINVAL); } + /* Check the device is already added or not */ mutex_lock(&devfreq_list_lock); devfreq = find_device_devfreq(dev); mutex_unlock(&devfreq_list_lock); @@ -754,6 +794,7 @@ struct devfreq *devfreq_add_device(struct device *dev, goto err_out; } + /* Initialize the devfreq instance */ devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL); if (!devfreq) { err = -ENOMEM; @@ -798,6 +839,7 @@ struct devfreq *devfreq_add_device(struct device *dev, devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); atomic_set(&devfreq->suspend_count, 0); + /* Register a device for devfreq instance */ dev_set_name(&devfreq->dev, "devfreq%d", atomic_inc_return(&devfreq_no)); err = device_register(&devfreq->dev); @@ -807,6 +849,7 @@ struct devfreq *devfreq_add_device(struct device *dev, goto err_out; } + /* Initialize the statistics of devfreq device behavior */ devfreq->stats.trans_table = devm_kzalloc(&devfreq->dev, array3_size(sizeof(unsigned int), devfreq->profile->max_state, @@ -831,10 +874,12 @@ struct devfreq *devfreq_add_device(struct device *dev, devfreq->stats.total_trans = 0; devfreq->stats.last_update = get_jiffies_64(); + /* Initialize notifiers for informing the transition of devfreq */ srcu_init_notifier_head(&devfreq->transition_notifier_list); mutex_unlock(&devfreq->lock); + /* Initialize PM QoS for applying the constraints */ err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req, DEV_PM_QOS_MIN_FREQUENCY, 0); if (err < 0) @@ -859,6 +904,7 @@ struct devfreq *devfreq_add_device(struct device *dev, mutex_lock(&devfreq_list_lock); + /* Find the devfreq governor and start */ governor = try_then_request_governor(devfreq->governor_name); if (IS_ERR(governor)) { dev_err(dev, "%s: Unable to find governor for the device\n", @@ -876,6 +922,7 @@ struct devfreq *devfreq_add_device(struct device *dev, goto err_init; } + /* Add the devfreq instance to global devfreq list finally */ list_add(&devfreq->node, &devfreq_list); mutex_unlock(&devfreq_list_lock); @@ -1076,7 +1123,7 @@ int devfreq_suspend_device(struct devfreq *devfreq) if (devfreq->suspend_freq) { mutex_lock(&devfreq->lock); - ret = devfreq_set_target(devfreq, devfreq->suspend_freq, 0); + ret = set_target(devfreq, devfreq->suspend_freq, 0); mutex_unlock(&devfreq->lock); if (ret) return ret; @@ -1106,7 +1153,7 @@ int devfreq_resume_device(struct devfreq *devfreq) if (devfreq->resume_freq) { mutex_lock(&devfreq->lock); - ret = devfreq_set_target(devfreq, devfreq->resume_freq, 0); + ret = set_target(devfreq, devfreq->resume_freq, 0); mutex_unlock(&devfreq->lock); if (ret) return ret;
Rename internal function name used by devfreq core without 'devfreq_' prefix in order to separate them from the exported functions. And add missing function description for improving the readability. Lastly, add the comments of devfreq_add_device to increase the understanding of behavior of devfreq_add_device. Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> --- drivers/devfreq/devfreq.c | 81 +++++++++++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 17 deletions(-)