| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <de7b3c55-a058-c8e8-4ebd-b2bb8dacf161@samsung.com>
Date: Wed, 8 Jan 2020 14:03:35 +0900
From: Chanwoo Choi <cw00.choi@...sung.com>
To: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: chanwoo@...nel.org, myungjoo.ham@...sung.com,
kyungmin.park@...sung.com
Subject: Re: [PATCH] PM / devfreq: Add missing function description and
rename static functions
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@...sung.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.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Powered by blists - more mailing lists