Hi Matthias, On 2018년 07월 07일 02:07, Matthias Kaehlcke wrote: > Hi, > > On Wed, Jul 04, 2018 at 11:51:30AM +0900, Chanwoo Choi wrote: >> Hi, >> >> On 2018년 07월 04일 08:46, Matthias Kaehlcke wrote: >>> Move variables related with devfreq policy changes from struct devfreq >>> to the new struct devfreq_policy and add a policy field to struct devfreq. >>> >>> The following variables are moved: >>> >>> df->min/max_freq => p->user.min/max_freq >>> df->scaling_min/max_freq => p->devinfo.min/max_freq >>> df->governor => p->governor >>> df->governor_name => p->governor_name >>> >>> Signed-off-by: Matthias Kaehlcke <[email protected]> >>> Reviewed-by: Brian Norris <[email protected]> >>> --- >>> Changes in v5: >>> - none >>> >>> Changes in v4: >>> - added 'Reviewed-by: Brian Norris <[email protected]>' tag >>> >>> Changes in v3: >>> - none >>> >>> Changes in v2: >>> - performance, powersave and simpleondemand governors don't need changes >>> with "PM / devfreq: Don't adjust to user limits in governors" >>> - formatting fixes >>> --- >>> drivers/devfreq/devfreq.c | 137 ++++++++++++++++------------- >>> drivers/devfreq/governor_passive.c | 4 +- >>> include/linux/devfreq.h | 38 +++++--- > > >>> 3 files changed, 103 insertions(+), 76 deletions(-) >>> >> >> (skip) >> >>> >>> diff --git a/drivers/devfreq/governor_passive.c >>> b/drivers/devfreq/governor_passive.c >>> index 3bc29acbd54e..e0987c749ec2 100644 >>> --- a/drivers/devfreq/governor_passive.c >>> +++ b/drivers/devfreq/governor_passive.c >>> @@ -99,12 +99,12 @@ static int update_devfreq_passive(struct devfreq >>> *devfreq, unsigned long freq) >>> { >>> int ret; >>> >>> - if (!devfreq->governor) >>> + if (!devfreq->policy.governor) >>> return -EINVAL; >>> >>> mutex_lock_nested(&devfreq->lock, SINGLE_DEPTH_NESTING); >>> >>> - ret = devfreq->governor->get_target_freq(devfreq, &freq); >>> + ret = devfreq->policy.governor->get_target_freq(devfreq, &freq); >>> if (ret < 0) >>> goto out; >>> >>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h >>> index 3aae5b3af87c..9bf23b976f4d 100644 >>> --- a/include/linux/devfreq.h >>> +++ b/include/linux/devfreq.h >>> @@ -109,6 +109,30 @@ struct devfreq_dev_profile { >>> unsigned int max_state; >>> }; >>> >>> +/** >>> + * struct devfreq_freq_limits - Devfreq frequency limits >>> + * @min_freq: minimum frequency >>> + * @max_freq: maximum frequency >>> + */ >>> +struct devfreq_freq_limits { >>> + unsigned long min_freq; >>> + unsigned long max_freq; >>> +}; >>> + >>> +/** >>> + * struct devfreq_policy - Devfreq policy >>> + * @user: frequency limits requested by the user >>> + * @devinfo: frequency limits of the device (available OPPs) >>> + * @governor: method how to choose frequency based on the usage. >> >> nitpick. remove '.' on the end of line. > > Ok > >>> + * @governor_name: devfreq governor name for use with this devfreq >>> + */ >>> +struct devfreq_policy { >>> + struct devfreq_freq_limits user; >>> + struct devfreq_freq_limits devinfo; >>> + const struct devfreq_governor *governor; >>> + char governor_name[DEVFREQ_NAME_LEN]; >>> +}; >>> + >>> /** >>> * struct devfreq - Device devfreq structure >>> * @node: list node - contains the devices with devfreq that have been >>> @@ -117,8 +141,6 @@ struct devfreq_dev_profile { >>> * @dev: device registered by devfreq class. dev.parent is the device >>> * using devfreq. >>> * @profile: device-specific devfreq profile >>> - * @governor: method how to choose frequency based on the usage. >>> - * @governor_name: devfreq governor name for use with this devfreq >>> * @nb: notifier block used to notify devfreq object that it >>> should >>> * reevaluate operable frequencies. Devfreq users may use >>> * devfreq.nb to the corresponding register notifier call chain. >>> @@ -126,10 +148,7 @@ struct devfreq_dev_profile { >>> * @previous_freq: previously configured frequency value. >>> * @data: Private data of the governor. The devfreq framework does not >>> * touch this. >>> - * @min_freq: Limit minimum frequency requested by user (0: none) >>> - * @max_freq: Limit maximum frequency requested by user (0: none) >>> - * @scaling_min_freq: Limit minimum frequency requested by OPP >>> interface >>> - * @scaling_max_freq: Limit maximum frequency requested by OPP >>> interface >>> + * @policy: Policy for frequency adjustments >> >> The devfreq_policy contains the range of frequency and governor information. >> But, this description focus on the frequency. You need to explain the more >> correct description of 'policy'. > > I wouldn't say that the focus is on 'frequency', but on 'frequency > adjustments', and the governor is an integral part of them.
OK. I agree your original description. > > I can change it to "Policy for frequency adjustments, including > frequency limits and the governor" if you prefer. I'm open to other > suggestions. (snip) -- Best Regards, Chanwoo Choi Samsung Electronics
