linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] PM / devfreq: Remove redundant governor_name from struct devfreq
       [not found] <CGME20201023101830epcas1p446aee869d0211a62f68cdf616b6203ed@epcas1p4.samsung.com>
@ 2020-10-23 10:32 ` Chanwoo Choi
  2020-10-26  3:20   ` Chanwoo Choi
  0 siblings, 1 reply; 2+ messages in thread
From: Chanwoo Choi @ 2020-10-23 10:32 UTC (permalink / raw)
  To: linux-pm, linux-kernel; +Cc: cw00.choi, chanwoo, myungjoo.ham, kyungmin.park

The devfreq structure instance contains the governor_name and a governor
instance. When need to show the governor name, better to use the name
of devfreq_governor structure. So, governor_name variable in struct devfreq
is a redundant and unneeded variable. Remove the redundant governor_name
of struct devfreq and then use the name of devfreq_governor instance.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
Changes from v1:
- Rebase it on "[PATCH v5 0/2] PM / devfreq: Add governor feature and attribute flag"

 drivers/devfreq/devfreq.c  | 18 +++++++-----------
 drivers/devfreq/governor.h |  2 ++
 include/linux/devfreq.h    |  4 ----
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index f6c55cf498ed..74f71588ace5 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -811,7 +811,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->dev.release = devfreq_dev_release;
 	INIT_LIST_HEAD(&devfreq->node);
 	devfreq->profile = profile;
-	strscpy(devfreq->governor_name, governor_name, DEVFREQ_NAME_LEN);
 	devfreq->previous_freq = profile->initial_freq;
 	devfreq->last_status.current_frequency = profile->initial_freq;
 	devfreq->data = data;
@@ -907,7 +906,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
 	mutex_lock(&devfreq_list_lock);
 
-	governor = try_then_request_governor(devfreq->governor_name);
+	governor = try_then_request_governor(governor_name);
 	if (IS_ERR(governor)) {
 		dev_err(dev, "%s: Unable to find governor for the device\n",
 			__func__);
@@ -1250,7 +1249,7 @@ int devfreq_add_governor(struct devfreq_governor *governor)
 		int ret = 0;
 		struct device *dev = devfreq->dev.parent;
 
-		if (!strncmp(devfreq->governor_name, governor->name,
+		if (!strncmp(devfreq->governor->name, governor->name,
 			     DEVFREQ_NAME_LEN)) {
 			/* The following should never occur */
 			if (devfreq->governor) {
@@ -1312,7 +1311,7 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 		int ret;
 		struct device *dev = devfreq->dev.parent;
 
-		if (!strncmp(devfreq->governor_name, governor->name,
+		if (!strncmp(devfreq->governor->name, governor->name,
 			     DEVFREQ_NAME_LEN)) {
 			/* we should have a devfreq governor! */
 			if (!devfreq->governor) {
@@ -1407,7 +1406,6 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	 */
 	prev_governor = df->governor;
 	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",
@@ -1415,13 +1413,11 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 
 		/* Restore previous governor */
 		df->governor = prev_governor;
-		strncpy(df->governor_name, prev_governor->name,
-			DEVFREQ_NAME_LEN);
 		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
 		if (ret) {
 			dev_err(dev,
 				"%s: reverting to Governor %s failed (%d)\n",
-				__func__, df->governor_name, ret);
+				__func__, prev_governor->name, ret);
 			df->governor = NULL;
 			goto out;
 		}
@@ -1457,7 +1453,7 @@ static ssize_t available_governors_show(struct device *d,
 	 */
 	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)) {
 		count = scnprintf(&buf[count], DEVFREQ_NAME_LEN,
-				  "%s ", df->governor_name);
+				  "%s ", df->governor->name);
 	/*
 	 * The devfreq device shows the registered governor except for
 	 * immutable governors such as passive governor .
@@ -1900,7 +1896,7 @@ static int devfreq_summary_show(struct seq_file *s, void *data)
 
 	list_for_each_entry_reverse(devfreq, &devfreq_list, node) {
 #if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE)
-		if (!strncmp(devfreq->governor_name, DEVFREQ_GOV_PASSIVE,
+		if (!strncmp(devfreq->governor->name, DEVFREQ_GOV_PASSIVE,
 							DEVFREQ_NAME_LEN)) {
 			struct devfreq_passive_data *data = devfreq->data;
 
@@ -1926,7 +1922,7 @@ static int devfreq_summary_show(struct seq_file *s, void *data)
 			"%-30s %-30s %-15s %-10s %10d %12ld %12ld %12ld\n",
 			dev_name(&devfreq->dev),
 			p_devfreq ? dev_name(&p_devfreq->dev) : "null",
-			devfreq->governor_name,
+			devfreq->governor->name,
 			polling_ms ? timer_name[timer] : "null",
 			polling_ms,
 			cur_freq,
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index df413b851bb2..2a52f97b542d 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -13,6 +13,8 @@
 
 #include <linux/devfreq.h>
 
+#define DEVFREQ_NAME_LEN			16
+
 #define to_devfreq(DEV)	container_of((DEV), struct devfreq, dev)
 
 /* Devfreq events */
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 2f4a74efa6be..82156721f459 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -15,8 +15,6 @@
 #include <linux/pm_opp.h>
 #include <linux/pm_qos.h>
 
-#define DEVFREQ_NAME_LEN 16
-
 /* DEVFREQ governor name */
 #define DEVFREQ_GOV_SIMPLE_ONDEMAND	"simple_ondemand"
 #define DEVFREQ_GOV_PERFORMANCE		"performance"
@@ -139,7 +137,6 @@ struct devfreq_stats {
  *		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.
@@ -176,7 +173,6 @@ struct devfreq {
 	struct device dev;
 	struct devfreq_dev_profile *profile;
 	const struct devfreq_governor *governor;
-	char governor_name[DEVFREQ_NAME_LEN];
 	struct notifier_block nb;
 	struct delayed_work work;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] PM / devfreq: Remove redundant governor_name from struct devfreq
  2020-10-23 10:32 ` [PATCH v2] PM / devfreq: Remove redundant governor_name from struct devfreq Chanwoo Choi
@ 2020-10-26  3:20   ` Chanwoo Choi
  0 siblings, 0 replies; 2+ messages in thread
From: Chanwoo Choi @ 2020-10-26  3:20 UTC (permalink / raw)
  To: linux-pm, linux-kernel; +Cc: chanwoo, myungjoo.ham, kyungmin.park

On 10/23/20 7:32 PM, Chanwoo Choi wrote:
> The devfreq structure instance contains the governor_name and a governor
> instance. When need to show the governor name, better to use the name
> of devfreq_governor structure. So, governor_name variable in struct devfreq
> is a redundant and unneeded variable. Remove the redundant governor_name
> of struct devfreq and then use the name of devfreq_governor instance.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> Changes from v1:
> - Rebase it on "[PATCH v5 0/2] PM / devfreq: Add governor feature and attribute flag"
> 
>  drivers/devfreq/devfreq.c  | 18 +++++++-----------
>  drivers/devfreq/governor.h |  2 ++
>  include/linux/devfreq.h    |  4 ----
>  3 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index f6c55cf498ed..74f71588ace5 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -811,7 +811,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  	devfreq->dev.release = devfreq_dev_release;
>  	INIT_LIST_HEAD(&devfreq->node);
>  	devfreq->profile = profile;
> -	strscpy(devfreq->governor_name, governor_name, DEVFREQ_NAME_LEN);
>  	devfreq->previous_freq = profile->initial_freq;
>  	devfreq->last_status.current_frequency = profile->initial_freq;
>  	devfreq->data = data;
> @@ -907,7 +906,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  
>  	mutex_lock(&devfreq_list_lock);
>  
> -	governor = try_then_request_governor(devfreq->governor_name);
> +	governor = try_then_request_governor(governor_name);
>  	if (IS_ERR(governor)) {
>  		dev_err(dev, "%s: Unable to find governor for the device\n",
>  			__func__);
> @@ -1250,7 +1249,7 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>  		int ret = 0;
>  		struct device *dev = devfreq->dev.parent;
>  
> -		if (!strncmp(devfreq->governor_name, governor->name,
> +		if (!strncmp(devfreq->governor->name, governor->name,
>  			     DEVFREQ_NAME_LEN)) {
>  			/* The following should never occur */
>  			if (devfreq->governor) {
> @@ -1312,7 +1311,7 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
>  		int ret;
>  		struct device *dev = devfreq->dev.parent;
>  
> -		if (!strncmp(devfreq->governor_name, governor->name,
> +		if (!strncmp(devfreq->governor->name, governor->name,
>  			     DEVFREQ_NAME_LEN)) {
>  			/* we should have a devfreq governor! */
>  			if (!devfreq->governor) {
> @@ -1407,7 +1406,6 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>  	 */
>  	prev_governor = df->governor;
>  	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",
> @@ -1415,13 +1413,11 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>  
>  		/* Restore previous governor */
>  		df->governor = prev_governor;
> -		strncpy(df->governor_name, prev_governor->name,
> -			DEVFREQ_NAME_LEN);
>  		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
>  		if (ret) {
>  			dev_err(dev,
>  				"%s: reverting to Governor %s failed (%d)\n",
> -				__func__, df->governor_name, ret);
> +				__func__, prev_governor->name, ret);
>  			df->governor = NULL;
>  			goto out;
>  		}
> @@ -1457,7 +1453,7 @@ static ssize_t available_governors_show(struct device *d,
>  	 */
>  	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)) {
>  		count = scnprintf(&buf[count], DEVFREQ_NAME_LEN,
> -				  "%s ", df->governor_name);
> +				  "%s ", df->governor->name);
>  	/*
>  	 * The devfreq device shows the registered governor except for
>  	 * immutable governors such as passive governor .
> @@ -1900,7 +1896,7 @@ static int devfreq_summary_show(struct seq_file *s, void *data)
>  
>  	list_for_each_entry_reverse(devfreq, &devfreq_list, node) {
>  #if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE)
> -		if (!strncmp(devfreq->governor_name, DEVFREQ_GOV_PASSIVE,
> +		if (!strncmp(devfreq->governor->name, DEVFREQ_GOV_PASSIVE,
>  							DEVFREQ_NAME_LEN)) {
>  			struct devfreq_passive_data *data = devfreq->data;
>  
> @@ -1926,7 +1922,7 @@ static int devfreq_summary_show(struct seq_file *s, void *data)
>  			"%-30s %-30s %-15s %-10s %10d %12ld %12ld %12ld\n",
>  			dev_name(&devfreq->dev),
>  			p_devfreq ? dev_name(&p_devfreq->dev) : "null",
> -			devfreq->governor_name,
> +			devfreq->governor->name,
>  			polling_ms ? timer_name[timer] : "null",
>  			polling_ms,
>  			cur_freq,
> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
> index df413b851bb2..2a52f97b542d 100644
> --- a/drivers/devfreq/governor.h
> +++ b/drivers/devfreq/governor.h
> @@ -13,6 +13,8 @@
>  
>  #include <linux/devfreq.h>
>  
> +#define DEVFREQ_NAME_LEN			16
> +
>  #define to_devfreq(DEV)	container_of((DEV), struct devfreq, dev)
>  
>  /* Devfreq events */
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 2f4a74efa6be..82156721f459 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -15,8 +15,6 @@
>  #include <linux/pm_opp.h>
>  #include <linux/pm_qos.h>
>  
> -#define DEVFREQ_NAME_LEN 16
> -
>  /* DEVFREQ governor name */
>  #define DEVFREQ_GOV_SIMPLE_ONDEMAND	"simple_ondemand"
>  #define DEVFREQ_GOV_PERFORMANCE		"performance"
> @@ -139,7 +137,6 @@ struct devfreq_stats {
>   *		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.
> @@ -176,7 +173,6 @@ struct devfreq {
>  	struct device dev;
>  	struct devfreq_dev_profile *profile;
>  	const struct devfreq_governor *governor;
> -	char governor_name[DEVFREQ_NAME_LEN];
>  	struct notifier_block nb;
>  	struct delayed_work work;
>  
> 

Applied it.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-10-26  3:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20201023101830epcas1p446aee869d0211a62f68cdf616b6203ed@epcas1p4.samsung.com>
2020-10-23 10:32 ` [PATCH v2] PM / devfreq: Remove redundant governor_name from struct devfreq Chanwoo Choi
2020-10-26  3:20   ` Chanwoo Choi

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).