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:14:24 -0700 [thread overview]
Message-ID: <20190926171424.GR133864@google.com> (raw)
In-Reply-To: <20190926171100.GQ133864@google.com>
On Thu, Sep 26, 2019 at 10:11:00AM -0700, Matthias Kaehlcke wrote:
> 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
~~~~~~~~~~~~~~~~~~~~~~~
I meant devfreq_get_status()
> 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;
prev parent reply other threads:[~2019-09-26 17:14 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
2019-09-26 17:14 ` Matthias Kaehlcke [this message]
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=20190926171424.GR133864@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.