On Mon, May 28, 2018 at 02:19:49PM +0900, MyungJoo Ham wrote: > >Policy notifiers are called before a frequency change and may narrow > >the min/max frequency range in devfreq_policy, which is used to adjust > >the target frequency if it is beyond this range. > > > >Also add a few helpers: > > - devfreq_verify_within_[dev_]limits() > > - should be used by the notifiers for policy adjustments. > > - dev_to_devfreq() > > - lookup a devfreq strict from a device pointer > > > >Signed-off-by: Matthias Kaehlcke <m...@chromium.org> > >--- > > drivers/devfreq/devfreq.c | 47 +++++++++++++++++++++------- > > include/linux/devfreq.h | 66 +++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 102 insertions(+), 11 deletions(-) > > Hello Matthias, > > > Why should we have yet another notifier from an instance of devfreq? > Wouldn't it better to let the current notifier (transition notifier) > handle new events as well by adding possible event states to it?
Honestly the main reason is that I sought inspiration from cpufreq, which uses a dedicated policy notifier. Unfortunately this change predates the git history so I don't know what was the exact rationale to do it this way. Some minor advantages that I see are: - transition notifiers aren't bothered about adjustments and viceversa - different data types are passed for transitions and adjustments, which makes code of notifiers that handle both a bit more messy. > Anyway, is this the reason why you've separated some data of devfreq > into "policy" struct? (I was wondering why while reading commit 6/11). The DEVFREQ_ADJUST is the reason for the "policy struct". With this change we are dealing with 3 types of frequency pairs: user (df->min/max_freq), devinfo (df->scaling_min/max_freq) and the policy/adjustable ones. I think it is cleaner to group them in a struct (and sub-structs), rather than having 6 individual <type>_min/max_freq variables. Also it allows to only pass the policy object to the notifiers, instead of the entire devfreq device. I opted to do the introduction of the struct policy in a separate NOP patch, because I think it is easier to review the 'reorg' churn and the functional change separately. Please let me know if you'd prefer to have certain things done differently. Thanks Matthias
