[3/4] PM / devfreq: Add devfreq_governor_start/stop()
diff mbox series

Message ID 20190214013042.254790-4-mka@chromium.org
State Deferred
Headers show
Series
  • PM / devfreq: Refactor load monitoring
Related show

Commit Message

Matthias Kaehlcke Feb. 14, 2019, 1:30 a.m. UTC
The following pattern is repeated multiple times:

- call governor event handler with DEVFREQ_GOV_START/STOP
- check return value
- in case of error log a message

Add devfreq_governor_start/stop() helper functions with this code
and call them instead.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/devfreq/devfreq.c | 96 +++++++++++++++++++++++----------------
 1 file changed, 58 insertions(+), 38 deletions(-)

Comments

Chanwoo Choi Feb. 14, 2019, 2:12 p.m. UTC | #1
Hi Matthias,

Looks good to me for making the function to remove the duplicate code.
But,  When I just tested the kernel build, following warnings occur
about devfreq_governor_stop().

In file included from ./include/linux/devfreq.h:16:0,
                 from drivers/devfreq/devfreq.c:23:
drivers/devfreq/devfreq.c: In function ‘devfreq_governor_stop’:
drivers/devfreq/devfreq.c:619:17: warning: format ‘%s’ expects
argument of type ‘char *’, but argument 4 has type ‘int’ [-Wformat=]
   dev_warn(dev, "%s: Governor %s not stopped: %d\n",
                 ^
./include/linux/device.h:1380:22: note: in definition of macro ‘dev_fmt’
 #define dev_fmt(fmt) fmt
                      ^
drivers/devfreq/devfreq.c:619:3: note: in expansion of macro ‘dev_warn’
   dev_warn(dev, "%s: Governor %s not stopped: %d\n",
   ^
drivers/devfreq/devfreq.c:619:17: warning: format ‘%d’ expects a
matching ‘int’ argument [-Wformat=]
   dev_warn(dev, "%s: Governor %s not stopped: %d\n",
                 ^
./include/linux/device.h:1380:22: note: in definition of macro ‘dev_fmt’
 #define dev_fmt(fmt) fmt
                      ^
drivers/devfreq/devfreq.c:619:3: note: in expansion of macro ‘dev_warn’
   dev_warn(dev, "%s: Governor %s not stopped: %d\n",

2019년 2월 14일 (목) 오후 7:16, Matthias Kaehlcke <mka@chromium.org>님이 작성:
>
> The following pattern is repeated multiple times:
>
> - call governor event handler with DEVFREQ_GOV_START/STOP
> - check return value
> - in case of error log a message
>
> Add devfreq_governor_start/stop() helper functions with this code
> and call them instead.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/devfreq/devfreq.c | 96 +++++++++++++++++++++++----------------
>  1 file changed, 58 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 7fab6c4cf719b..eb86461648d74 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -580,6 +580,53 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
>         return ret;
>  }
>
> +/**
> + * devfreq_governor_start() - Start the devfreq governor of the device.
> + * @devfreq:   the devfreq instance
> + */
> +static int devfreq_governor_start(struct devfreq *devfreq)
> +{
> +       struct device *dev = devfreq->dev.parent;
> +       int err;
> +
> +       if (WARN(mutex_is_locked(&devfreq->lock),
> +                "mutex must *not* be held by the caller\n"))
> +               return -EINVAL;
> +
> +       err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
> +                                              NULL);
> +       if (err) {
> +               dev_err(dev, "Governor %s not started: %d\n",
> +                       devfreq->governor->name, err);
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * devfreq_governor_stop() - Stop the devfreq governor of the device.
> + * @devfreq:   the devfreq instance
> + */
> +static int devfreq_governor_stop(struct devfreq *devfreq)
> +{
> +       struct device *dev = devfreq->dev.parent;
> +       int err;
> +
> +       if (WARN(mutex_is_locked(&devfreq->lock),
> +                "mutex must *not* be held by the caller\n"))
> +               return -EINVAL;
> +
> +       err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_STOP, NULL);
> +       if (err) {
> +               dev_warn(dev, "%s: Governor %s not stopped: %d\n",
> +                        devfreq->governor->name, err);
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
>  /**
>   * devfreq_dev_release() - Callback for struct device to release the device.
>   * @dev:       the devfreq device
> @@ -720,13 +767,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
>         }
>
>         devfreq->governor = governor;
> -       err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
> -                                               NULL);
> -       if (err) {
> -               dev_err(dev, "%s: Unable to start governor for the device\n",
> -                       __func__);
> +       err = devfreq_governor_start(devfreq);
> +       if (err)
>                 goto err_init;
> -       }
>
>         list_add(&devfreq->node, &devfreq_list);
>
> @@ -1029,24 +1072,13 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>                                 dev_warn(dev,
>                                          "%s: Governor %s already present\n",
>                                          __func__, devfreq->governor->name);
> -                               ret = devfreq->governor->event_handler(devfreq,
> -                                                       DEVFREQ_GOV_STOP, NULL);
> -                               if (ret) {
> -                                       dev_warn(dev,
> -                                                "%s: Governor %s stop = %d\n",
> -                                                __func__,
> -                                                devfreq->governor->name, ret);
> -                               }
> +                               ret = devfreq_governor_stop(devfreq);
> +
>                                 /* Fall through */
>                         }
> +
>                         devfreq->governor = governor;
> -                       ret = devfreq->governor->event_handler(devfreq,
> -                                               DEVFREQ_GOV_START, NULL);
> -                       if (ret) {
> -                               dev_warn(dev, "%s: Governor %s start=%d\n",
> -                                        __func__, devfreq->governor->name,
> -                                        ret);
> -                       }
> +                       devfreq_governor_start(devfreq);
>                 }
>         }
>
> @@ -1081,7 +1113,6 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
>                 goto err_out;
>         }
>         list_for_each_entry(devfreq, &devfreq_list, node) {
> -               int ret;
>                 struct device *dev = devfreq->dev.parent;
>
>                 if (!strncmp(devfreq->governor_name, governor->name,
> @@ -1093,13 +1124,8 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
>                                 continue;
>                                 /* Fall through */
>                         }
> -                       ret = devfreq->governor->event_handler(devfreq,
> -                                               DEVFREQ_GOV_STOP, NULL);
> -                       if (ret) {
> -                               dev_warn(dev, "%s: Governor %s stop=%d\n",
> -                                        __func__, devfreq->governor->name,
> -                                        ret);
> -                       }
> +
> +                       devfreq_governor_stop(devfreq);
>                         devfreq->governor = NULL;
>                 }
>         }
> @@ -1149,19 +1175,13 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>         }
>
>         if (df->governor) {
> -               ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
> -               if (ret) {
> -                       dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
> -                                __func__, df->governor->name, ret);
> +               ret = devfreq_governor_stop(df);
> +               if (ret)
>                         goto out;
> -               }
>         }
>         df->governor = governor;
>         strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN);
> -       ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
> -       if (ret)
> -               dev_warn(dev, "%s: Governor %s not started(%d)\n",
> -                        __func__, df->governor->name, ret);
> +       ret = devfreq_governor_start(df);
>  out:
>         mutex_unlock(&devfreq_list_lock);
>
> --
> 2.20.1.791.gb4d0f1c61a-goog
>
Chanwoo Choi Feb. 14, 2019, 2:32 p.m. UTC | #2
Hi Matthias,

When I contributed the something to devfreq.c, if the newly added functions
are internal/static, just added the function without 'devfreq_' prefix
in order to distinguish them from the exported function as following:
- find_available_min_freq()
- find_available_max_freq()
- set_freq_table()

So, the governor_start/stop are the static function used only in devfreq.c,
in order to sustain the consistency of function naming, I recommened
that changes them as following:
- devfreq_governor_start -> governor_start
- devfreq_governor_stop -> governor_stop

2019년 2월 14일 (목) 오후 11:12, Chanwoo Choi <cwchoi00@gmail.com>님이 작성:
>
> Hi Matthias,
>
> Looks good to me for making the function to remove the duplicate code.
> But,  When I just tested the kernel build, following warnings occur
> about devfreq_governor_stop().
>
> In file included from ./include/linux/devfreq.h:16:0,
>                  from drivers/devfreq/devfreq.c:23:
> drivers/devfreq/devfreq.c: In function ‘devfreq_governor_stop’:
> drivers/devfreq/devfreq.c:619:17: warning: format ‘%s’ expects
> argument of type ‘char *’, but argument 4 has type ‘int’ [-Wformat=]
>    dev_warn(dev, "%s: Governor %s not stopped: %d\n",
>                  ^
> ./include/linux/device.h:1380:22: note: in definition of macro ‘dev_fmt’
>  #define dev_fmt(fmt) fmt
>                       ^
> drivers/devfreq/devfreq.c:619:3: note: in expansion of macro ‘dev_warn’
>    dev_warn(dev, "%s: Governor %s not stopped: %d\n",
>    ^
> drivers/devfreq/devfreq.c:619:17: warning: format ‘%d’ expects a
> matching ‘int’ argument [-Wformat=]
>    dev_warn(dev, "%s: Governor %s not stopped: %d\n",
>                  ^
> ./include/linux/device.h:1380:22: note: in definition of macro ‘dev_fmt’
>  #define dev_fmt(fmt) fmt
>                       ^
> drivers/devfreq/devfreq.c:619:3: note: in expansion of macro ‘dev_warn’
>    dev_warn(dev, "%s: Governor %s not stopped: %d\n",
>
> 2019년 2월 14일 (목) 오후 7:16, Matthias Kaehlcke <mka@chromium.org>님이 작성:
> >
> > The following pattern is repeated multiple times:
> >
> > - call governor event handler with DEVFREQ_GOV_START/STOP
> > - check return value
> > - in case of error log a message
> >
> > Add devfreq_governor_start/stop() helper functions with this code
> > and call them instead.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  drivers/devfreq/devfreq.c | 96 +++++++++++++++++++++++----------------
> >  1 file changed, 58 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index 7fab6c4cf719b..eb86461648d74 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -580,6 +580,53 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
> >         return ret;
> >  }
> >
> > +/**
> > + * devfreq_governor_start() - Start the devfreq governor of the device.
> > + * @devfreq:   the devfreq instance
> > + */
> > +static int devfreq_governor_start(struct devfreq *devfreq)
> > +{
> > +       struct device *dev = devfreq->dev.parent;
> > +       int err;
> > +
> > +       if (WARN(mutex_is_locked(&devfreq->lock),
> > +                "mutex must *not* be held by the caller\n"))
> > +               return -EINVAL;
> > +
> > +       err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
> > +                                              NULL);
> > +       if (err) {
> > +               dev_err(dev, "Governor %s not started: %d\n",
> > +                       devfreq->governor->name, err);
> > +               return err;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +/**
> > + * devfreq_governor_stop() - Stop the devfreq governor of the device.
> > + * @devfreq:   the devfreq instance
> > + */
> > +static int devfreq_governor_stop(struct devfreq *devfreq)
> > +{
> > +       struct device *dev = devfreq->dev.parent;
> > +       int err;
> > +
> > +       if (WARN(mutex_is_locked(&devfreq->lock),
> > +                "mutex must *not* be held by the caller\n"))
> > +               return -EINVAL;
> > +
> > +       err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_STOP, NULL);
> > +       if (err) {
> > +               dev_warn(dev, "%s: Governor %s not stopped: %d\n",
> > +                        devfreq->governor->name, err);
> > +               return err;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  /**
> >   * devfreq_dev_release() - Callback for struct device to release the device.
> >   * @dev:       the devfreq device
> > @@ -720,13 +767,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
> >         }
> >
> >         devfreq->governor = governor;
> > -       err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
> > -                                               NULL);
> > -       if (err) {
> > -               dev_err(dev, "%s: Unable to start governor for the device\n",
> > -                       __func__);
> > +       err = devfreq_governor_start(devfreq);
> > +       if (err)
> >                 goto err_init;
> > -       }
> >
> >         list_add(&devfreq->node, &devfreq_list);
> >
> > @@ -1029,24 +1072,13 @@ int devfreq_add_governor(struct devfreq_governor *governor)
> >                                 dev_warn(dev,
> >                                          "%s: Governor %s already present\n",
> >                                          __func__, devfreq->governor->name);
> > -                               ret = devfreq->governor->event_handler(devfreq,
> > -                                                       DEVFREQ_GOV_STOP, NULL);
> > -                               if (ret) {
> > -                                       dev_warn(dev,
> > -                                                "%s: Governor %s stop = %d\n",
> > -                                                __func__,
> > -                                                devfreq->governor->name, ret);
> > -                               }
> > +                               ret = devfreq_governor_stop(devfreq);
> > +
> >                                 /* Fall through */
> >                         }
> > +
> >                         devfreq->governor = governor;
> > -                       ret = devfreq->governor->event_handler(devfreq,
> > -                                               DEVFREQ_GOV_START, NULL);
> > -                       if (ret) {
> > -                               dev_warn(dev, "%s: Governor %s start=%d\n",
> > -                                        __func__, devfreq->governor->name,
> > -                                        ret);
> > -                       }
> > +                       devfreq_governor_start(devfreq);
> >                 }
> >         }
> >
> > @@ -1081,7 +1113,6 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
> >                 goto err_out;
> >         }
> >         list_for_each_entry(devfreq, &devfreq_list, node) {
> > -               int ret;
> >                 struct device *dev = devfreq->dev.parent;
> >
> >                 if (!strncmp(devfreq->governor_name, governor->name,
> > @@ -1093,13 +1124,8 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
> >                                 continue;
> >                                 /* Fall through */
> >                         }
> > -                       ret = devfreq->governor->event_handler(devfreq,
> > -                                               DEVFREQ_GOV_STOP, NULL);
> > -                       if (ret) {
> > -                               dev_warn(dev, "%s: Governor %s stop=%d\n",
> > -                                        __func__, devfreq->governor->name,
> > -                                        ret);
> > -                       }
> > +
> > +                       devfreq_governor_stop(devfreq);
> >                         devfreq->governor = NULL;
> >                 }
> >         }
> > @@ -1149,19 +1175,13 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
> >         }
> >
> >         if (df->governor) {
> > -               ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
> > -               if (ret) {
> > -                       dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
> > -                                __func__, df->governor->name, ret);
> > +               ret = devfreq_governor_stop(df);
> > +               if (ret)
> >                         goto out;
> > -               }
> >         }
> >         df->governor = governor;
> >         strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN);
> > -       ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
> > -       if (ret)
> > -               dev_warn(dev, "%s: Governor %s not started(%d)\n",
> > -                        __func__, df->governor->name, ret);
> > +       ret = devfreq_governor_start(df);
> >  out:
> >         mutex_unlock(&devfreq_list_lock);
> >
> > --
> > 2.20.1.791.gb4d0f1c61a-goog
> >
>
>
> --
> Best Regards,
> Chanwoo Choi
Matthias Kaehlcke Feb. 14, 2019, 6:30 p.m. UTC | #3
On Thu, Feb 14, 2019 at 11:12:55PM +0900, Chanwoo Choi wrote:
> Hi Matthias,
> 
> Looks good to me for making the function to remove the duplicate code.
> But,  When I just tested the kernel build, following warnings occur
> about devfreq_governor_stop().
> 
> In file included from ./include/linux/devfreq.h:16:0,
>                  from drivers/devfreq/devfreq.c:23:
> drivers/devfreq/devfreq.c: In function ‘devfreq_governor_stop’:
> drivers/devfreq/devfreq.c:619:17: warning: format ‘%s’ expects
> argument of type ‘char *’, but argument 4 has type ‘int’ [-Wformat=]
>    dev_warn(dev, "%s: Governor %s not stopped: %d\n",
>                  ^
> ./include/linux/device.h:1380:22: note: in definition of macro ‘dev_fmt’
>  #define dev_fmt(fmt) fmt
>                       ^
> drivers/devfreq/devfreq.c:619:3: note: in expansion of macro ‘dev_warn’
>    dev_warn(dev, "%s: Governor %s not stopped: %d\n",
>    ^
> drivers/devfreq/devfreq.c:619:17: warning: format ‘%d’ expects a
> matching ‘int’ argument [-Wformat=]
>    dev_warn(dev, "%s: Governor %s not stopped: %d\n",
>                  ^
> ./include/linux/device.h:1380:22: note: in definition of macro ‘dev_fmt’
>  #define dev_fmt(fmt) fmt
>                       ^
> drivers/devfreq/devfreq.c:619:3: note: in expansion of macro ‘dev_warn’
>    dev_warn(dev, "%s: Governor %s not stopped: %d\n",

For some reason the warnings don't pop up in my 4.19 build and I
missed them when compile testing upstream :(

I'll fix the format string in the next version.

Thanks for the review and build test!

Matthias
Matthias Kaehlcke Feb. 14, 2019, 6:32 p.m. UTC | #4
Hi Chanwoo,

On Thu, Feb 14, 2019 at 11:32:40PM +0900, Chanwoo Choi wrote:
> Hi Matthias,
> 
> When I contributed the something to devfreq.c, if the newly added functions
> are internal/static, just added the function without 'devfreq_' prefix
> in order to distinguish them from the exported function as following:
> - find_available_min_freq()
> - find_available_max_freq()
> - set_freq_table()
> 
> So, the governor_start/stop are the static function used only in devfreq.c,
> in order to sustain the consistency of function naming, I recommened
> that changes them as following:
> - devfreq_governor_start -> governor_start
> - devfreq_governor_stop -> governor_stop

Sounds good, I'll update this in the next version.

Thanks

Matthias

Patch
diff mbox series

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 7fab6c4cf719b..eb86461648d74 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -580,6 +580,53 @@  static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
 	return ret;
 }
 
+/**
+ * devfreq_governor_start() - Start the devfreq governor of the device.
+ * @devfreq:	the devfreq instance
+ */
+static int devfreq_governor_start(struct devfreq *devfreq)
+{
+	struct device *dev = devfreq->dev.parent;
+	int err;
+
+	if (WARN(mutex_is_locked(&devfreq->lock),
+		 "mutex must *not* be held by the caller\n"))
+		return -EINVAL;
+
+	err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
+					       NULL);
+	if (err) {
+		dev_err(dev, "Governor %s not started: %d\n",
+			devfreq->governor->name, err);
+		return err;
+	}
+
+	return 0;
+}
+
+/**
+ * devfreq_governor_stop() - Stop the devfreq governor of the device.
+ * @devfreq:	the devfreq instance
+ */
+static int devfreq_governor_stop(struct devfreq *devfreq)
+{
+	struct device *dev = devfreq->dev.parent;
+	int err;
+
+	if (WARN(mutex_is_locked(&devfreq->lock),
+		 "mutex must *not* be held by the caller\n"))
+		return -EINVAL;
+
+	err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_STOP, NULL);
+	if (err) {
+		dev_warn(dev, "%s: Governor %s not stopped: %d\n",
+			 devfreq->governor->name, err);
+		return err;
+	}
+
+	return 0;
+}
+
 /**
  * devfreq_dev_release() - Callback for struct device to release the device.
  * @dev:	the devfreq device
@@ -720,13 +767,9 @@  struct devfreq *devfreq_add_device(struct device *dev,
 	}
 
 	devfreq->governor = governor;
-	err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
-						NULL);
-	if (err) {
-		dev_err(dev, "%s: Unable to start governor for the device\n",
-			__func__);
+	err = devfreq_governor_start(devfreq);
+	if (err)
 		goto err_init;
-	}
 
 	list_add(&devfreq->node, &devfreq_list);
 
@@ -1029,24 +1072,13 @@  int devfreq_add_governor(struct devfreq_governor *governor)
 				dev_warn(dev,
 					 "%s: Governor %s already present\n",
 					 __func__, devfreq->governor->name);
-				ret = devfreq->governor->event_handler(devfreq,
-							DEVFREQ_GOV_STOP, NULL);
-				if (ret) {
-					dev_warn(dev,
-						 "%s: Governor %s stop = %d\n",
-						 __func__,
-						 devfreq->governor->name, ret);
-				}
+				ret = devfreq_governor_stop(devfreq);
+
 				/* Fall through */
 			}
+
 			devfreq->governor = governor;
-			ret = devfreq->governor->event_handler(devfreq,
-						DEVFREQ_GOV_START, NULL);
-			if (ret) {
-				dev_warn(dev, "%s: Governor %s start=%d\n",
-					 __func__, devfreq->governor->name,
-					 ret);
-			}
+			devfreq_governor_start(devfreq);
 		}
 	}
 
@@ -1081,7 +1113,6 @@  int devfreq_remove_governor(struct devfreq_governor *governor)
 		goto err_out;
 	}
 	list_for_each_entry(devfreq, &devfreq_list, node) {
-		int ret;
 		struct device *dev = devfreq->dev.parent;
 
 		if (!strncmp(devfreq->governor_name, governor->name,
@@ -1093,13 +1124,8 @@  int devfreq_remove_governor(struct devfreq_governor *governor)
 				continue;
 				/* Fall through */
 			}
-			ret = devfreq->governor->event_handler(devfreq,
-						DEVFREQ_GOV_STOP, NULL);
-			if (ret) {
-				dev_warn(dev, "%s: Governor %s stop=%d\n",
-					 __func__, devfreq->governor->name,
-					 ret);
-			}
+
+			devfreq_governor_stop(devfreq);
 			devfreq->governor = NULL;
 		}
 	}
@@ -1149,19 +1175,13 @@  static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	}
 
 	if (df->governor) {
-		ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
-		if (ret) {
-			dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
-				 __func__, df->governor->name, ret);
+		ret = devfreq_governor_stop(df);
+		if (ret)
 			goto out;
-		}
 	}
 	df->governor = governor;
 	strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN);
-	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
-	if (ret)
-		dev_warn(dev, "%s: Governor %s not started(%d)\n",
-			 __func__, df->governor->name, ret);
+	ret = devfreq_governor_start(df);
 out:
 	mutex_unlock(&devfreq_list_lock);