From: Matthias Kaehlcke <mka@chromium.org>
To: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>,
Kyungmin Park <kyungmin.park@samsung.com>,
Chanwoo Choi <cw00.choi@samsung.com>,
Thierry Reding <thierry.reding@gmail.com>,
Jonathan Hunter <jonathanh@nvidia.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ingo Molnar <mingo@redhat.com>,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-tegra@vger.kernel.org,
Douglas Anderson <dianders@chromium.org>,
linux-tegra-owner@vger.kernel.org
Subject: Re: [PATCH 1/2] devfreq: Rename devfreq_update_status() to devfreq_update_stats() and viceversa
Date: Thu, 26 Sep 2019 10:11:00 -0700 [thread overview]
Message-ID: <20190926171100.GQ133864@google.com> (raw)
In-Reply-To: <3015d1aec68ca2b35a263d61bf13077e@codethink.co.uk>
On Thu, Sep 26, 2019 at 08:21:51AM +0100, Ben Dooks wrote:
> /
>
> On 2019-09-25 19:43, Matthias Kaehlcke wrote:
> > devfreq has two functions with very similar names,
> > devfreq_update_status()
> > and devfreq_update_stats(). _update_status() currently updates
> > frequency transitions statistics, while _update_stats() retrieves the
> > device 'status'. The function names are inversed with respect to what
> > the functions are actually doing, rename devfreq_update_status() to
> > devfreq_update_stats() and viceversa.
>
> Wouldn't having devfreq_get_stats() be a better name for this if it
> is retrieving the stats?
struct devfreq_dev_status is a bit ambiguous. It contains 'stat' fields
like 'total_time' and 'busy_time', but also 'current_frequency' which is
more a 'status'. Given the name of the struct and the name of the hook
profile->get_dev_status I'm inclined to refer to it as 'status', also to
disambiguate it from the transition stats.
That said I'd welcome a name that's easier to differantiate from the other
devfreq_update_stat* function, like devfreq_update_status() or
devfreq_refresh_status().
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > We could also rename the current devfreq_update_stats() to
> > devfreq_refresh_status() to make it easier to distinguish it from
> > devfreq_update_stats().
> > ---
> > drivers/devfreq/devfreq.c | 12 ++++++------
> > drivers/devfreq/governor.h | 4 ++--
> > drivers/devfreq/governor_passive.c | 2 +-
> > drivers/devfreq/governor_simpleondemand.c | 2 +-
> > drivers/devfreq/tegra30-devfreq.c | 2 +-
> > 5 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index 446490c9d635..fb4318d59aa9 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -151,11 +151,11 @@ static int set_freq_table(struct devfreq *devfreq)
> > }
> >
> > /**
> > - * devfreq_update_status() - Update statistics of devfreq behavior
> > + * devfreq_update_stats() - Update statistics of devfreq behavior
> > * @devfreq: the devfreq instance
> > * @freq: the update target frequency
> > */
> > -int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
> > +int devfreq_update_stats(struct devfreq *devfreq, unsigned long freq)
> > {
> > int lev, prev_lev, ret = 0;
> > unsigned long cur_time;
> > @@ -191,7 +191,7 @@ int devfreq_update_status(struct devfreq *devfreq,
> > unsigned long freq)
> > devfreq->last_stat_updated = cur_time;
> > return ret;
> > }
> > -EXPORT_SYMBOL(devfreq_update_status);
> > +EXPORT_SYMBOL(devfreq_update_stats);
> >
> > /**
> > * find_devfreq_governor() - find devfreq governor from name
> > @@ -311,7 +311,7 @@ static int devfreq_set_target(struct devfreq
> > *devfreq, unsigned long new_freq,
> > freqs.new = new_freq;
> > devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE);
> >
> > - if (devfreq_update_status(devfreq, new_freq))
> > + if (devfreq_update_stats(devfreq, new_freq))
> > dev_err(&devfreq->dev,
> > "Couldn't update frequency transition information.\n");
> >
> > @@ -450,7 +450,7 @@ void devfreq_monitor_suspend(struct devfreq
> > *devfreq)
> > return;
> > }
> >
> > - devfreq_update_status(devfreq, devfreq->previous_freq);
> > + devfreq_update_stats(devfreq, devfreq->previous_freq);
> > devfreq->stop_polling = true;
> > mutex_unlock(&devfreq->lock);
> > cancel_delayed_work_sync(&devfreq->work);
> > @@ -1398,7 +1398,7 @@ static ssize_t trans_stat_show(struct device *dev,
> > unsigned int max_state = devfreq->profile->max_state;
> >
> > if (!devfreq->stop_polling &&
> > - devfreq_update_status(devfreq, devfreq->previous_freq))
> > + devfreq_update_stats(devfreq, devfreq->previous_freq))
> > return 0;
> > if (max_state == 0)
> > return sprintf(buf, "Not Supported.\n");
> > diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
> > index bbe5ff9fcecf..e11f447be2b5 100644
> > --- a/drivers/devfreq/governor.h
> > +++ b/drivers/devfreq/governor.h
> > @@ -64,9 +64,9 @@ extern void devfreq_interval_update(struct devfreq
> > *devfreq,
> > extern int devfreq_add_governor(struct devfreq_governor *governor);
> > extern int devfreq_remove_governor(struct devfreq_governor *governor);
> >
> > -extern int devfreq_update_status(struct devfreq *devfreq, unsigned long
> > freq);
> > +extern int devfreq_update_stats(struct devfreq *devfreq, unsigned long
> > freq);
> >
> > -static inline int devfreq_update_stats(struct devfreq *df)
> > +static inline int devfreq_update_status(struct devfreq *df)
> > {
> > return df->profile->get_dev_status(df->dev.parent, &df->last_status);
> > }
> > diff --git a/drivers/devfreq/governor_passive.c
> > b/drivers/devfreq/governor_passive.c
> > index be6eeab9c814..1c746b96d3db 100644
> > --- a/drivers/devfreq/governor_passive.c
> > +++ b/drivers/devfreq/governor_passive.c
> > @@ -110,7 +110,7 @@ static int update_devfreq_passive(struct devfreq
> > *devfreq, unsigned long freq)
> > goto out;
> >
> > if (devfreq->profile->freq_table
> > - && (devfreq_update_status(devfreq, freq)))
> > + && (devfreq_update_stats(devfreq, freq)))
> > dev_err(&devfreq->dev,
> > "Couldn't update frequency transition information.\n");
> >
> > diff --git a/drivers/devfreq/governor_simpleondemand.c
> > b/drivers/devfreq/governor_simpleondemand.c
> > index 3d809f228619..2cbf26bdcfd6 100644
> > --- a/drivers/devfreq/governor_simpleondemand.c
> > +++ b/drivers/devfreq/governor_simpleondemand.c
> > @@ -25,7 +25,7 @@ static int devfreq_simple_ondemand_func(struct devfreq
> > *df,
> > unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
> > struct devfreq_simple_ondemand_data *data = df->data;
> >
> > - err = devfreq_update_stats(df);
> > + err = devfreq_update_status(df);
> > if (err)
> > return err;
> >
> > diff --git a/drivers/devfreq/tegra30-devfreq.c
> > b/drivers/devfreq/tegra30-devfreq.c
> > index a6ba75f4106d..536273a811fe 100644
> > --- a/drivers/devfreq/tegra30-devfreq.c
> > +++ b/drivers/devfreq/tegra30-devfreq.c
> > @@ -526,7 +526,7 @@ static int tegra_governor_get_target(struct
> > devfreq *devfreq,
> > unsigned int i;
> > int err;
> >
> > - err = devfreq_update_stats(devfreq);
> > + err = devfreq_update_status(devfreq);
> > if (err)
> > return err;
next prev parent reply other threads:[~2019-09-26 17:11 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20190925184505epcas2p3f8f4395f37df4b1fe33309393e8af4df@epcas2p3.samsung.com>
2019-09-25 18:43 ` [PATCH 1/2] devfreq: Rename devfreq_update_status() to devfreq_update_stats() and viceversa Matthias Kaehlcke
2019-09-25 18:43 ` [PATCH 2/2] devfreq: Rename df->previous_freq to df->cur_freq Matthias Kaehlcke
2019-09-26 1:41 ` Chanwoo Choi
2019-09-26 17:35 ` Matthias Kaehlcke
2019-09-26 1:36 ` [PATCH 1/2] devfreq: Rename devfreq_update_status() to devfreq_update_stats() and viceversa Chanwoo Choi
2019-09-26 16:51 ` Matthias Kaehlcke
2019-09-26 7:21 ` Ben Dooks
2019-09-26 17:11 ` Matthias Kaehlcke [this message]
2019-09-26 17:14 ` Matthias Kaehlcke
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190926171100.GQ133864@google.com \
--to=mka@chromium.org \
--cc=ben.dooks@codethink.co.uk \
--cc=cw00.choi@samsung.com \
--cc=dianders@chromium.org \
--cc=jonathanh@nvidia.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-tegra-owner@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=myungjoo.ham@samsung.com \
--cc=rostedt@goodmis.org \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).