[PATCH v4 7/8] PM / devfreq: Define the constant governor name

public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed

* [PATCH v4 7/8] PM / devfreq: Define the constant governor name
       [not found] ` <1507880904-31956-1-git-send-email-cw00.choi@samsung.com>
@ 2017-10-13  7:48   ` Chanwoo Choi
  2017-10-17 15:02     ` MyungJoo Ham
  2017-10-13  7:48   ` [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device Chanwoo Choi
  1 sibling, 1 reply; 5+ messages in thread
From: Chanwoo Choi @ 2017-10-13  7:48 UTC (permalink / raw)
  To: linux-arm-kernel

Prior to that, the devfreq device uses the governor name when adding
the itself. In order to prevent the mistake used the wrong governor name,
this patch defines the governor name as a constant and then uses them
instead of using the string directly.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung•com>
Cc: Kukjin Kim <kgene@kernel•org>
Cc: Krzysztof Kozlowski <krzk@kernel•org>
Cc: linux-samsung-soc at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 drivers/devfreq/exynos-bus.c              | 5 +++--
 drivers/devfreq/governor_passive.c        | 2 +-
 drivers/devfreq/governor_performance.c    | 2 +-
 drivers/devfreq/governor_powersave.c      | 2 +-
 drivers/devfreq/governor_simpleondemand.c | 2 +-
 drivers/devfreq/governor_userspace.c      | 2 +-
 drivers/devfreq/rk3399_dmc.c              | 2 +-
 include/linux/devfreq.h                   | 7 +++++++
 8 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 49f68929e024..c25658b26598 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -436,7 +436,8 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	ondemand_data->downdifferential = 5;
 
 	/* Add devfreq device to monitor and handle the exynos bus */
-	bus->devfreq = devm_devfreq_add_device(dev, profile, "simple_ondemand",
+	bus->devfreq = devm_devfreq_add_device(dev, profile,
+						DEVFREQ_GOV_SIMPLE_ONDEMAND,
 						ondemand_data);
 	if (IS_ERR(bus->devfreq)) {
 		dev_err(dev, "failed to add devfreq device\n");
@@ -488,7 +489,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	passive_data->parent = parent_devfreq;
 
 	/* Add devfreq device for exynos bus with passive governor */
-	bus->devfreq = devm_devfreq_add_device(dev, profile, "passive",
+	bus->devfreq = devm_devfreq_add_device(dev, profile, DEVFREQ_GOV_PASSIVE,
 						passive_data);
 	if (IS_ERR(bus->devfreq)) {
 		dev_err(dev,
diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index 673ad8cc9a1d..3bc29acbd54e 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -183,7 +183,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_passive = {
-	.name = "passive",
+	.name = DEVFREQ_GOV_PASSIVE,
 	.immutable = 1,
 	.get_target_freq = devfreq_passive_get_target_freq,
 	.event_handler = devfreq_passive_event_handler,
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
index c72f942f30a8..4d23ecfbd948 100644
--- a/drivers/devfreq/governor_performance.c
+++ b/drivers/devfreq/governor_performance.c
@@ -42,7 +42,7 @@ static int devfreq_performance_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_performance = {
-	.name = "performance",
+	.name = DEVFREQ_GOV_PERFORMANCE,
 	.get_target_freq = devfreq_performance_func,
 	.event_handler = devfreq_performance_handler,
 };
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
index 0c6bed567e6d..0c42f23249ef 100644
--- a/drivers/devfreq/governor_powersave.c
+++ b/drivers/devfreq/governor_powersave.c
@@ -39,7 +39,7 @@ static int devfreq_powersave_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_powersave = {
-	.name = "powersave",
+	.name = DEVFREQ_GOV_POWERSAVE,
 	.get_target_freq = devfreq_powersave_func,
 	.event_handler = devfreq_powersave_handler,
 };
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index ae72ba5e78df..28e0f2de7100 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -125,7 +125,7 @@ static int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_simple_ondemand = {
-	.name = "simple_ondemand",
+	.name = DEVFREQ_GOV_SIMPLE_ONDEMAND,
 	.get_target_freq = devfreq_simple_ondemand_func,
 	.event_handler = devfreq_simple_ondemand_handler,
 };
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index 77028c27593c..080607c3f34d 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -87,7 +87,7 @@ static ssize_t show_freq(struct device *dev, struct device_attribute *attr,
 	NULL,
 };
 static const struct attribute_group dev_attr_group = {
-	.name	= "userspace",
+	.name	= DEVFREQ_GOV_USERSPACE,
 	.attrs	= dev_entries,
 };
 
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 1b89ebbad02c..5dfbfa3cc878 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -431,7 +431,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 
 	data->devfreq = devm_devfreq_add_device(dev,
 					   &rk3399_devfreq_dmc_profile,
-					   "simple_ondemand",
+					   DEVFREQ_GOV_SIMPLE_ONDEMAND,
 					   &data->ondemand_data);
 	if (IS_ERR(data->devfreq))
 		return PTR_ERR(data->devfreq);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 5dc6190e479c..682419a32aa3 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -19,6 +19,13 @@
 
 #define DEVFREQ_NAME_LEN 16
 
+/* DEVFREQ governor name */
+#define DEVFREQ_GOV_SIMPLE_ONDEMAND	"simple_ondemand"
+#define DEVFREQ_GOV_PERFORMANCE		"performance"
+#define DEVFREQ_GOV_POWERSAVE		"powersave"
+#define DEVFREQ_GOV_USERSPACE		"userspace"
+#define DEVFREQ_GOV_PASSIVE		"passive"
+
 /* DEVFREQ notifier interface */
 #define DEVFREQ_TRANSITION_NOTIFIER	(0)
 
-- 
1.9.1

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

* [PATCH v4 7/8] PM / devfreq: Define the constant governor name
  2017-10-13  7:48   ` [PATCH v4 7/8] PM / devfreq: Define the constant governor name Chanwoo Choi
@ 2017-10-17 15:02     ` MyungJoo Ham
  0 siblings, 0 replies; 5+ messages in thread
From: MyungJoo Ham @ 2017-10-17 15:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung•com> wrote:
> Prior to that, the devfreq device uses the governor name when adding
> the itself. In order to prevent the mistake used the wrong governor name,
> this patch defines the governor name as a constant and then uses them
> instead of using the string directly.
>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung•com>
> Cc: Kukjin Kim <kgene@kernel•org>
> Cc: Krzysztof Kozlowski <krzk@kernel•org>
> Cc: linux-samsung-soc at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org

Acked-by: MyungJoo Ham <myungjoo.ham@samsung•com>

> ---
>  drivers/devfreq/exynos-bus.c              | 5 +++--
>  drivers/devfreq/governor_passive.c        | 2 +-
>  drivers/devfreq/governor_performance.c    | 2 +-
>  drivers/devfreq/governor_powersave.c      | 2 +-
>  drivers/devfreq/governor_simpleondemand.c | 2 +-
>  drivers/devfreq/governor_userspace.c      | 2 +-
>  drivers/devfreq/rk3399_dmc.c              | 2 +-
>  include/linux/devfreq.h                   | 7 +++++++
>  8 files changed, 16 insertions(+), 8 deletions(-)
>
[]


-- 
MyungJoo Ham, Ph.D.
S/W Center, Samsung Electronics

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

* [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device
       [not found] ` <1507880904-31956-1-git-send-email-cw00.choi@samsung.com>
  2017-10-13  7:48   ` [PATCH v4 7/8] PM / devfreq: Define the constant governor name Chanwoo Choi
@ 2017-10-13  7:48   ` Chanwoo Choi
  2017-10-17 15:11     ` MyungJoo Ham
  1 sibling, 1 reply; 5+ messages in thread
From: Chanwoo Choi @ 2017-10-13  7:48 UTC (permalink / raw)
  To: linux-arm-kernel

This patch registers the Exynos Bus-Frequency scaling device
as a cooling device of thermal management.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung•com>
Cc: Kukjin Kim <kgene@kernel•org>
Cc: Krzysztof Kozlowski <krzk@kernel•org>
Cc: linux-samsung-soc at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 drivers/devfreq/exynos-bus.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index c25658b26598..1c7521b65c2f 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -15,6 +15,7 @@
 #include <linux/clk.h>
 #include <linux/devfreq.h>
 #include <linux/devfreq-event.h>
+#include <linux/devfreq_cooling.h>
 #include <linux/device.h>
 #include <linux/export.h>
 #include <linux/module.h>
@@ -41,6 +42,8 @@ struct exynos_bus {
 	struct clk *clk;
 	unsigned int voltage_tolerance;
 	unsigned int ratio;
+
+	struct thermal_cooling_device *cdev;
 };
 
 /*
@@ -468,6 +471,19 @@ static int exynos_bus_probe(struct platform_device *pdev)
 		goto err;
 	}
 
+	/*
+	 * Register devfreq cooling device if thermal DT code
+	 * takes care of matching them.
+	 */
+	if (of_find_property(np, "#cooling-cells", NULL)) {
+		bus->cdev = of_devfreq_cooling_register(np, bus->devfreq);
+		if (IS_ERR(bus->cdev)) {
+			dev_err(dev, "running exynos-bus without cooling device\n");
+			bus->cdev = NULL;
+		}
+	}
+	of_node_put(np);
+
 	goto out;
 passive:
 	/* Initialize the struct profile and governor data for passive device */
@@ -514,6 +530,16 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static int exynos_bus_remove(struct platform_device *pdev)
+{
+	struct exynos_bus *bus = platform_get_drvdata(pdev);
+
+	if (bus->cdev)
+		devfreq_cooling_unregister(bus->cdev);
+
+	return 0;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int exynos_bus_resume(struct device *dev)
 {
@@ -556,6 +582,7 @@ static int exynos_bus_suspend(struct device *dev)
 
 static struct platform_driver exynos_bus_platdrv = {
 	.probe		= exynos_bus_probe,
+	.remove		= exynos_bus_remove,
 	.driver = {
 		.name	= "exynos-bus",
 		.pm	= &exynos_bus_pm,
-- 
1.9.1

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

* [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device
  2017-10-13  7:48   ` [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device Chanwoo Choi
@ 2017-10-17 15:11     ` MyungJoo Ham
  2017-10-18  2:08       ` Chanwoo Choi
  0 siblings, 1 reply; 5+ messages in thread
From: MyungJoo Ham @ 2017-10-17 15:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung•com> wrote:
> This patch registers the Exynos Bus-Frequency scaling device
> as a cooling device of thermal management.
>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung•com>
> Cc: Kukjin Kim <kgene@kernel•org>
> Cc: Krzysztof Kozlowski <krzk@kernel•org>
> Cc: linux-samsung-soc at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org

I've got a question below.

> ---
>  drivers/devfreq/exynos-bus.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index c25658b26598..1c7521b65c2f 100644
[]> @@ -468,6 +471,19 @@ static int exynos_bus_probe(struct
platform_device *pdev)
>                 goto err;
>         }
>
> +       /*
> +        * Register devfreq cooling device if thermal DT code
> +        * takes care of matching them.
> +        */
> +       if (of_find_property(np, "#cooling-cells", NULL)) {
> +               bus->cdev = of_devfreq_cooling_register(np, bus->devfreq);
> +               if (IS_ERR(bus->cdev)) {
> +                       dev_err(dev, "running exynos-bus without cooling device\n");
> +                       bus->cdev = NULL;
> +               }
> +       }
> +       of_node_put(np);

Is this of_node_put() is a pair of of_find_property? or for something else?
(do you need to call put for of_find_property? or for something else?
I'm not seeing a function with "get")


Cheers,
MyungJoo

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

* [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device
  2017-10-17 15:11     ` MyungJoo Ham
@ 2017-10-18  2:08       ` Chanwoo Choi
  0 siblings, 0 replies; 5+ messages in thread
From: Chanwoo Choi @ 2017-10-18  2:08 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 2017? 10? 18? 00:11, MyungJoo Ham wrote:
> On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung•com> wrote:
>> This patch registers the Exynos Bus-Frequency scaling device
>> as a cooling device of thermal management.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung•com>
>> Cc: Kukjin Kim <kgene@kernel•org>
>> Cc: Krzysztof Kozlowski <krzk@kernel•org>
>> Cc: linux-samsung-soc at vger.kernel.org
>> Cc: linux-arm-kernel at lists.infradead.org
>> Cc: linux-kernel at vger.kernel.org
> 
> I've got a question below.
> 
>> ---
>>  drivers/devfreq/exynos-bus.c | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>> index c25658b26598..1c7521b65c2f 100644
> []> @@ -468,6 +471,19 @@ static int exynos_bus_probe(struct
> platform_device *pdev)
>>                 goto err;
>>         }
>>
>> +       /*
>> +        * Register devfreq cooling device if thermal DT code
>> +        * takes care of matching them.
>> +        */
>> +       if (of_find_property(np, "#cooling-cells", NULL)) {
>> +               bus->cdev = of_devfreq_cooling_register(np, bus->devfreq);
>> +               if (IS_ERR(bus->cdev)) {
>> +                       dev_err(dev, "running exynos-bus without cooling device\n");
>> +                       bus->cdev = NULL;
>> +               }
>> +       }
>> +       of_node_put(np);
> 
> Is this of_node_put() is a pair of of_find_property? or for something else?
> (do you need to call put for of_find_property? or for something else?
> I'm not seeing a function with "get")

You're right. The of_node_put(np) call is unneeded.
Actually, the extcon-bus.c have to use the of_node_get instead of accessing
the 'dev->of_node' directly.

And, when I test this patch, I got a bug related to passive governor.
I need more time for debugging and redevelopment.
So, I'll drop this patch on next patchset (v5).

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

end of thread, other threads:[~2017-10-18  2:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20171013074836epcas2p4e70bca2ebe9446353abf5c14a9d16460@epcas2p4.samsung.com>
     [not found] ` <1507880904-31956-1-git-send-email-cw00.choi@samsung.com>
2017-10-13  7:48   ` [PATCH v4 7/8] PM / devfreq: Define the constant governor name Chanwoo Choi
2017-10-17 15:02     ` MyungJoo Ham
2017-10-13  7:48   ` [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device Chanwoo Choi
2017-10-17 15:11     ` MyungJoo Ham
2017-10-18  2:08       ` Chanwoo Choi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox