2019-10-12 06:53:50

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 01/11] thermal: Move default governor config option to the internal header

The default governor set at compilation time is a thermal internal
business, no need to export to the global thermal header.

Move the config options to the internal header.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 11 +++++++++++
include/linux/thermal.h | 11 -----------
2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 207b0cda70da..f1206d67047f 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -12,6 +12,17 @@
#include <linux/device.h>
#include <linux/thermal.h>

+/* Default Thermal Governor */
+#if defined(CONFIG_THERMAL_DEFAULT_GOV_STEP_WISE)
+#define DEFAULT_THERMAL_GOVERNOR "step_wise"
+#elif defined(CONFIG_THERMAL_DEFAULT_GOV_FAIR_SHARE)
+#define DEFAULT_THERMAL_GOVERNOR "fair_share"
+#elif defined(CONFIG_THERMAL_DEFAULT_GOV_USER_SPACE)
+#define DEFAULT_THERMAL_GOVERNOR "user_space"
+#elif defined(CONFIG_THERMAL_DEFAULT_GOV_POWER_ALLOCATOR)
+#define DEFAULT_THERMAL_GOVERNOR "power_allocator"
+#endif
+
/* Initial state of a cooling device during binding */
#define THERMAL_NO_TARGET -1UL

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index e45659c75920..a389d4621814 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -43,17 +43,6 @@
#define MILLICELSIUS_TO_DECI_KELVIN_WITH_OFFSET(t, off) (((t) / 100) + (off))
#define MILLICELSIUS_TO_DECI_KELVIN(t) MILLICELSIUS_TO_DECI_KELVIN_WITH_OFFSET(t, 2732)

-/* Default Thermal Governor */
-#if defined(CONFIG_THERMAL_DEFAULT_GOV_STEP_WISE)
-#define DEFAULT_THERMAL_GOVERNOR "step_wise"
-#elif defined(CONFIG_THERMAL_DEFAULT_GOV_FAIR_SHARE)
-#define DEFAULT_THERMAL_GOVERNOR "fair_share"
-#elif defined(CONFIG_THERMAL_DEFAULT_GOV_USER_SPACE)
-#define DEFAULT_THERMAL_GOVERNOR "user_space"
-#elif defined(CONFIG_THERMAL_DEFAULT_GOV_POWER_ALLOCATOR)
-#define DEFAULT_THERMAL_GOVERNOR "power_allocator"
-#endif
-
struct thermal_zone_device;
struct thermal_cooling_device;
struct thermal_instance;
--
2.17.1


2019-10-12 06:53:51

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 02/11] thermal: Move struct thermal_attr to the private header

The structure belongs to the thermal core internals but it is exported
in the include/linux/thermal.h

For better self-encapsulation and less impact for the compilation if a
change is made on it. Move the structure in the thermal core internal
header file.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 5 +++++
include/linux/thermal.h | 6 +-----
2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index f1206d67047f..0e964240524d 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -41,6 +41,11 @@ extern struct thermal_governor *__governor_thermal_table_end[];
__governor < __governor_thermal_table_end; \
__governor++)

+struct thermal_attr {
+ struct device_attribute attr;
+ char name[THERMAL_NAME_LENGTH];
+};
+
/*
* This structure is used to describe the behavior of
* a certain cooling device on a certain trip point
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a389d4621814..603766a4068c 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -46,6 +46,7 @@
struct thermal_zone_device;
struct thermal_cooling_device;
struct thermal_instance;
+struct thermal_attr;

enum thermal_device_mode {
THERMAL_DEVICE_DISABLED = 0,
@@ -130,11 +131,6 @@ struct thermal_cooling_device {
struct list_head node;
};

-struct thermal_attr {
- struct device_attribute attr;
- char name[THERMAL_NAME_LENGTH];
-};
-
/**
* struct thermal_zone_device - structure for a thermal zone
* @id: unique id number for each thermal zone
--
2.17.1

2019-10-12 06:53:57

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 05/11] thermal: Move set_trips function to the internal header

The function is not used in other place than the thermal directory. It
does not make sense to export its definition in the global header as
there is no use of it.

Move its the definition in the internal header and allow better
self-encapsulation.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 2 ++
include/linux/thermal.h | 3 ---
2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 6f6e0dcba4f2..301f5603def1 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -72,6 +72,8 @@ struct thermal_trip {
enum thermal_trip_type type;
};

+void thermal_zone_set_trips(struct thermal_zone_device *tz);
+
/*
* This structure is used to describe the behavior of
* a certain cooling device on a certain trip point
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 88e1faa3d72c..761d77571533 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -398,7 +398,6 @@ int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int,
struct thermal_cooling_device *);
void thermal_zone_device_update(struct thermal_zone_device *,
enum thermal_notify_event);
-void thermal_zone_set_trips(struct thermal_zone_device *);

struct thermal_cooling_device *thermal_cooling_device_register(const char *,
void *, const struct thermal_cooling_device_ops *);
@@ -444,8 +443,6 @@ static inline int thermal_zone_unbind_cooling_device(
static inline void thermal_zone_device_update(struct thermal_zone_device *tz,
enum thermal_notify_event event)
{ }
-static inline void thermal_zone_set_trips(struct thermal_zone_device *tz)
-{ }
static inline struct thermal_cooling_device *
thermal_cooling_device_register(char *type, void *devdata,
const struct thermal_cooling_device_ops *ops)
--
2.17.1

2019-10-12 06:54:09

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 08/11] thermal: Change IS_ENABLED to IFDEF in the header file

The thermal framework can not be compiled as a module. The IS_ENABLED
macro is useless here and can be replaced by an ifdef.

Signed-off-by: Daniel Lezcano <[email protected]>
---
include/linux/thermal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 4436addc0e83..d77baa523093 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -384,7 +384,7 @@ void devm_thermal_zone_of_sensor_unregister(struct device *dev,

#endif

-#if IS_ENABLED(CONFIG_THERMAL)
+#ifdef CONFIG_THERMAL
struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
void *, struct thermal_zone_device_ops *,
struct thermal_zone_params *, int, int);
--
2.17.1

2019-10-12 06:54:14

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 11/11] thermal: Move thermal governor structure to internal header

The thermal governor structure is a big structure where no
user should change value inside except via helper functions.

Move the structure to the internal header thus preventing external
code to be tempted by hacking the structure's variables.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 20 ++++++++++++++++++++
include/linux/thermal.h | 21 +--------------------
2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index c75309d858ce..e54150fa4c5b 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -46,6 +46,26 @@ struct thermal_attr {
char name[THERMAL_NAME_LENGTH];
};

+/**
+ * struct thermal_governor - structure that holds thermal governor information
+ * @name: name of the governor
+ * @bind_to_tz: callback called when binding to a thermal zone. If it
+ * returns 0, the governor is bound to the thermal zone,
+ * otherwise it fails.
+ * @unbind_from_tz: callback called when a governor is unbound from a
+ * thermal zone.
+ * @throttle: callback called for every trip point even if temperature is
+ * below the trip point temperature
+ * @governor_list: node in thermal_governor_list (in thermal_core.c)
+ */
+struct thermal_governor {
+ char name[THERMAL_NAME_LENGTH];
+ int (*bind_to_tz)(struct thermal_zone_device *tz);
+ void (*unbind_from_tz)(struct thermal_zone_device *tz);
+ int (*throttle)(struct thermal_zone_device *tz, int trip);
+ struct list_head governor_list;
+};
+
static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
{
return cdev->ops->get_requested_power && cdev->ops->state2power &&
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 8daa179918a1..04264e8a2bce 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -45,6 +45,7 @@

struct thermal_zone_device;
struct thermal_cooling_device;
+struct thermal_governor;
struct thermal_instance;
struct thermal_attr;

@@ -206,26 +207,6 @@ struct thermal_zone_device {
enum thermal_notify_event notify_event;
};

-/**
- * struct thermal_governor - structure that holds thermal governor information
- * @name: name of the governor
- * @bind_to_tz: callback called when binding to a thermal zone. If it
- * returns 0, the governor is bound to the thermal zone,
- * otherwise it fails.
- * @unbind_from_tz: callback called when a governor is unbound from a
- * thermal zone.
- * @throttle: callback called for every trip point even if temperature is
- * below the trip point temperature
- * @governor_list: node in thermal_governor_list (in thermal_core.c)
- */
-struct thermal_governor {
- char name[THERMAL_NAME_LENGTH];
- int (*bind_to_tz)(struct thermal_zone_device *tz);
- void (*unbind_from_tz)(struct thermal_zone_device *tz);
- int (*throttle)(struct thermal_zone_device *tz, int trip);
- struct list_head governor_list;
-};
-
/* Structure that holds binding parameters for a zone */
struct thermal_bind_params {
struct thermal_cooling_device *cdev;
--
2.17.1

2019-10-12 06:54:32

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 10/11] thermal: Remove thermal_zone_device_update() stub

All users of the function depends on THERMAL, no stub is
needed. Remove it.

Signed-off-by: Daniel Lezcano <[email protected]>
---
include/linux/thermal.h | 3 ---
1 file changed, 3 deletions(-)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index ae72fe771e07..8daa179918a1 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -427,9 +427,6 @@ static inline struct thermal_zone_device *thermal_zone_device_register(
static inline void thermal_zone_device_unregister(
struct thermal_zone_device *tz)
{ }
-static inline void thermal_zone_device_update(struct thermal_zone_device *tz,
- enum thermal_notify_event event)
-{ }
static inline struct thermal_cooling_device *
thermal_cooling_device_register(char *type, void *devdata,
const struct thermal_cooling_device_ops *ops)
--
2.17.1

2019-10-12 06:55:00

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 06/11] thermal: Move get_tz_trend to the internal header

The function is not used in other place than the thermal directory. It
does not make sense to export its definition in the global header as
there is no use of it.

Move its the definition in the internal header and allow better
self-encapsulation.

Take the opportunity to add the parameter names to make checkpatch
happy and remove the pointless stubs.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 2 ++
include/linux/thermal.h | 4 +---
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 301f5603def1..c03a07164ca7 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -74,6 +74,8 @@ struct thermal_trip {

void thermal_zone_set_trips(struct thermal_zone_device *tz);

+int get_tz_trend(struct thermal_zone_device *tz, int trip);
+
/*
* This structure is used to describe the behavior of
* a certain cooling device on a certain trip point
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 761d77571533..a87f551d114d 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -415,7 +415,6 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
int thermal_zone_get_slope(struct thermal_zone_device *tz);
int thermal_zone_get_offset(struct thermal_zone_device *tz);

-int get_tz_trend(struct thermal_zone_device *, int);
struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
struct thermal_cooling_device *, int);
void thermal_cdev_update(struct thermal_cooling_device *);
@@ -474,8 +473,7 @@ static inline int thermal_zone_get_slope(
static inline int thermal_zone_get_offset(
struct thermal_zone_device *tz)
{ return -ENODEV; }
-static inline int get_tz_trend(struct thermal_zone_device *tz, int trip)
-{ return -ENODEV; }
+
static inline struct thermal_instance *
get_thermal_instance(struct thermal_zone_device *tz,
struct thermal_cooling_device *cdev, int trip)
--
2.17.1

2019-10-12 06:55:33

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 07/11] thermal: Move get_thermal_instance to the internal header

The function is not used in other place than the thermal directory. It
does not make sense to export its definition in the global header as
there is no use of it.

Move its the definition in the internal header and allow better
self-encapsulation.

Take the opportunity to add the parameter names to make checkpatch
happy and remove the pointless stubs.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 5 +++++
include/linux/thermal.h | 6 ------
2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index c03a07164ca7..c75309d858ce 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -76,6 +76,11 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz);

int get_tz_trend(struct thermal_zone_device *tz, int trip);

+struct thermal_instance *
+get_thermal_instance(struct thermal_zone_device *tz,
+ struct thermal_cooling_device *cdev,
+ int trip);
+
/*
* This structure is used to describe the behavior of
* a certain cooling device on a certain trip point
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a87f551d114d..4436addc0e83 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -415,8 +415,6 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
int thermal_zone_get_slope(struct thermal_zone_device *tz);
int thermal_zone_get_offset(struct thermal_zone_device *tz);

-struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
- struct thermal_cooling_device *, int);
void thermal_cdev_update(struct thermal_cooling_device *);
void thermal_notify_framework(struct thermal_zone_device *, int);
#else
@@ -474,10 +472,6 @@ static inline int thermal_zone_get_offset(
struct thermal_zone_device *tz)
{ return -ENODEV; }

-static inline struct thermal_instance *
-get_thermal_instance(struct thermal_zone_device *tz,
- struct thermal_cooling_device *cdev, int trip)
-{ return ERR_PTR(-ENODEV); }
static inline void thermal_cdev_update(struct thermal_cooling_device *cdev)
{ }
static inline void thermal_notify_framework(struct thermal_zone_device *tz,
--
2.17.1

2019-10-12 06:56:33

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 04/11] thermal: Move trip point structure definition to private header

The struct thermal_trip is only used by the thermal internals, it is
pointless to export the definition in the global header.

Move the structure to the thermal_core.h internal header.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 13 +++++++++++++
include/linux/thermal.h | 15 ---------------
2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 5953bb527ec2..6f6e0dcba4f2 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -58,6 +58,19 @@ int power_actor_get_min_power(struct thermal_cooling_device *cdev,
struct thermal_zone_device *tz, u32 *min_power);
int power_actor_set_power(struct thermal_cooling_device *cdev,
struct thermal_instance *ti, u32 power);
+/**
+ * struct thermal_trip - representation of a point in temperature domain
+ * @np: pointer to struct device_node that this trip point was created from
+ * @temperature: temperature value in miliCelsius
+ * @hysteresis: relative hysteresis in miliCelsius
+ * @type: trip point type
+ */
+struct thermal_trip {
+ struct device_node *np;
+ int temperature;
+ int hysteresis;
+ enum thermal_trip_type type;
+};

/*
* This structure is used to describe the behavior of
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index fae9ff2b079a..88e1faa3d72c 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -343,21 +343,6 @@ struct thermal_zone_of_device_ops {
int (*set_trip_temp)(void *, int, int);
};

-/**
- * struct thermal_trip - representation of a point in temperature domain
- * @np: pointer to struct device_node that this trip point was created from
- * @temperature: temperature value in miliCelsius
- * @hysteresis: relative hysteresis in miliCelsius
- * @type: trip point type
- */
-
-struct thermal_trip {
- struct device_node *np;
- int temperature;
- int hysteresis;
- enum thermal_trip_type type;
-};
-
/* Function declarations */
#ifdef CONFIG_THERMAL_OF
struct thermal_zone_device *
--
2.17.1

2019-10-12 06:57:04

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 09/11] thermal: Remove stubs for thermal_zone_[un]bind_cooling_device

All callers of the functions depends on THERMAL, it is pointless to
define stubs. Remove them.

Signed-off-by: Daniel Lezcano <[email protected]>
---
include/linux/thermal.h | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index d77baa523093..ae72fe771e07 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -427,16 +427,6 @@ static inline struct thermal_zone_device *thermal_zone_device_register(
static inline void thermal_zone_device_unregister(
struct thermal_zone_device *tz)
{ }
-static inline int thermal_zone_bind_cooling_device(
- struct thermal_zone_device *tz, int trip,
- struct thermal_cooling_device *cdev,
- unsigned long upper, unsigned long lower,
- unsigned int weight)
-{ return -ENODEV; }
-static inline int thermal_zone_unbind_cooling_device(
- struct thermal_zone_device *tz, int trip,
- struct thermal_cooling_device *cdev)
-{ return -ENODEV; }
static inline void thermal_zone_device_update(struct thermal_zone_device *tz,
enum thermal_notify_event event)
{ }
--
2.17.1

2019-10-12 06:57:38

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 03/11] thermal: Move internal IPA functions

The exported IPA functions are used by the IPA. It is pointless to
declare the functions in the thermal.h file.

For better self-encapsulation and less impact for the compilation if a
change is made on it. Move the code in the thermal core internal
header file.

As the users depends on THERMAL then it is pointless to have the stub,
remove them.

Take also the opportunity to fix checkpatch warnings/errors when
moving the code around.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.h | 13 +++++++++++++
include/linux/thermal.h | 24 ------------------------
2 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 0e964240524d..5953bb527ec2 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -46,6 +46,19 @@ struct thermal_attr {
char name[THERMAL_NAME_LENGTH];
};

+static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
+{
+ return cdev->ops->get_requested_power && cdev->ops->state2power &&
+ cdev->ops->power2state;
+}
+
+int power_actor_get_max_power(struct thermal_cooling_device *cdev,
+ struct thermal_zone_device *tz, u32 *max_power);
+int power_actor_get_min_power(struct thermal_cooling_device *cdev,
+ struct thermal_zone_device *tz, u32 *min_power);
+int power_actor_set_power(struct thermal_cooling_device *cdev,
+ struct thermal_instance *ti, u32 power);
+
/*
* This structure is used to describe the behavior of
* a certain cooling device on a certain trip point
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 603766a4068c..fae9ff2b079a 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -400,18 +400,6 @@ void devm_thermal_zone_of_sensor_unregister(struct device *dev,
#endif

#if IS_ENABLED(CONFIG_THERMAL)
-static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
-{
- return cdev->ops->get_requested_power && cdev->ops->state2power &&
- cdev->ops->power2state;
-}
-
-int power_actor_get_max_power(struct thermal_cooling_device *,
- struct thermal_zone_device *tz, u32 *max_power);
-int power_actor_get_min_power(struct thermal_cooling_device *,
- struct thermal_zone_device *tz, u32 *min_power);
-int power_actor_set_power(struct thermal_cooling_device *,
- struct thermal_instance *, u32);
struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
void *, struct thermal_zone_device_ops *,
struct thermal_zone_params *, int, int);
@@ -449,18 +437,6 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
void thermal_cdev_update(struct thermal_cooling_device *);
void thermal_notify_framework(struct thermal_zone_device *, int);
#else
-static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
-{ return false; }
-static inline int power_actor_get_max_power(struct thermal_cooling_device *cdev,
- struct thermal_zone_device *tz, u32 *max_power)
-{ return 0; }
-static inline int power_actor_get_min_power(struct thermal_cooling_device *cdev,
- struct thermal_zone_device *tz,
- u32 *min_power)
-{ return -ENODEV; }
-static inline int power_actor_set_power(struct thermal_cooling_device *cdev,
- struct thermal_instance *tz, u32 power)
-{ return 0; }
static inline struct thermal_zone_device *thermal_zone_device_register(
const char *type, int trips, int mask, void *devdata,
struct thermal_zone_device_ops *ops,
--
2.17.1

2019-10-14 07:54:03

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 11/11] thermal: Move thermal governor structure to internal header

Hi Daniel,

I love your patch! Yet something to improve:

[auto build test ERROR on soc-thermal/next]
[cannot apply to v5.4-rc3 next-20191011]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Daniel-Lezcano/thermal-Move-default-governor-config-option-to-the-internal-header/20191014-103735
base: https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git next
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/platform/x86/acerhdf.c: In function 'acerhdf_register_thermal':
>> drivers/platform/x86/acerhdf.c:747:30: error: dereferencing pointer to incomplete type 'struct thermal_governor'
if (strcmp(thz_dev->governor->name,
^~

vim +747 drivers/platform/x86/acerhdf.c

e86435eb91b2bf Peter Feuerer 2009-06-21 731
1d0c3fd01afbd5 Paul Gortmaker 2018-09-20 732 static int __init acerhdf_register_thermal(void)
e86435eb91b2bf Peter Feuerer 2009-06-21 733 {
e86435eb91b2bf Peter Feuerer 2009-06-21 734 cl_dev = thermal_cooling_device_register("acerhdf-fan", NULL,
e86435eb91b2bf Peter Feuerer 2009-06-21 735 &acerhdf_cooling_ops);
e86435eb91b2bf Peter Feuerer 2009-06-21 736
e86435eb91b2bf Peter Feuerer 2009-06-21 737 if (IS_ERR(cl_dev))
e86435eb91b2bf Peter Feuerer 2009-06-21 738 return -EINVAL;
e86435eb91b2bf Peter Feuerer 2009-06-21 739
7e8b6d737da9c6 Peter Feuerer 2014-11-28 740 thz_dev = thermal_zone_device_register("acerhdf", 2, 0, NULL,
48c8dd64345ba2 Peter Feuerer 2014-11-28 741 &acerhdf_dev_ops,
48c8dd64345ba2 Peter Feuerer 2014-11-28 742 &acerhdf_zone_params, 0,
e86435eb91b2bf Peter Feuerer 2009-06-21 743 (kernelmode) ? interval*1000 : 0);
e86435eb91b2bf Peter Feuerer 2009-06-21 744 if (IS_ERR(thz_dev))
e86435eb91b2bf Peter Feuerer 2009-06-21 745 return -EINVAL;
e86435eb91b2bf Peter Feuerer 2009-06-21 746
48c8dd64345ba2 Peter Feuerer 2014-11-28 @747 if (strcmp(thz_dev->governor->name,
48c8dd64345ba2 Peter Feuerer 2014-11-28 748 acerhdf_zone_params.governor_name)) {
48c8dd64345ba2 Peter Feuerer 2014-11-28 749 pr_err("Didn't get thermal governor %s, perhaps not compiled into thermal subsystem.\n",
48c8dd64345ba2 Peter Feuerer 2014-11-28 750 acerhdf_zone_params.governor_name);
48c8dd64345ba2 Peter Feuerer 2014-11-28 751 return -EINVAL;
48c8dd64345ba2 Peter Feuerer 2014-11-28 752 }
48c8dd64345ba2 Peter Feuerer 2014-11-28 753
e86435eb91b2bf Peter Feuerer 2009-06-21 754 return 0;
e86435eb91b2bf Peter Feuerer 2009-06-21 755 }
e86435eb91b2bf Peter Feuerer 2009-06-21 756

:::::: The code at line 747 was first introduced by commit
:::::: 48c8dd64345ba2a8c41556095c7adacb1c8af7c1 acerhdf: Use bang-bang thermal governor

:::::: TO: Peter Feuerer <[email protected]>
:::::: CC: Darren Hart <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.49 kB)
.config.gz (42.50 kB)
Download all attachments

2019-10-14 09:56:06

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 11/11] thermal: Move thermal governor structure to internal header

Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on soc-thermal/next]
[cannot apply to v5.4-rc3 next-20191011]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Daniel-Lezcano/thermal-Move-default-governor-config-option-to-the-internal-header/20191014-103735
base: https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git next
config: i386-randconfig-g003-201941 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All warnings (new ones prefixed by >>):

In file included from include/linux/export.h:45:0,
from include/linux/linkage.h:7,
from include/linux/kernel.h:8,
from drivers/platform/x86/acerhdf.c:23:
drivers/platform/x86/acerhdf.c: In function 'acerhdf_register_thermal':
drivers/platform/x86/acerhdf.c:747:30: error: dereferencing pointer to incomplete type 'struct thermal_governor'
if (strcmp(thz_dev->governor->name,
^
include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
>> drivers/platform/x86/acerhdf.c:747:2: note: in expansion of macro 'if'
if (strcmp(thz_dev->governor->name,
^~

vim +/if +747 drivers/platform/x86/acerhdf.c

e86435eb91b2bf Peter Feuerer 2009-06-21 731
1d0c3fd01afbd5 Paul Gortmaker 2018-09-20 732 static int __init acerhdf_register_thermal(void)
e86435eb91b2bf Peter Feuerer 2009-06-21 733 {
e86435eb91b2bf Peter Feuerer 2009-06-21 734 cl_dev = thermal_cooling_device_register("acerhdf-fan", NULL,
e86435eb91b2bf Peter Feuerer 2009-06-21 735 &acerhdf_cooling_ops);
e86435eb91b2bf Peter Feuerer 2009-06-21 736
e86435eb91b2bf Peter Feuerer 2009-06-21 737 if (IS_ERR(cl_dev))
e86435eb91b2bf Peter Feuerer 2009-06-21 738 return -EINVAL;
e86435eb91b2bf Peter Feuerer 2009-06-21 739
7e8b6d737da9c6 Peter Feuerer 2014-11-28 740 thz_dev = thermal_zone_device_register("acerhdf", 2, 0, NULL,
48c8dd64345ba2 Peter Feuerer 2014-11-28 741 &acerhdf_dev_ops,
48c8dd64345ba2 Peter Feuerer 2014-11-28 742 &acerhdf_zone_params, 0,
e86435eb91b2bf Peter Feuerer 2009-06-21 743 (kernelmode) ? interval*1000 : 0);
e86435eb91b2bf Peter Feuerer 2009-06-21 744 if (IS_ERR(thz_dev))
e86435eb91b2bf Peter Feuerer 2009-06-21 745 return -EINVAL;
e86435eb91b2bf Peter Feuerer 2009-06-21 746
48c8dd64345ba2 Peter Feuerer 2014-11-28 @747 if (strcmp(thz_dev->governor->name,
48c8dd64345ba2 Peter Feuerer 2014-11-28 748 acerhdf_zone_params.governor_name)) {
48c8dd64345ba2 Peter Feuerer 2014-11-28 749 pr_err("Didn't get thermal governor %s, perhaps not compiled into thermal subsystem.\n",
48c8dd64345ba2 Peter Feuerer 2014-11-28 750 acerhdf_zone_params.governor_name);
48c8dd64345ba2 Peter Feuerer 2014-11-28 751 return -EINVAL;
48c8dd64345ba2 Peter Feuerer 2014-11-28 752 }
48c8dd64345ba2 Peter Feuerer 2014-11-28 753
e86435eb91b2bf Peter Feuerer 2009-06-21 754 return 0;
e86435eb91b2bf Peter Feuerer 2009-06-21 755 }
e86435eb91b2bf Peter Feuerer 2009-06-21 756

:::::: The code at line 747 was first introduced by commit
:::::: 48c8dd64345ba2a8c41556095c7adacb1c8af7c1 acerhdf: Use bang-bang thermal governor

:::::: TO: Peter Feuerer <[email protected]>
:::::: CC: Darren Hart <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (4.09 kB)
.config.gz (32.33 kB)
Download all attachments

2019-10-14 14:33:25

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH 11/11] thermal: Move thermal governor structure to internal header

On Sat, Oct 12, 2019 at 12:23 PM Daniel Lezcano
<[email protected]> wrote:
>
> The thermal governor structure is a big structure where no
> user should change value inside except via helper functions.
>
> Move the structure to the internal header thus preventing external
> code to be tempted by hacking the structure's variables.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
> drivers/thermal/thermal_core.h | 20 ++++++++++++++++++++
> include/linux/thermal.h | 21 +--------------------
> 2 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index c75309d858ce..e54150fa4c5b 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -46,6 +46,26 @@ struct thermal_attr {
> char name[THERMAL_NAME_LENGTH];
> };
>
> +/**
> + * struct thermal_governor - structure that holds thermal governor information
> + * @name: name of the governor
> + * @bind_to_tz: callback called when binding to a thermal zone. If it
> + * returns 0, the governor is bound to the thermal zone,
> + * otherwise it fails.
> + * @unbind_from_tz: callback called when a governor is unbound from a
> + * thermal zone.
> + * @throttle: callback called for every trip point even if temperature is
> + * below the trip point temperature
> + * @governor_list: node in thermal_governor_list (in thermal_core.c)
> + */
> +struct thermal_governor {
> + char name[THERMAL_NAME_LENGTH];
> + int (*bind_to_tz)(struct thermal_zone_device *tz);
> + void (*unbind_from_tz)(struct thermal_zone_device *tz);
> + int (*throttle)(struct thermal_zone_device *tz, int trip);
> + struct list_head governor_list;
> +};
> +
> static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
> {
> return cdev->ops->get_requested_power && cdev->ops->state2power &&
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 8daa179918a1..04264e8a2bce 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -45,6 +45,7 @@
>
> struct thermal_zone_device;
> struct thermal_cooling_device;
> +struct thermal_governor;
> struct thermal_instance;
> struct thermal_attr;
>
> @@ -206,26 +207,6 @@ struct thermal_zone_device {
> enum thermal_notify_event notify_event;
> };
>
> -/**
> - * struct thermal_governor - structure that holds thermal governor information
> - * @name: name of the governor

AFAICT, some drivers actually like to specify what governor to use
when calling thermal_zone_device_register(), by passing the
thermal_zone_params parameter. You will probably need to provide for
helper functions to return the value of governor name, I think.

> - * @bind_to_tz: callback called when binding to a thermal zone. If it
> - * returns 0, the governor is bound to the thermal zone,
> - * otherwise it fails.
> - * @unbind_from_tz: callback called when a governor is unbound from a
> - * thermal zone.
> - * @throttle: callback called for every trip point even if temperature is
> - * below the trip point temperature
> - * @governor_list: node in thermal_governor_list (in thermal_core.c)
> - */
> -struct thermal_governor {
> - char name[THERMAL_NAME_LENGTH];
> - int (*bind_to_tz)(struct thermal_zone_device *tz);
> - void (*unbind_from_tz)(struct thermal_zone_device *tz);
> - int (*throttle)(struct thermal_zone_device *tz, int trip);
> - struct list_head governor_list;
> -};
> -
> /* Structure that holds binding parameters for a zone */
> struct thermal_bind_params {
> struct thermal_cooling_device *cdev;
> --
> 2.17.1
>

2019-10-14 14:33:57

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH 05/11] thermal: Move set_trips function to the internal header

On Sat, Oct 12, 2019 at 12:23 PM Daniel Lezcano
<[email protected]> wrote:
>
> The function is not used in other place than the thermal directory. It

Grammar nit: is not used any place other than the thermal directory

> does not make sense to export its definition in the global header as
> there is no use of it.
>
> Move its the definition in the internal header and allow better
> self-encapsulation.

Grammar nit: Move the definition to the internal header

If you respin please fix the same for other patches in the series.




>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
> drivers/thermal/thermal_core.h | 2 ++
> include/linux/thermal.h | 3 ---
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 6f6e0dcba4f2..301f5603def1 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -72,6 +72,8 @@ struct thermal_trip {
> enum thermal_trip_type type;
> };
>
> +void thermal_zone_set_trips(struct thermal_zone_device *tz);
> +
> /*
> * This structure is used to describe the behavior of
> * a certain cooling device on a certain trip point
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 88e1faa3d72c..761d77571533 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -398,7 +398,6 @@ int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int,
> struct thermal_cooling_device *);
> void thermal_zone_device_update(struct thermal_zone_device *,
> enum thermal_notify_event);
> -void thermal_zone_set_trips(struct thermal_zone_device *);
>
> struct thermal_cooling_device *thermal_cooling_device_register(const char *,
> void *, const struct thermal_cooling_device_ops *);
> @@ -444,8 +443,6 @@ static inline int thermal_zone_unbind_cooling_device(
> static inline void thermal_zone_device_update(struct thermal_zone_device *tz,
> enum thermal_notify_event event)
> { }
> -static inline void thermal_zone_set_trips(struct thermal_zone_device *tz)
> -{ }
> static inline struct thermal_cooling_device *
> thermal_cooling_device_register(char *type, void *devdata,
> const struct thermal_cooling_device_ops *ops)
> --
> 2.17.1
>